pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: snapshot scalability: cache snapshots using a xact completion co
9+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2020-08-20 c62a0a49f3 Revert "Make vacuum a bit more verbose to debug BF failure."
2020-08-18 07f32fcd23 Fix race condition in snapshot caching when 2PC is used.
* pgsql: snapshot scalability: cache snapshots using a xact completion co
@ 2020-08-18 04:30 Andres Freund <andres@anarazel.de>
2020-08-18 05:02 ` Re: pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
2020-08-18 05:18 ` Re: pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
0 siblings, 2 replies; 9+ messages in thread
From: Andres Freund @ 2020-08-18 04:30 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
snapshot scalability: cache snapshots using a xact completion counter.
Previous commits made it faster/more scalable to compute snapshots. But not
building a snapshot is still faster. Now that GetSnapshotData() does not
maintain RecentGlobal* anymore, that is actually not too hard:
This commit introduces xactCompletionCount, which tracks the number of
top-level transactions with xids (i.e. which may have modified the database)
that completed in some form since the start of the server.
We can avoid rebuilding the snapshot's contents whenever the current
xactCompletionCount is the same as it was when the snapshot was
originally built. Currently this check happens while holding
ProcArrayLock. While it's likely possible to perform the check without
acquiring ProcArrayLock, it seems better to do that separately /
later, some careful analysis is required. Even with the lock this is a
significant win on its own.
On a smaller two socket machine this gains another ~1.03x, on a larger
machine the effect is roughly double (earlier patch version tested
though). If we were able to safely avoid the lock there'd be another
significant gain on top of that.
Author: Andres Freund <andres@anarazel.de>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Reviewed-By: Thomas Munro <thomas.munro@gmail.com>
Reviewed-By: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/20200301083601.ews6hz5dduc3w2se@alap3.anarazel.de
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/623a9ba79bbdd11c5eccb30b8bd5c446130e521c
Modified Files
--------------
src/backend/replication/logical/snapbuild.c | 1 +
src/backend/storage/ipc/procarray.c | 125 +++++++++++++++++++++++-----
src/backend/utils/time/snapmgr.c | 4 +
src/include/access/transam.h | 9 ++
src/include/utils/snapshot.h | 7 ++
5 files changed, 126 insertions(+), 20 deletions(-)
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: snapshot scalability: cache snapshots using a xact completion co
2020-08-18 04:30 pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
@ 2020-08-18 05:02 ` Andres Freund <andres@anarazel.de>
1 sibling, 0 replies; 9+ messages in thread
From: Andres Freund @ 2020-08-18 05:02 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
Hi,
On 2020-08-18 00:55:22 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > snapshot scalability: cache snapshots using a xact completion counter.
>
> buildfarm doesn't like this a bit ...
Yea, looking already. Unless that turns out to be incredibly bad luck
and only the first three animals failed (there's a few passes after), or
unless I find the issue in the next 30min or so, I'll revert.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: snapshot scalability: cache snapshots using a xact completion co
2020-08-18 04:30 pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
@ 2020-08-18 05:18 ` Andres Freund <andres@anarazel.de>
2020-08-18 20:28 ` Re: pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
1 sibling, 1 reply; 9+ messages in thread
From: Andres Freund @ 2020-08-18 05:18 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
On 2020-08-18 13:52:46 +0900, Michael Paquier wrote:
> On Tue, Aug 18, 2020 at 04:30:21AM +0000, Andres Freund wrote:
> spurfowl and more animals are telling us that this commit has broken
> 2PC:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=spurfowl&dt=2020-08-18%2004%3A31%3A11
It looks like it's a bit more subtle than outright breaking 2PC. We're
now at 3 out of 18 BF members having failed. I locally ran also quite a
few loops of the normal regression tests without finding an issue.
I'd written to Tom that I was planning to revert unless the number of
failures were lower than initially indicated. But that actually seems to
have come to pass (the failures are quicker to report because they don't
run the subsequent tests, of course). I'd like to let the failures
accumulate a bit longer, say until tomorrow Midday if I haven't figured
it out by then. With the hope of finding some detail to help pinpoint
the issue.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: snapshot scalability: cache snapshots using a xact completion co
2020-08-18 04:30 pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
2020-08-18 05:18 ` Re: pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
@ 2020-08-18 20:28 ` Andres Freund <andres@anarazel.de>
2020-08-18 23:45 ` Re: pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 9+ messages in thread
From: Andres Freund @ 2020-08-18 20:28 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Michael Paquier <michael@paquier.xyz>; pgsql-committers@lists.postgresql.org
Hi,
On 2020-08-18 01:21:17 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I'd written to Tom that I was planning to revert unless the number of
> > failures were lower than initially indicated. But that actually seems to
> > have come to pass (the failures are quicker to report because they don't
> > run the subsequent tests, of course). I'd like to let the failures
> > accumulate a bit longer, say until tomorrow Midday if I haven't figured
> > it out by then. With the hope of finding some detail to help pinpoint
> > the issue.
>
> There's certainly no obvious pattern here, so I agree with waiting for
> more data.
FWIW, I think I have found the bug, but I'm still working to reproduce
the issue reliably enough that I can verify that the fix actually works.
The issue is basically that 2PC PREPARE is weird, WRT procarray. The
last snapshot built with GetSnapshotData() before the PREPARE doesn't
include its own transaction in ->xip[], as normal. PrepareTransaction()
removes the "normal" entry with ProcArrayClearTransaction(), which so
far doesn't increase the xact completion count. Because the xact
completion count is not increased, snapshots can be reused as long as
they're taken before the 2PC transaction is finished. That's fine for
other backends, but for the backend doing the PrepareTransaction() it's
not, because there ->xip doesn't include the own backend.
It's a bit tricky to reproduce exactly the issue the BF is occasionally
hitting, because the way ->xmax is computed *limits* the
damage. Combined with the use of SERIALIZABLE (preventing recomputation
of the data snapshot) that makes it somewhat hard to hit.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: snapshot scalability: cache snapshots using a xact completion co
2020-08-18 04:30 pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
2020-08-18 05:18 ` Re: pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
2020-08-18 20:28 ` Re: pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
@ 2020-08-18 23:45 ` Andres Freund <andres@anarazel.de>
2020-08-19 22:37 ` Re: pgsql: snapshot scalability: cache snapshots using a xact completion co Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 9+ messages in thread
From: Andres Freund @ 2020-08-18 23:45 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Michael Paquier <michael@paquier.xyz>; pgsql-committers@lists.postgresql.org
On 2020-08-18 13:28:05 -0700, Andres Freund wrote:
> Hi,
>
> On 2020-08-18 01:21:17 -0400, Tom Lane wrote:
> > Andres Freund <andres@anarazel.de> writes:
> > > I'd written to Tom that I was planning to revert unless the number of
> > > failures were lower than initially indicated. But that actually seems to
> > > have come to pass (the failures are quicker to report because they don't
> > > run the subsequent tests, of course). I'd like to let the failures
> > > accumulate a bit longer, say until tomorrow Midday if I haven't figured
> > > it out by then. With the hope of finding some detail to help pinpoint
> > > the issue.
> >
> > There's certainly no obvious pattern here, so I agree with waiting for
> > more data.
>
> FWIW, I think I have found the bug, but I'm still working to reproduce
> the issue reliably enough that I can verify that the fix actually works.
>
> The issue is basically that 2PC PREPARE is weird, WRT procarray. The
> last snapshot built with GetSnapshotData() before the PREPARE doesn't
> include its own transaction in ->xip[], as normal. PrepareTransaction()
> removes the "normal" entry with ProcArrayClearTransaction(), which so
> far doesn't increase the xact completion count. Because the xact
> completion count is not increased, snapshots can be reused as long as
> they're taken before the 2PC transaction is finished. That's fine for
> other backends, but for the backend doing the PrepareTransaction() it's
> not, because there ->xip doesn't include the own backend.
>
> It's a bit tricky to reproduce exactly the issue the BF is occasionally
> hitting, because the way ->xmax is computed *limits* the
> damage. Combined with the use of SERIALIZABLE (preventing recomputation
> of the data snapshot) that makes it somewhat hard to hit.
I pushed a fix. After a while I figured out that it's not actually that
hard to test reliably. But it does require multiple sessions
interacting, particularly another session needs to acquire and commit a
transaction id that's later than the prepared transaction's.
I think it's worth adding an isolation test. But it doesn't seem like
extending prepared-transactions.spec makes too much sense, it doesn't
fit in well. It's a lot easier to reproduce the issue without
SERIALIZABLE, for example. Generally the file seems more about
serializable than 2PC...
So unless somebody disagrees I'm gonna add a new
prepared-transactions-2.spec.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: snapshot scalability: cache snapshots using a xact completion co
2020-08-18 04:30 pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
2020-08-18 05:18 ` Re: pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
2020-08-18 20:28 ` Re: pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
2020-08-18 23:45 ` Re: pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
@ 2020-08-19 22:37 ` Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 0 replies; 9+ messages in thread
From: Alvaro Herrera @ 2020-08-19 22:37 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Michael Paquier <michael@paquier.xyz>; pgsql-committers@lists.postgresql.org
On 2020-Aug-18, Andres Freund wrote:
> So unless somebody disagrees I'm gonna add a new
> prepared-transactions-2.spec.
I think keeping things separate if they're not really related is
sensible.
I think it might be a good idea to add that test to older branches too,
even if it's just 13 -- at least temporarily.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2020-08-19 22:37 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-18 04:30 pgsql: snapshot scalability: cache snapshots using a xact completion co Andres Freund <andres@anarazel.de>
2020-08-18 05:02 ` Andres Freund <andres@anarazel.de>
2020-08-18 05:18 ` Andres Freund <andres@anarazel.de>
2020-08-18 20:28 ` Andres Freund <andres@anarazel.de>
2020-08-18 23:45 ` Andres Freund <andres@anarazel.de>
2020-08-19 22:37 ` Alvaro Herrera <alvherre@2ndquadrant.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox