On Sat, Jun 23, 2018 at 08:48:03AM +0900, Michael Paquier wrote:
> That's exactly why I have started this thread so as both problems are
> addressed separately:
> https://www.postgresql.org/message-id/20180622061535.GD5215@paquier.xyz
> And back-patching the errno patch while only bothering about messages on
> HEAD matches also what I got in mind. I'll come back to this thread
> once the errno issues are all addressed.
As this one is done, I have been looking at that this thread again.
Peter Eisentraut has pushed as e5d11b9 something which does not need to
worry about pluralization of error messages. So I have moved to this
message style for all messages. All of this is done as 0001.
I have been thinking as well about a common interface which could be
used to read/write/fsync transient files:
void WriteTransientFile(int fd, char *buf, Size count, int elevel,
const char *filename, uint32 wait_event_info);
bool ReadTransientFile(int fd, char *buf, Size count, int elevel,
const char *filename, uint32 wait_event_info);
void SyncTransientFile(int fd, int elevel, const char *filename
uint32 wait_event_info);
There are rather equivalent things with FileRead and FileWrite but the
purpose is different as well.
If you look at 0002, this shaves a bit of code:
6 files changed, 128 insertions(+), 200 deletions(-)
There are also a couple of advantages here:
- Centralize errno handling for transient files with ENOSPC for write(2)
and read count for read(2)
- Wait events have to be defined, so those would unlikely get forgotten
in the future.
- Error handling for CloseTransientFile in code paths is centralized.
ReadTransientFile could be redefined to return the number of bytes read
as result with caller checking for errno, but that feels a bit duplicate
work for twophase.c. WriteTransientFile and SyncTransientFile could
also have the same treatment for consistency but they would not really
be used now. Do you guys think that this is worth pursuing? Merging
0001 and 0002 together may make sense then.
--
Michael