On Fri, Jun 29, 2018 at 08:48:33AM -0400, Robert Haas wrote:
> Are there other instances of fsync() that also need to be covered?
Yeah, I double-checked the calls to pg_fsync back when I looked at this
patch, but now that I look again I see at least two more of them:
- fsync_fname_ext.
- write_auto_conf_file, where a wait event for a write() is missing as
well.
Hm. I am wondering if it would not be worth extending pg_fsync() with
an argument for a wait event and introduce a sort of pg_write() which
wraps write() with an extra wait event argument, and similarly for
read() (warning, conflict with Windows ahead!). That's somehow related
to the feeling I had when working with transient file's writes and reads
a couple of days back. Those are most likely going to be forgotten
again and again in the future. In both cases the caller would still be
responsible for looking at errno and decide the error handling, but I
got no feedback about the idea on transient files.
There are also a couple of places where the wait events are longer than
they should. For example in writeTimeLineHistory, there is a wait event
for write() which is still switched on even within an error code path.
And on top of it I think that a call to pgstat_report_wait_end() is
missing in the error code path as on ERROR the session still exists.
That's a bug. Those need an extra lookup and visibly some cleanup back
to v10.
--
Michael