From: | Stephen Frost <sfrost(at)snowman(dot)net> |
---|---|
To: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Postgres, fsync, and OSs (specifically linux) |
Date: | 2018-05-19 00:27:57 |
Message-ID: | 20180519002757.GU27724@tamriel.snowman.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Greetings,
* Ashutosh Bapat (ashutosh(dot)bapat(at)enterprisedb(dot)com) wrote:
> On Thu, May 17, 2018 at 11:45 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > On Thu, May 17, 2018 at 12:44 PM, Andres Freund <andres(at)anarazel(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Abhijit Menon-Sen | 2018-05-19 00:32:10 | Re: Postgres, fsync, and OSs (specifically linux) |
Previous Message | Stephen Frost | 2018-05-19 00:13:17 | Re: Flexible permissions for REFRESH MATERIALIZED VIEW |