Thread: Re: [BUGS] Fails to work on live images due to fsync() onpg_commit_ts before doing any write there

On Tue, 07 Nov 2017, Stephen Frost wrote:
> While I agree with this, I'm not entirely convinced that this isn't an
> issue with the implementation of the underlying filesystem after all.  I
> haven't had a chance to go read those other bug reports, but my fsync()
> manpage pretty clearly seems to say that fsync should only be returning
> EINVAL if it's called on a special file (FIFO, pipe, et al).  There's
> certainly no indication that it's ok for the same file to sometimes
> support fsync() and other times *not* support fsync().  That's pretty
> bizarre.

The manpage shows "EROFS" on the same line as "EINVAL", and "EROFS" stands
for "Read-Only FileSystem". So it seems to be normal for read-only
filesystems to return errors (arguably squashfs returns EINVAL and
it would have been better to use EROFS).

And overlayfs, by its nature, is made to forward file system calls
to different underlying filesystems (that thus have different
characteristics).

That said I agree that overlayfs should be smarter and it should hide
EROFS/EINVAL on fsync() when it knows that the call is delegated to a
read-only filesystem.

Still I believe that this issue should be fixed in both sides. It's not
smart from PostgreSQL to call fsync() when it has not made any change.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Raphael,

* Raphael Hertzog (hertzog@debian.org) wrote:
> Still I believe that this issue should be fixed in both sides. It's not
> smart from PostgreSQL to call fsync() when it has not made any change.

Why?

Thanks!

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> * Raphael Hertzog (hertzog@debian.org) wrote:
>> Still I believe that this issue should be fixed in both sides. It's not
>> smart from PostgreSQL to call fsync() when it has not made any change.

> Why?

My thought about this is just to ignore EINVAL when fsync'ing a directory,
as we already do with EBADF.
        regards, tom lane


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > * Raphael Hertzog (hertzog@debian.org) wrote:
> >> Still I believe that this issue should be fixed in both sides. It's not
> >> smart from PostgreSQL to call fsync() when it has not made any change.
>
> > Why?
>
> My thought about this is just to ignore EINVAL when fsync'ing a directory,
> as we already do with EBADF.

Yeah, I suppose we could, just not sure that an EINVAL should really be
getting returned here, imv.

Thanks!

Stephen

On Tue, 07 Nov 2017, Stephen Frost wrote:
> Raphael,
> 
> * Raphael Hertzog (hertzog@debian.org) wrote:
> > Still I believe that this issue should be fixed in both sides. It's not
> > smart from PostgreSQL to call fsync() when it has not made any change.
> 
> Why?

Because it's useless. Why call fsync() when you know that it doesn't do
anything?

And because I have just shown that doing this can have unexpected
side-effects in some specific conditions.

But this is really a matter of aesthetics and trade-offs. I don't know the
code. It might be difficult to track the fact that changes have been made.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> My thought about this is just to ignore EINVAL when fsync'ing a directory,
>> as we already do with EBADF.

> Yeah, I suppose we could, just not sure that an EINVAL should really be
> getting returned here, imv.

