Date: Fri, 18 May 2018 20:27:57 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>,
	Craig Ringer <craig@2ndquadrant.com>,
	Thomas Munro <thomas.munro@enterprisedb.com>,
	Simon Riggs <simon@2ndquadrant.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Postgres, fsync, and OSs (specifically linux)
Message-ID: <20180519002757.GU27724@tamriel.snowman.net>
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>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="/mOTGTYwFs6hsZJl"
Content-Disposition: inline
In-Reply-To: 
 <CAFjFpRco+fapy2383raQhzNJRpVivEcSxFBxL5M-4YJZ8T-wbg@mail.gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


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

Greetings,

* Ashutosh Bapat (ashutosh.bapat@enterprisedb.com) wrote:
> On Thu, May 17, 2018 at 11:45 PM, Robert Haas <robertmhaas@gmail.com> wro=
te:
> > On Thu, May 17, 2018 at 12:44 PM, Andres Freund <andres@anarazel.de> wr=
ote:
> >> Hi,
> >>
> >> On 2018-05-10 09:50:03 +0800, Craig Ringer wrote:
> >>>       while ((src =3D (RewriteMappingFile *) hash_seq_search(&seq_sta=
tus)) !=3D NULL)
> >>>       {
> >>>               if (FileSync(src->vfd, WAIT_EVENT_LOGICAL_REWRITE_SYNC)=
 !=3D 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), ...)
>=20
> 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

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

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

iQIcBAEBCgAGBQJa/2+NAAoJEO1sijiDR2RV9TMQALmxdrBVTWc/JnJ2sbdHc9Q5
tY0fAMuWpCN7a0AfdhM025+hIMota+rW/3aR5AZNHAsriJRd3UBHbSNZncly6UNA
rldTiv5lxZY02oTlrnt4283csrtOc7ox5gNmjLlnHPkAUXUTx3z79uaZIAr8Zrn1
PV/TNScrtrhTgGP7Ha/DBR8u6zfmJEN+JTyVm/NFt+vfiKHoYUuSmPuJXHqXrhTp
hf6BQoHkS6iGklDY2CfcNIdxJ6MPiH9jRslRfJcYvZkR3CeAT52IsGEp8pM8lZ5x
s3QmjXwG/AMU/I7MGxtcnOkh2KuPWVFsVstQPkzjYA9ELwVdbEzxipKrK4l+ofbd
vbWxSHgt0HzKzNZaDQQkhLQrMHQvIur2MZYqz6m05Ngbvvj9dGIHEq1eUFIj0gUX
/Ntdmjsix7XLu3Bgkmua20qrmGBgLs7dvuupYK2xLIKqMZBSk/yvojEiuAbglSXP
ZxknfGaXqvSQAUwMdpBorP2djwC4nPpUSHWxbAJHBk3kH7lhg7cKKRFAq4pzoUo7
l1lWu0BGPS7HtUcPus+w6T+wPSYJEAIWOvzbgxYG1oZtPuxQvfnl91ud8Nl9yWCa
fIWmBvqwiV+vsDlU279U69YYrPz/nhjtTzrfVzXsYzXIwGvq5JM2qRNUwMxqalwf
DysJSrnZXhfj6IG1/VqY
=SAjI
-----END PGP SIGNATURE-----

--/mOTGTYwFs6hsZJl--