Hi, On 2018-06-11 10:15:39 +0100, Simon Riggs wrote: > diff --git a/src/backend/storage/ipc/procarray.c > b/src/backend/storage/ipc/procarray.c > index 9db184f8fe..c280744fdd 100644 > --- a/src/backend/storage/ipc/procarray.c > +++ b/src/backend/storage/ipc/procarray.c > @@ -1995,10 +1995,6 @@ GetRunningTransactionData(void) > volatile PGXACT *pgxact = &allPgXact[pgprocno]; > TransactionId xid; > > - /* Ignore procs running LAZY VACUUM */ > - if (pgxact->vacuumFlags & PROC_IN_VACUUM) > - continue; > - > /* Fetch xid just once - see GetNewTransactionId */ > xid = pgxact->xid; > > @@ -2009,13 +2005,21 @@ GetRunningTransactionData(void) > if (!TransactionIdIsValid(xid)) > continue; > > - xids[count++] = xid; > - > + /* > + * Be careful not to exclude any xids from calculating the > values of > + * oldestRunningXid and suboverflowed. > + */ > if (TransactionIdPrecedes(xid, oldestRunningXid)) > oldestRunningXid = xid; > > if (pgxact->overflowed) > suboverflowed = true; > + > + /* Ignore procs running LAZY VACUUM */ > + if (pgxact->vacuumFlags & PROC_IN_VACUUM) > + continue; > + > + xids[count++] = xid;
I don't think this is a good idea. We shouldn't continue down the path of having running xacts not actually running xacts, but rather go back to including everything. The case presented in the thread didn't actually do what it claimed originally, and there's a fair amount of potential for the excluded xids to cause problems down the line. Especially not when the fixes should be backpatched. I think the earlier patch should be reverted, and then the AEL lock release problem should be fixed separately. Greetings, Andres Freund
