Date: Mon, 18 Feb 2019 11:23:39 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: John Klann <jk7255@gmail.com>,
	PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #15636: PostgreSQL 11.1 pg_basebackup backup to a CIFS
 destination throws fsync error at end of backup
Message-ID: <20190218162338.GG6197@tamriel.snowman.net>
References: 
 <CAHyX5+VcEtcoOAo4QQ_20gv7kLYBDnx_YOAnEJQN9KftH0qp9Q@mail.gmail.com>
 <CAEepm=0eC5ySeOsXMqXQFeJ64mhgAqxAyuh7JpjCz7uT2Ld0jQ@mail.gmail.com>
 <CAHyX5+XqkmHwxLC5aeYOjHFAvnf8X1o3grkzGJOvc-kOGpjQ8g@mail.gmail.com>
 <CAHyX5+VO-UTE-pK2i1rWvweU2CpVtHuhXnok29Z66EpLHmiHvw@mail.gmail.com>
 <CAEepm=05ff+GRYdVE7+SdDuTVbPqW+Gmpwqir0NtAWNM70jg3g@mail.gmail.com>
 <CAHyX5+W2OXAg=qEnObA3g4zU3SkwxTztSfmgmivWnWQWcsc_tA@mail.gmail.com>
 <CAEepm=1P0WD_g=ELNmBJFhwKuCjYZOi4p4rutKj3kw7QneLB2A@mail.gmail.com>
 <CAHyX5+UxmOn-qpb+++NOr3MfZiXk6WZDKFYTEj+vPbu+3nrhXg@mail.gmail.com>
 <CAHyX5+X60D0jtf4dgyt11mH=7XvuW1UOLstY9sBLgukyf8Oo4g@mail.gmail.com>
 <CAEepm=27vTw2xyTh5kAXns8h5zOKUmzBxUL82c-KFFgyUQ0MHw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="xdAuejbAAr3yyIdQ"
Content-Disposition: inline
In-Reply-To: 
 <CAEepm=27vTw2xyTh5kAXns8h5zOKUmzBxUL82c-KFFgyUQ0MHw@mail.gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


--xdAuejbAAr3yyIdQ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Thomas Munro (thomas.munro@enterprisedb.com) wrote:
> On Sat, Feb 16, 2019 at 4:39 AM John Klann <jk7255@gmail.com> wrote:
> > I was able to test the patch without issue and it appears to have worke=
d see below. Let me know if there is further testing I can do or logging yo=
u would like.
>=20
> Thanks.  So that leaves the question for the list: should we accept
> this as a bug and consider back-patching it?  Our relationship with
> fsync() is ... complicated at the moment.

Yes, we should accept this as a bug and back-patch it, imv.

> Considering that we already
> tolerated EBADF for directories on some other (forgotten?) operating
> system, I think I would vote +1 for doing that ... if I didn't know
> that they'd fixed this on the Linux side, and I see now that they've
> back-patched that even to long term kernel 3.16:
>=20
> https://cdn.kernel.org/pub/linux/kernel/v3.x/ChangeLog-3.16.60

Which seems like it's just another good reason why we should also
back-patch it.

> PS I found an earlier discussion of this topic that didn't go
> anywhere:  https://www.postgresql.org/message-id/flat/E1RhkjA-0005bq-B6%4=
0wrigleys.postgresql.org

As does this.

I would be much more concerned about a change like this if we either
didn't know it was a directory that we got fsync() EINVAL for.  Since we
know it's a directory and we know there's filesystems out there which do
synchronous metadata changes and we know they return EINVAL in those
cases, I'm inclined to say we should allow it.  In the above thread, Tom
mentioned the standard, which pretty clearly distinguishes between "this
thing you gave to fsync() doesn't support synchronization" and "an I/O
error occured" and it's really the latter that we care about here.

Further, saying "just use --no-sync" is *not* a workaround.  That's
throwing the baby out with the bathwater.  A workaround would be an
option like "--do-not-complain-about-directory-fsync-einval", but
suggesting that all fsync's be removed is terrible.

Thanks!

Stephen

--xdAuejbAAr3yyIdQ
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJcatwKAAoJEO1sijiDR2RViv8P/1ixA631c4KIFy+t8edq4Tmz
Xplka5rRnDr2IXR38G/MK3zT9yDoCus7Su3GhiJSQBj/SVySv51C21X0e4C1B819
nEF/DIv/HHNary7U5nixCSKhzLZjXIv0U0PJAnNlaWF6cBUJZZ5N4Tte/mUsMKb8
0/o42wCl7W2slDU8Tervv76EM0puIV+Gcq2966X1mFH9ZkXrsHRJr7+Xw134Zvs3
yUsHMl8rhEEmRkCTW8XYy+zy3UlnODIcwiX0FsjnQoRNeyW1m0RI66m2fdiCwqvF
Ypj28KDwxSptpjjcJotdHYU3Ar0gFIRDs+znoD/6OwVjCaH5VS1cQ+fhxfOuhWUL
hn3PmykPKdo8fDJzXT7dMf6vwnBxJix9W1S9Wu5fPlTLhCXcXKLUcJMWBNTMY6yX
ZbU8GBSx8abFHlXdUf6t81VHhWLDPfFEmIrcMPgceRdnBdiMF1L7IxVjI52NNN1e
yC5uB/Cx5iN80hgrXBZgMAGD1RPh89AhoGcz7lk5qTKBp52yJISuPqHX/E5+pDUM
u/04KrGFNZ+Fz5RdJ/DiSFJj8Ok1rWOENEikWx0FwFN4/4sP8XZ05CCz96Ttf6tB
ZFHU8RZzqBE2PgF3JO7ZQ3PXPOe54/4MRb0cB/OlJ/1NdJWmcGJ2bwBK0x40T+aR
cpddl6pGnXnH9JC/WOZf
=AveV
-----END PGP SIGNATURE-----

--xdAuejbAAr3yyIdQ--