From 5a2c184058c51a41b855b9e824102d1395402ffa Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 13 Nov 2014 19:06:43 +0100 Subject: [PATCH] Fix xmin/xmax horizon computation during logical decoding initialization. When building the initial historic catalog snapshot there were scenarios where snapbuild.c would use incorrect xmin/xmax values when starting from a xl_running_xacts record. The values used were always a bit suspect, but happened to be correct in the easy to test cases. Notably the values used when the the initial snapshot was computed while no other transactions were running were correct. This is likely to be the cause of the occasional buildfarm failures on animals markhor and tick; but it's quite possible to reproduce problems without CLOBBER_CACHE_ALWAYS. Backpatch to 9.4, where logical decoding was introduced. --- src/backend/replication/logical/snapbuild.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index d636ccb63b..28e6c01e29 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -28,7 +28,7 @@ * * As the percentage of transactions modifying the catalog normally is fairly * small in comparisons to ones only manipulating user data, we keep track of - * the committed catalog modifying ones inside (xmin, xmax) instead of keeping + * the committed catalog modifying ones inside [xmin, xmax) instead of keeping * track of all running transactions like it's done in a normal snapshot. Note * that we're generally only looking at transactions that have acquired an * xid. That is we keep a list of transactions between snapshot->(xmin, xmax) @@ -1250,10 +1250,11 @@ SnapBuildFindSnapshot(SnapBuild *builder, XLogRecPtr lsn, xl_running_xacts *runn /* can decode everything after this */ builder->start_decoding_at = lsn + 1; - builder->xmin = running->oldestRunningXid; - builder->xmax = running->latestCompletedXid; - TransactionIdAdvance(builder->xmax); + /* As no transactions were running xmin/xmax can be trivially set. */ + builder->xmin = running->nextXid; /* < are finished */ + builder->xmax = running->nextXid; /* >= are running */ + /* so we can safely use the faster comparisons */ Assert(TransactionIdIsNormal(builder->xmin)); Assert(TransactionIdIsNormal(builder->xmax)); @@ -1294,9 +1295,14 @@ SnapBuildFindSnapshot(SnapBuild *builder, XLogRecPtr lsn, xl_running_xacts *runn * instead of running transactions we don't need to know anything * about uncommitted subtransactions. */ - builder->xmin = running->oldestRunningXid; - builder->xmax = running->latestCompletedXid; - TransactionIdAdvance(builder->xmax); + + /* + * Start with an xmin/xmax that's correct for future, when all the + * currently running transactions have finished. We'll update both + * while waiting for the pending transactions to finish. + */ + builder->xmin = running->nextXid; /* < are finished */ + builder->xmax = running->nextXid; /* >= are running */ /* so we can safely use the faster comparisons */ Assert(TransactionIdIsNormal(builder->xmin));