Обсуждение: Re: [BUGS] BUG #13350: blindly fsyncing data dir considered harmful
All,
(sending to -core to request a release)
* andrew@tao11.riddles.org.uk (andrew@tao11.riddles.org.uk) wrote:
> Operating system: Debian (and probably others)
> The addition of a recursive fsync of the data dir on startup (in the absence
> of a clean shutdown) causes startup to fail if the data dir contains
> symlinks to files which the postgres user can't write to.
>
> This is the standard configuration for many SSL-enabled setups, including
> the standard debian packaging defaults. Accordingly, crash recovery now
> ALWAYS fails on such systems without manual intervention.
Andrew did a great job summarizing the problem, don't know that there's
much to add there.
This was back-patched all the way and released with the latest round of
minor releases, and given that it means crash recovery fails for a large
number of deployed systems, I think we need to fix (or revert) the
recursive fsync change (d8ac77ab178ddb2ae043b8c463cd30c031e793d0 and
related) and do new releases very shortly.
Thanks!
Stephen
<p dir="ltr"><br /> On May 25, 2015 5:12 PM, "Stephen Frost" <<a href="mailto:sfrost@snowman.net">sfrost@snowman.net</a>>wrote:<br /> ><br /> > All,<br /> ><br /> > (sendingto -core to request a release)<br /> ><br /> > * <a href="mailto:andrew@tao11.riddles.org.uk">andrew@tao11.riddles.org.uk</a>(<a href="mailto:andrew@tao11.riddles.org.uk">andrew@tao11.riddles.org.uk</a>)wrote:<br /> > > Operating system: Debian(and probably others)<br /> ><br /> > > The addition of a recursive fsync of the data dir on startup (inthe absence<br /> > > of a clean shutdown) causes startup to fail if the data dir contains<br /> > > symlinksto files which the postgres user can't write to.<br /> > ><br /> > > This is the standard configurationfor many SSL-enabled setups, including<br /> > > the standard debian packaging defaults. Accordingly,crash recovery now<br /> > > ALWAYS fails on such systems without manual intervention.<br /> ><br />> Andrew did a great job summarizing the problem, don't know that there's<br /> > much to add there.<br /> ><br/> > This was back-patched all the way and released with the latest round of<br /> > minor releases, and giventhat it means crash recovery fails for a large<br /> > number of deployed systems, I think we need to fix (or revert)the<br /> > recursive fsync change (d8ac77ab178ddb2ae043b8c463cd30c031e793d0 and<br /> > related) and do newreleases very shortly.<br /> ><p dir="ltr">Agreed, this is a pretty bad regression and we need to at least do somethingand out out a release asap - either revert or if we can find a better way (see the other thread about this issuefor some other ideas). <p dir="ltr">It happens to be the default shipment on Debian and Ubuntu but it's definitely nota platform specific problem I believe, so we should put out a "real" release and not expect packagers to carry a specificpatch for it. <p dir="ltr">/Magnus
* Magnus Hagander (magnus@hagander.net) wrote:
> On May 25, 2015 5:12 PM, "Stephen Frost" <sfrost@snowman.net> wrote:
> > This was back-patched all the way and released with the latest round of
> > minor releases, and given that it means crash recovery fails for a large
> > number of deployed systems, I think we need to fix (or revert) the
> > recursive fsync change (d8ac77ab178ddb2ae043b8c463cd30c031e793d0 and
> > related) and do new releases very shortly.
>
> Agreed, this is a pretty bad regression and we need to at least do
> something and out out a release asap - either revert or if we can find a
> better way (see the other thread about this issue for some other ideas).
>
> It happens to be the default shipment on Debian and Ubuntu but it's
> definitely not a platform specific problem I believe, so we should put out
> a "real" release and not expect packagers to carry a specific patch for it.
Agreed, there are certainly other reasons why a file might exist which
can't be written to by the postgres user, we really can't have crash
recovery fail because of it.
Further, I believe that a lot of the .deb-based distributions use
the same technique of using symlinks (Ubuntu included, but even those
who aren't downstream of Debian), and they would all have to be updated
with such a patch. Sadly, suggesting to stay on a prior version (eg:
9.4.1) really isn't acceptable either, given the corruption risks
which 9.4.2 addressed.
Thanks!
Stephen
What exactly is failing? Is it that fsync is returning -1 ? Should we just ignore errors from fsync if it happens in this stage? That might be safer than determining which files should or shouldn't be fsynced. That would also have an impact on people starting up on a flaky file system perhaps. I'm imagining either a database mounted on a filesystem with corruption trying to extract what they can or perhaps something like an NFS filesystem with dangling .nfs files. On the one hand limping along as best we can is the general Postgres strategy when the filesystem starts failing but on the other hand we have had circumstances in the past when users had a database on a network filesystem that wasn't really ready for use yet when Postgres tried to start up and starting up anyways didn't do them any favours.
On Mon, May 25, 2015 at 04:37:59PM +0100, Greg Stark wrote: > What exactly is failing? > > Is it that fsync is returning -1 ? Should we just ignore errors from > fsync if it happens in this stage? That might be safer than > determining which files should or shouldn't be fsynced. Interesting idea. We could skip fsync -1 failures only for symbolic links. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
On May 25, 2015 8:52:33 AM PDT, Bruce Momjian <bruce@momjian.us> wrote: >On Mon, May 25, 2015 at 04:37:59PM +0100, Greg Stark wrote: >> What exactly is failing? >> >> Is it that fsync is returning -1 ? Should we just ignore errors from >> fsync if it happens in this stage? That might be safer than >> determining which files should or shouldn't be fsynced. > >Interesting idea. We could skip fsync -1 failures only for symbolic >links. Now there's already a thread on jackets about this. Andres --- Please excuse brevity and formatting - I am writing this on my mobile phone.
Greg Stark <stark@mit.edu> writes:
> What exactly is failing?
> Is it that fsync is returning -1 ?
According to the original report from Christoph Berg, it was open()
not fsync() that was failing, at least in permissions-based cases.
I'm not sure if we should just uniformly ignore all failures in this
phase. That would have the merit of clearly not creating any new
startup failure cases compared to the previous code, but as you say
sometimes it might mean ignoring real problems.
Another idea would be to elog(LOG) such failures but press on anyway.
Lastly, there is a difference between ignoring failures in fsync_fname()
and ignoring failures in walkdir(). The latter is presumably a general
purpose subroutine and so I would expect it to throw errors if it gets a
failure in, say, opendir(). Which it does. But do we want that in this
context? Unreadable directories underneath $PGDATA are probably not a
good thing, but OTOH this would still mean that there's a new startup
failure condition compared to before.
regards, tom lane
* Andres Freund (andres@anarazel.de) wrote:
> On May 25, 2015 8:52:33 AM PDT, Bruce Momjian <bruce@momjian.us> wrote:
> >On Mon, May 25, 2015 at 04:37:59PM +0100, Greg Stark wrote:
> >> What exactly is failing?
> >>
> >> Is it that fsync is returning -1 ? Should we just ignore errors from
> >> fsync if it happens in this stage? That might be safer than
> >> determining which files should or shouldn't be fsynced.
> >
> >Interesting idea. We could skip fsync -1 failures only for symbolic
> >links.
>
> Now there's already a thread on jackets about this.
Agreed- we should probably keep the discussion about fixing it (or
addressing the original issue in some way that doesn't run into this
issue) on the other thread. This is simply to point out the user impact
and that we need to fix it quickly.
Thanks!
Stephen
All, If it's any consolation, the folks at kernel.org are having a bad week too: http://lwn.net/Articles/645720/ -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 2015-05-25 14:11:34 -0700, Josh Berkus wrote: > If it's any consolation, the folks at kernel.org are having a bad week > too: http://lwn.net/Articles/645720/ This doesn't seem to come even close to their problems ;). A problem that you can fix by making permissions isn't that bad. Greetings, Andres Freund