diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 0555b02a8d..a9ad40e935 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -2370,27 +2370,34 @@ GetSnapshotData(Snapshot snapshot) * * Again, our own XIDs are not included in the snapshot. */ - if (!suboverflowed) + if (subxidStates[pgxactoff].overflowed) + suboverflowed = true; + + /* + * Include all subxids, whether or not we overflowed. This is + * important because it can reduce the number of accesses to SUBTRANS + * when we search snapshots, which is important for scalability, + * especially in the presence of both overflowed and long running xacts. + * This is true when fraction of subxids held in subxip is a large + * fraction of the total subxids in use. In the case where one or more + * transactions had more subxids in progress than the total size of + * the cache this might not be true, but we consider that case to be + * unlikely, even if it is possible. + */ { + int nsubxids = subxidStates[pgxactoff].count; - if (subxidStates[pgxactoff].overflowed) - suboverflowed = true; - else + if (nsubxids > 0) { - int nsubxids = subxidStates[pgxactoff].count; - - if (nsubxids > 0) - { - int pgprocno = pgprocnos[pgxactoff]; - PGPROC *proc = &allProcs[pgprocno]; + int pgprocno = pgprocnos[pgxactoff]; + PGPROC *proc = &allProcs[pgprocno]; - pg_read_barrier(); /* pairs with GetNewTransactionId */ + pg_read_barrier(); /* pairs with GetNewTransactionId */ - memcpy(snapshot->subxip + subcount, - (void *) proc->subxids.xids, - nsubxids * sizeof(TransactionId)); - subcount += nsubxids; - } + memcpy(snapshot->subxip + subcount, + (void *) proc->subxids.xids, + nsubxids * sizeof(TransactionId)); + subcount += nsubxids; } } } diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 5bc2a15160..42130f1fea 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -638,13 +638,9 @@ CopySnapshot(Snapshot snapshot) newsnap->xip = NULL; /* - * Setup subXID array. Don't bother to copy it if it had overflowed, - * though, because it's not used anywhere in that case. Except if it's a - * snapshot taken during recovery; all the top-level XIDs are in subxip as - * well in that case, so we mustn't lose them. + * Setup subXID array. */ - if (snapshot->subxcnt > 0 && - (!snapshot->suboverflowed || snapshot->takenDuringRecovery)) + if (snapshot->subxcnt > 0) { newsnap->subxip = (TransactionId *) ((char *) newsnap + subxipoff); memcpy(newsnap->subxip, snapshot->subxip, @@ -1238,8 +1234,10 @@ ExportSnapshot(Snapshot snapshot) snapshot->subxcnt + nchildren > GetMaxSnapshotSubxidCount()) appendStringInfoString(&buf, "sof:1\n"); else - { appendStringInfoString(&buf, "sof:0\n"); + + /* then unconditionally, since we always include all subxids */ + { appendStringInfo(&buf, "sxcnt:%d\n", snapshot->subxcnt + nchildren); for (i = 0; i < snapshot->subxcnt; i++) appendStringInfo(&buf, "sxp:%u\n", snapshot->subxip[i]); @@ -1490,7 +1488,7 @@ ImportSnapshot(const char *idstr) snapshot.suboverflowed = parseIntFromText("sof:", &filebuf, path); - if (!snapshot.suboverflowed) + /* then unconditionally, since we always include all subxids */ { snapshot.subxcnt = xcnt = parseIntFromText("sxcnt:", &filebuf, path); @@ -1504,11 +1502,6 @@ ImportSnapshot(const char *idstr) for (i = 0; i < xcnt; i++) snapshot.subxip[i] = parseXidFromText("sxp:", &filebuf, path); } - else - { - snapshot.subxcnt = 0; - snapshot.subxip = NULL; - } snapshot.takenDuringRecovery = parseIntFromText("rec:", &filebuf, path);