From: Stephen Frost <sfrost@snowman.net>
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Craig Ringer <craig@2ndquadrant.com>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Simon Riggs <simon@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Postgres, fsync, and OSs (specifically linux)
Date: Fri, 18 May 2018 20:27:57 -0400
Message-ID: <20180519002757.GU27724@tamriel.snowman.net> (raw)
In-Reply-To: <CAFjFpRco+fapy2383raQhzNJRpVivEcSxFBxL5M-4YJZ8T-wbg@mail.gmail.com>
References: <20180430021738.2zip3pfovzm6bpjc@alap3.anarazel.de>
<CAMsr+YGq5CdhTB_OScCraSf0__vk6nFVhpp6fONOfOZAZKG=rQ@mail.gmail.com>
<CAMsr+YF=FfuNCcBJXV4tDsxQQ=Yu5GfhZ-z6iiu+Y4x2MYzX0Q@mail.gmail.com>
<20180430160945.5s5qfoqryhtmugxl@alap3.anarazel.de>
<CAMsr+YGsvYmJ0aRuV7zNf-rmi53bzFF8yrsgrNBm9yOY8FYD0g@mail.gmail.com>
<20180509225555.rmk3azmgrwfwudw5@alap3.anarazel.de>
<CAMsr+YEcv3gvLHX-gTMnA0Z_YCChJ+rCMTGnx1aQMUFmp4Lvbw@mail.gmail.com>
<20180517164440.szk6vhbdprcm462a@alap3.anarazel.de>
<CA+Tgmob0sD3BGo46ZXK4hJw=2NDco10Zsx=rzTMxVXY0kHRvxg@mail.gmail.com>
<CAFjFpRco+fapy2383raQhzNJRpVivEcSxFBxL5M-4YJZ8T-wbg@mail.gmail.com>
Greetings,
* Ashutosh Bapat (ashutosh.bapat@enterprisedb.com) wrote:
> On Thu, May 17, 2018 at 11:45 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> > On Thu, May 17, 2018 at 12:44 PM, Andres Freund <andres@anarazel.de> wrote:
> >> Hi,
> >>
> >> On 2018-05-10 09:50:03 +0800, Craig Ringer wrote:
> >>> while ((src = (RewriteMappingFile *) hash_seq_search(&seq_status)) != NULL)
> >>> {
> >>> if (FileSync(src->vfd, WAIT_EVENT_LOGICAL_REWRITE_SYNC) != 0)
> >>> - ereport(ERROR,
> >>> + ereport(PANIC,
> >>> (errcode_for_file_access(),
> >>> errmsg("could not fsync file \"%s\": %m", src->path)));
> >>
> >> To me this (and the other callers) doesn't quite look right. First, I
> >> think we should probably be a bit more restrictive about when PANIC
> >> out. It seems like we should PANIC on ENOSPC and EIO, but possibly not
> >> others. Secondly, I think we should centralize the error handling. It
> >> seems likely that we'll acrue some platform specific workarounds, and I
> >> don't want to copy that knowledge everywhere.
> >
> > Maybe something like:
> >
> > ereport(promote_eio_to_panic(ERROR), ...)
>
> Well, searching for places where error is reported with level PANIC,
> using word PANIC would miss these instances. People will have to
> remember to search with promote_eio_to_panic. May be handle the errors
> inside FileSync() itself or a wrapper around that.
No, that search wouldn't miss those instances- such a search would find
promote_eio_to_panic() and then someone would go look up the uses of
that function. That hardly seems like a serious issue for folks hacking
on PG.
I'm not saying that having a wrapper around FileSync() would be bad or
having it handle things, but I don't agree with the general notion that
we can't have a function which handles the complicated bits about the
kind of error because someone grep'ing the source for PANIC might have
to do an additional lookup.
Thanks!
Stephen
Attachments:
signature.asc (application/pgp-signature, 818B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: sfrost@snowman.net, ashutosh.bapat@enterprisedb.com, robertmhaas@gmail.com, andres@anarazel.de, craig@2ndquadrant.com, thomas.munro@enterprisedb.com, simon@2ndquadrant.com
Subject: Re: Postgres, fsync, and OSs (specifically linux)
In-Reply-To: <20180519002757.GU27724@tamriel.snowman.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox