Fix SxactGlobalXmin tracking.

Commit bb16aba50 broke the code that maintains SxactGlobalXmin.  It
could get stuck when a well-timed READ ONLY transaction runs.  If
SxactGlobalXmin stops advancing, transactions on the
FinishedSerializableTransactions queue are never cleaned up, so
resources are effectively leaked.  Revert that hunk of the commit.

Also revert another similar hunk that was probably harmless, but
unnecessary and unjustified, relating to the DOOMED flag in case of
RO_SAFE early release.

Author: Thomas Munro
Reported-by: Tom Lane
Discussion: https://postgr.es/m/16170.1557251214%40sss.pgh.pa.us

diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 4e4d04bae37b5abec48a8ce6f1c89b63db044f7f..7f607b2bf8c2b969a01b8a039b9faab29af414a1 100644 (file)
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -3438,8 +3438,7 @@ ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe)
                 * cleanup. This means it should not be considered when calculating
                 * SxactGlobalXmin.
                 */
-               if (!isReadOnlySafe)
-                       MySerializableXact->flags |= SXACT_FLAG_DOOMED;
+               MySerializableXact->flags |= SXACT_FLAG_DOOMED;
                MySerializableXact->flags |= SXACT_FLAG_ROLLED_BACK;
 
                /*
@@ -3635,8 +3634,7 @@ ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe)
         * was launched.
         */
        needToClear = false;
-       if (!isReadOnlySafe &&
-               TransactionIdEquals(MySerializableXact->xmin, PredXact->SxactGlobalXmin))
+       if (TransactionIdEquals(MySerializableXact->xmin, PredXact->SxactGlobalXmin))
        {
                Assert(PredXact->SxactGlobalXminCount > 0);
                if (--(PredXact->SxactGlobalXminCount) == 0)