Date: Wed, 17 Mar 2021 17:43:31 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Andres Freund <andres@anarazel.de>,
	Thomas Munro <thomas.munro@gmail.com>,
	Jakub Wartak <Jakub.Wartak@tomtom.com>,
	Alvaro Herrera <alvherre@2ndquadrant.com>,
	Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	Dmitry Dolgov <9erthalion6@gmail.com>,
	David Steele <david@pgmasters.net>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: WIP: WAL prefetch (another approach)
Message-ID: <20210317214331.GF20766@tamriel.snowman.net>
References: 
 <CA+hUKGJ1=pOiNjSgXYJnjE3OyRtp8tjMRcON256e5EFpzPpgtA@mail.gmail.com>
 <VI1PR0701MB69608CBCE44D80857E59572EF6CA0@VI1PR0701MB6960.eurprd07.prod.outlook.com>
 <CA+hUKGKFeYPL9K+SRixcsx1+6HsHhqK+POZyrnnZjw1jERpGcQ@mail.gmail.com>
 <20210210215033.GA27507@tamriel.snowman.net>
 <103ec76d-8bbc-09e1-460c-8785751431b5@enterprisedb.com>
 <20210212044614.5wqjkjbcmbwzever@alap3.anarazel.de>
 <20210213213930.GM27507@tamriel.snowman.net>
 <eed83eaa-91e1-91fe-32d0-ece36c0fa810@enterprisedb.com>
 <20210214231815.GO27507@tamriel.snowman.net>
 <17b3f450-048c-f3a6-7086-259523f33cbc@enterprisedb.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="Ww0II/mGvpRbaaI0"
Content-Disposition: inline
In-Reply-To: <17b3f450-048c-f3a6-7086-259523f33cbc@enterprisedb.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Archived-At: 
 <https://www.postgresql.org/message-id/20210317214331.GF20766%40tamriel.snowman.net>
Precedence: bulk


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

Greetings,

* Tomas Vondra (tomas.vondra@enterprisedb.com) wrote:
> Right, I was just going to point out the FPIs are not necessary - what
> matters is the presence of long streaks of WAL records touching the same
> set of blocks. But people with workloads where this is common likely
> don't need the WAL prefetching at all - the replica can keep up just
> fine, because it doesn't need to do much I/O anyway (and if it can't
> then prefetching won't help much anyway). So just don't enable the
> prefetching, and there'll be no overhead.

Isn't this exactly the common case though..?  Checkpoints happening
every 5 minutes, the replay of the FPI happens first and then the record
is updated and everything's in SB for the later changes?  You mentioned
elsewhere that this would improve 80% of cases but that doesn't seem to
be backed up by anything and certainly doesn't seem likely to be the
case if we're talking about across all PG deployments.  I also disagree
that asking the kernel to go do random I/O for us, even as a prefetch,
is entirely free simply because we won't actually need those pages.  At
the least, it potentially pushes out pages that we might need shortly
=66rom the filesystem cache, no?

> If it was up to me, I'd just get the patch committed as is. Delaying the
> feature because of concerns that it might have some negative effect in
> some cases, when that can be simply mitigated by disabling the feature,
> is not really beneficial for our users.

I don't know that we actually know how many cases it might have a
negative effect on or what the actual amount of such negative case there
might be- that's really why we should probably try to actually benchmark
it and get real numbers behind it, particularly when the chances of
running into such a negative effect with the default configuration (that
is, FPWs enabled) on the more typical platforms (as in, not ZFS) is more
likely to occur in the field than the cases where FPWs are disabled and
someone's running on ZFS.

Perhaps more to the point, it'd be nice to see how this change actually
improves the caes where PG is running with more-or-less the defaults on
the more commonly deployed filesystems.  If it doesn't then maybe it
shouldn't be the default..?  Surely the folks running on ZFS and running
with FPWs disabled would be able to manage to enable it if they
wished to and we could avoid entirely the question of if this has a
negative impact on the more common cases.

Guess I'm just not a fan of pushing out a change that will impact
everyone by default, in a possibly negative way (or positive, though
that doesn't seem terribly likely, but who knows), without actually
measuring what that impact will look like in those more common cases.
Showing that it's a great win when you're on ZFS or running with FPWs
disabled is good and the expected best case, but we should be
considering the worst case too when it comes to performance
improvements.

Anyhow, ultimately I don't know that there's much more to discuss on
this thread with regard to this particular topic, at least.  As I said
before, if everyone else is on board and not worried about it then so be
it; I feel that at least the concern that I raised has been heard.

Thanks,

Stephen

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

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

iQIcBAEBCgAGBQJgUngCAAoJEO1sijiDR2RVH6kP/R5aQqNQo42uGG8n8a2Rs/er
qmhJdPPrhdDHd5Iwgu4dQAk2dmltXoh10sAtjOqEYOhyYZ6KHHgE03z/SW03lHh8
qGvN1oF5RI6SRveZf8BkdIdUeVXc1wdD3M6U0xBd/RxCQ+dpFodfk2t68SgLw9L0
J5pY4GDYZ8ogv+bmY5o2KbkqsQfHHTwgRpB7dLgP58qqsrKsenv0drw21qm+l89R
rbJcjal+qh9LCJMDGlCLj/H8QXx1HfmikAy3Pnf5Vqa+MYgOmMa79EMjdI+pz2x2
imjjcvbaAGb8a55mOmFoVT9OmmYQUvdXJj1jezGxTiHSc+zy4rytJLSHDLaa4sEr
Qfy7pmwxZqdYTN8Dkj7WsUMYe939MlQhSLARiH+DREGW/I9EKhZ7zVmtyataWpMk
jNARjhm2uZMbcmtNTOBhVq6eNP52xty/tegUBpvtBAnanwEGkRapOyEI+kSTdzf2
IxQWI0/ltjVfL54p+uKVM6iz9TXjMA0iO8vuvMtWwz7/IMs+guPKZVLFJP5mEhs0
Jh4mRbzNxPLUKR7x6HFpy5fZghe1nejsT2e26PSFOrVXA7Am9XgmGRmQvzmCHbuR
ZLT5D4tkLOaHUQNmcLrAjM+jidyNqCOLeZBWsTKc+ePzp43+EhaK/T/OePNjXGQ0
xZZ03DflwhiVSsNnDLr2
=NYEG
-----END PGP SIGNATURE-----

--Ww0II/mGvpRbaaI0--