Yeah, it does sound more like a filesystem bug than anything else.
(I completely reject the notion that it's an application error to
fsync a directory when you haven't modified it.)

In view of the fact that we just wrapped 10.1, it'd be 3 months before
any change from our side would reach the wild.  I think a key question
here is whether the Kali developers are likely to fix it from their side
in less time than that.
        regards, tom lane


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

On 2017-11-07 16:06:02 +0100, Raphael Hertzog wrote:
> On Tue, 07 Nov 2017, Stephen Frost wrote:
> > Raphael,
> > 
> > * Raphael Hertzog (hertzog@debian.org) wrote:
> > > Still I believe that this issue should be fixed in both sides. It's not
> > > smart from PostgreSQL to call fsync() when it has not made any change.
> > 
> > Why?
> 
> Because it's useless. Why call fsync() when you know that it doesn't do
> anything?

It's not generally useless, no. You don't know if the relevant directory
hasn't been modified since postgres was started last / crashed
last. That matters for the durability and atomicity of renames
etc. There's simply no way to track that in postgres, given it's
arbitrary external effects.

Greetings,

Andres Freund


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

On Tue, 07 Nov 2017, Tom Lane wrote:
> Yeah, it does sound more like a filesystem bug than anything else.
> (I completely reject the notion that it's an application error to
> fsync a directory when you haven't modified it.)
> 
> In view of the fact that we just wrapped 10.1, it'd be 3 months before
> any change from our side would reach the wild.  I think a key question
> here is whether the Kali developers are likely to fix it from their side
> in less time than that.

Fix what?

Following your logic, if anything needs to be fixed, it's the Linux kernel
that has to be fixed, and in particular its overlayfs filesystem. The time
to get a fix there is also relatively long. If they work on a fix quickly,
it might get merged for Linux 3.15 which we will not have in Kali before
2-3 months too.

But I can always cherry-pick a patch, either in PostgreSQL or in Linux.
BTW I'm happy to test a PostgreSQL patch for instance.

For now, I have added an ugly work-around that creates a file in the
pg_commit_ts directory (and immediately drops it) just before starting
PostgreSQL:
http://git.kali.org/gitweb/?p=packages/kali-defaults.git;a=commitdiff;h=f4287fd774f8d0686ba919f57355215a6b9633e3

Note that it's perfectly fine for me if you decide to resolve this issue
by ignoring EINVAL and/or EROFS on fsync on directories.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

On 2017-11-07 16:23:21 +0100, Raphael Hertzog wrote:
> On Tue, 07 Nov 2017, Tom Lane wrote:
> > Yeah, it does sound more like a filesystem bug than anything else.
> > (I completely reject the notion that it's an application error to
> > fsync a directory when you haven't modified it.)
> >
> > In view of the fact that we just wrapped 10.1, it'd be 3 months before
> > any change from our side would reach the wild.  I think a key question
> > here is whether the Kali developers are likely to fix it from their side
> > in less time than that.
>
> Fix what?
>
> Following your logic, if anything needs to be fixed, it's the Linux kernel
> that has to be fixed, and in particular its overlayfs filesystem.

Right. The kernel knows the file/directory ain't dirty. We don't.


> The time
> to get a fix there is also relatively long. If they work on a fix quickly,
> it might get merged for Linux 3.15 which we will not have in Kali before
> 2-3 months too.

So the same timeframe as a new PG release?


> For now, I have added an ugly work-around that creates a file in the
> pg_commit_ts directory (and immediately drops it) just before starting
> PostgreSQL:
> http://git.kali.org/gitweb/?p=packages/kali-defaults.git;a=commitdiff;h=f4287fd774f8d0686ba919f57355215a6b9633e3

Hm. The comment:
+    # Ensure the directory is copied over in the tmpfs
suggests that the overlay isn't persistent? If that's indeed the case,
you could just disable fsyncs alltogether.

Greetings,

Andres Freund


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

On Tue, 07 Nov 2017, Andres Freund wrote:
> > The time
> > to get a fix there is also relatively long. If they work on a fix quickly,
> > it might get merged for Linux 3.15 which we will not have in Kali before
> > 2-3 months too.
> 
> So the same timeframe as a new PG release?

Yes, looks like so.

> > For now, I have added an ugly work-around that creates a file in the
> > pg_commit_ts directory (and immediately drops it) just before starting
> > PostgreSQL:
> > http://git.kali.org/gitweb/?p=packages/kali-defaults.git;a=commitdiff;h=f4287fd774f8d0686ba919f57355215a6b9633e3
> 
> Hm. The comment:
> +    # Ensure the directory is copied over in the tmpfs
> suggests that the overlay isn't persistent? If that's indeed the case,
> you could just disable fsyncs alltogether.

Well, it's the case by default with no persistence. But some users enable
persistence and then it's no longer a tmpfs which is used.

The problem also exists when you enable persistence but start with an
empty upper layer so disabling fsyncs is not really an option.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: https://www.freexian.com/services/debian-lts.html
Learn to master Debian: https://debian-handbook.info/get/


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs