On Tue, Oct 19, 2021 at 2:25 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Tue, Oct 12, 2021 at 11:30 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > On Tue, Oct 12, 2021 at 10:35 AM Kyotaro Horiguchi > > <horikyota....@gmail.com> wrote: > > > > > > At Tue, 12 Oct 2021 13:59:59 +0900 (JST), Kyotaro Horiguchi > > > <horikyota....@gmail.com> wrote in > > > > So I came up with the attached version. > > > > > > Sorry, it was losing a piece of change. > > > > Yeah, at a high level this is on the idea I have in mind, I will do a > > detailed review in a day or two. Thanks for working on this. > > While doing the detailed review, I think there are a couple of > problems with the patch, the main problem of storing all the xid in > the snap->subxip is that once we mark the snapshot overflown then the > XidInMVCCSnapshot, will not search the subxip array, instead it will > fetch the topXid and search in the snap->xip array.
I missed that you are marking the snapshot as takenDuringRecovery so your fix looks fine. Another issue is > that the total xids could be GetMaxSnapshotSubxidCount() > +GetMaxSnapshotXidCount(). > I have fixed this, the updated patch is attached. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
From b6ca7a05b05a9244bf400d14b75dfcd1f99c76cd Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota....@gmail.com> Date: Tue, 12 Oct 2021 13:53:27 +0900 Subject: [PATCH v3] Allow overflowed snapshot when creating logical replication slot Snapshot can hold top XIDs up to the number of server processes but SnapBuildInitialSnapshot tries to store all top-level and sub XIDs to there and fails for certain circumstances. Instead, create a "takenDuringRecovery" snapshot instead, which stores all XIDs in subxip array. Addition to that, allow to create an overflowed snapshot by adding to reorder buffer a function to tell whether an XID is a top-level or not. --- src/backend/replication/logical/reorderbuffer.c | 20 +++++++++ src/backend/replication/logical/snapbuild.c | 57 +++++++++++++++++++------ src/include/replication/reorderbuffer.h | 1 + 3 files changed, 66 insertions(+), 12 deletions(-) diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 46e6660..4e452cc 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -3327,6 +3327,26 @@ ReorderBufferXidHasBaseSnapshot(ReorderBuffer *rb, TransactionId xid) /* + * ReorderBufferXidIsKnownSubXact + * Returns true if the xid is a known subtransaction. + */ +bool +ReorderBufferXidIsKnownSubXact(ReorderBuffer *rb, TransactionId xid) +{ + ReorderBufferTXN *txn; + + txn = ReorderBufferTXNByXid(rb, xid, false, + NULL, InvalidXLogRecPtr, false); + + /* a known subtxn? */ + if (txn && rbtxn_is_known_subxact(txn)) + return true; + + return false; +} + + +/* * --------------------------------------- * Disk serialization support * --------------------------------------- diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index dbdc172..9fd28d4 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -531,8 +531,9 @@ SnapBuildInitialSnapshot(SnapBuild *builder) { Snapshot snap; TransactionId xid; - TransactionId *newxip; - int newxcnt = 0; + TransactionId *newsubxip; + int maxxidcnt; + bool overflowed = false; Assert(!FirstSnapshotSet); Assert(XactIsoLevel == XACT_REPEATABLE_READ); @@ -568,9 +569,13 @@ SnapBuildInitialSnapshot(SnapBuild *builder) MyProc->xmin = snap->xmin; - /* allocate in transaction context */ - newxip = (TransactionId *) - palloc(sizeof(TransactionId) * GetMaxSnapshotXidCount()); + /* + * Allocate in transaction context. We use only subxip to store xids. See + * GetSnapshotData for details. + */ + newsubxip = (TransactionId *) palloc(sizeof(TransactionId) * + (GetMaxSnapshotXidCount() + + GetMaxSnapshotSubxidCount())); /* * snapbuild.c builds transactions in an "inverted" manner, which means it @@ -578,6 +583,9 @@ SnapBuildInitialSnapshot(SnapBuild *builder) * classical snapshot by marking all non-committed transactions as * in-progress. This can be expensive. */ +retry: + newsubxcnt = 0; + for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);) { void *test; @@ -591,12 +599,29 @@ SnapBuildInitialSnapshot(SnapBuild *builder) if (test == NULL) { - if (newxcnt >= GetMaxSnapshotXidCount()) - ereport(ERROR, - (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), - errmsg("initial slot snapshot too large"))); + bool add = true; - newxip[newxcnt++] = xid; + /* exlude subxids when subxip is overflowed */ + if (overflowed && + ReorderBufferXidIsKnownSubXact(builder->reorder, xid)) + add = false; + + if (add) + { + if (newsubxcnt >= GetMaxSnapshotSubxidCount()) + { + if (overflowed) + ereport(ERROR, + (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), + errmsg("initial slot snapshot too large"))); + + /* retry excluding subxids */ + overflowed = true; + goto retry; + } + + newsubxip[newsubxcnt++] = xid; + } } TransactionIdAdvance(xid); @@ -604,8 +629,16 @@ SnapBuildInitialSnapshot(SnapBuild *builder) /* adjust remaining snapshot fields as needed */ snap->snapshot_type = SNAPSHOT_MVCC; - snap->xcnt = newxcnt; - snap->xip = newxip; + snap->xcnt = 0; + snap->subxcnt = newsubxcnt; + snap->subxip = newsubxip; + snap->suboverflowed = overflowed; + + /* + * Although this snapshot is taken actually not during recovery, all XIDs + * are stored in subxip even if it is not overflowed. + */ + snap->takenDuringRecovery = true; return snap; } diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h index 5b40ff7..e5fa105 100644 --- a/src/include/replication/reorderbuffer.h +++ b/src/include/replication/reorderbuffer.h @@ -669,6 +669,7 @@ void ReorderBufferXidSetCatalogChanges(ReorderBuffer *, TransactionId xid, XLog bool ReorderBufferXidHasCatalogChanges(ReorderBuffer *, TransactionId xid); bool ReorderBufferXidHasBaseSnapshot(ReorderBuffer *, TransactionId xid); +bool ReorderBufferXidIsKnownSubXact(ReorderBuffer *rb, TransactionId xid); bool ReorderBufferRememberPrepareInfo(ReorderBuffer *rb, TransactionId xid, XLogRecPtr prepare_lsn, XLogRecPtr end_lsn, TimestampTz prepare_time, -- 1.8.3.1