Re: Fix some error handling for read() and errno

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Postgres hackers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Robbie Harwood <rharwood(at)redhat(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, hlinnaka <hlinnaka(at)iki(dot)fi>
Subject: Re: Fix some error handling for read() and errno
Date: 2018-07-14 06:37:56
Message-ID: 20180714063756.GA1777@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Jul 14, 2018 at 12:50:02AM -0400, Alvaro Herrera wrote:
> I did read them, though not in minute detail. 0002 seems to result in
> code easier to read. If there are particular places that deviate from
> the obvious patterns, I didn't notice them.
>
> In 0001 one thing I wasn't terribly in love with was random deviations
> in sprintf format specifiers for things that should be identical, ie.
> %lu in some places and %zu in others, for "read only %d of %d". It
> seems you should pick the more general one (%zu) and use casts to Size
> (or is it size_t?) in the places that have other types. That way you
> *really* decrease translator effort to the bare minimum :-)

Yeah, that would be really the point, hence let's use "could not read
file \"%s\": read %d of %zu" everywhere with a cast to Size. That's
used in some other places as well.

> Ah, in 0001 you have one case of "could not read _from_" (in
> SimpleXLogPageRead). The "from" is not present in the other places.
> Looks odd.

Right.

> I'm not sure about putting the wait event stuff inside the new
> functions. It looks odd, though I understand why you did it.

I don't really find that strange as any transient files should be
tracked by I/O wait events :)

> No opinion on the 2PC stuff -- didn't look at that.

For now, I think that just moving forward with 0001, and then revisit
0002 once the other 2PC patch is settled makes the most sense. On the
other thread, the current 2PC behavior can create silent data loss so
I would like to back-patch it, so that would be less work.
--
Michael

Attachment Content-Type Size
0001-Rework-error-messages-around-file-handling.patch text/x-diff 23.8 KB
0002-Add-interface-to-read-write-fsync-with-transient-fil.patch text/x-diff 16.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2018-07-14 07:26:26 Re: GiST VACUUM
Previous Message Alvaro Herrera 2018-07-14 04:50:02 Re: Fix some error handling for read() and errno