Date: Mon, 31 Jan 2022 15:20:11 +0900 (JST)
Message-Id: <20220131.152011.148738317202375552.horikyota.ntt@gmail.com>
To: dilipbalaut@gmail.com
Cc: rjuju123@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Error "initial slot snapshot too large" in create replication
 slot
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <CAFiTN-vb_Mv+rRVDWtagvdkow-eWaT_xsGLSDiPfzL41UKiDwQ@mail.gmail.com>
	<20220131.143158.896330709668373625.horikyota.ntt@gmail.com>
References: <CAFiTN-ui+vtbUq_-3+SzT4KU_=iAJz7_-en+_tnPz-p34h76mQ@mail.gmail.com>
	<20220112103855.ib4455yyebaxkuwp@jrouhaud>
	<CAFiTN-vb_Mv+rRVDWtagvdkow-eWaT_xsGLSDiPfzL41UKiDwQ@mail.gmail.com>
User-Agent: Mew version 6.8 on Emacs 26.1
Mime-Version: 1.0
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Mon_Jan_31_15_20_11_2022_830)--"
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20220131.152011.148738317202375552.horikyota.ntt%40gmail.com>
Precedence: bulk

----Next_Part(Mon_Jan_31_15_20_11_2022_830)--
Content-Type: Text/Plain; charset=iso-8859-7
Content-Transfer-Encoding: base64

QXQgTW9uLCAxNyBKYW4gMjAyMiAwOToyNzoxNCArMDUzMCwgRGlsaXAgS3VtYXIgPGRpbGlwYmFs
YXV0QGdtYWlsLmNvbT4gd3JvdGUgaW4gDQo+IE9uIFdlZCwgSmFuIDEyLCAyMDIyIGF0IDQ6MDkg
UE0gSnVsaWVuIFJvdWhhdWQgPHJqdWp1MTIzQGdtYWlsLmNvbT4gd3JvdGU6DQo+ID4gVGhlIGNm
Ym90IHJlcG9ydHMgdGhhdCB0aGlzIHBhdGNoIGRvZXNuJ3QgY29tcGlsZToNCj4gPiBodHRwczov
L2NpcnJ1cy1jaS5jb20vdGFzay81NjQyMDAwMDczNDkwNDMyP2xvZ3M9YnVpbGQNCj4gPg0KPiA+
IFswMzowMToyNC40NzddIHNuYXBidWlsZC5jOiBJbiBmdW5jdGlvbiChU25hcEJ1aWxkSW5pdGlh
bFNuYXBzaG90ojoNCj4gPiBbMDM6MDE6MjQuNDc3XSBzbmFwYnVpbGQuYzo1ODc6MjogZXJyb3I6
IKFuZXdzdWJ4Y250oiB1bmRlY2xhcmVkIChmaXJzdA0KPiA+IHVzZSBpbiB0aGlzIGZ1bmN0aW9u
KTsgZGlkIHlvdSBtZWFuIKFuZXdzdWJ4aXCiPw0KPiA+IFswMzowMToyNC40NzddICAgNTg3IHwg
IG5ld3N1YnhjbnQgPSAwOw0KPiA+IFswMzowMToyNC40NzddICAgICAgIHwgIF5+fn5+fn5+fn4N
Cj4gPiBbMDM6MDE6MjQuNDc3XSAgICAgICB8ICBuZXdzdWJ4aXANCj4gPiBbMDM6MDE6MjQuNDc3
XSBzbmFwYnVpbGQuYzo1ODc6Mjogbm90ZTogZWFjaCB1bmRlY2xhcmVkIGlkZW50aWZpZXIgaXMN
Cj4gPiByZXBvcnRlZCBvbmx5IG9uY2UgZm9yIGVhY2ggZnVuY3Rpb24gaXQgYXBwZWFycyBpbg0K
PiA+IFswMzowMToyNC40NzddIHNuYXBidWlsZC5jOjUzNTo4OiB3YXJuaW5nOiB1bnVzZWQgdmFy
aWFibGUgoW1heHhpZGNudKINCj4gPiBbLVd1bnVzZWQtdmFyaWFibGVdDQo+ID4gWzAzOjAxOjI0
LjQ3N10gICA1MzUgfCAgaW50ICAgbWF4eGlkY250Ow0KPiA+IFswMzowMToyNC40NzddICAgICAg
IHwgICAgICAgIF5+fn5+fn5+fg0KPiA+DQo+ID4gQ291bGQgeW91IHNlbmQgYSBuZXcgdmVyc2lv
bj8gIEluIHRoZSBtZWFudGltZSBJIHdpbGwgc3dpdGNoIHRoZSBwYXRjaCB0bw0KPiA+IFdhaXRp
bmcgb24gQXV0aG9yLg0KPiA+DQo+IA0KPiBUaGFua3MgZm9yIG5vdGlmeWluZywgSSB3aWxsIHdv
cmsgb24gdGhpcyBhbmQgc2VuZCB0aGUgdXBkYXRlIHBhdGNoIHNvb24uDQoNCm1lPiBNbW0uIFRo
ZSBzaXplIG9mIHRoZSBhcnJheSBjYW5ub3QgYmUgbGFyZ2VyIHRoYW4gdGhlIG51bWJlcnMgdGhl
DQptZT4gKkNvbm50KCkgZnVuY3Rpb25zIHJldHVybi4gIFRodXMgd2UgY2Fubm90IGF0dGFjaCB0
aGUgb3ZlcnNpemVkIGFycmF5DQptZT4gdG8gLT5zdWJ4aXAuICAoSSBkb24ndCByZWNhbGwgY2xl
YXJseSBidXQgdGhhdCB3b3VsZCBsZWFkIHRvIGFzc2VydGlvbg0KbWU+IGZhaWx1cmUgc29tZXdo
ZXJlLi4pDQoNClRoZW4sIEkgZml4ZWQgdGhlIHYzIGVycm9yIGFuZCBwb3N0IHY0Lg0KDQpUbyBy
ZWNhcDoNCg0KU25hcEJVaWxkSW5pdGlhbFNuYXBzaG90IHRyaWVzIHRvIHN0b3JlIFhJRFMgb2Yg
Ym90aCB0b3AgYW5kIHN1Yg0KdHJhbnNhY3Rpb25zIGludG8gc25hcHNob3QtPnhpcCBhcnJheSBi
dXQgdGhlIGFycmF5IGlzIGVhc2lseQ0Kb3ZlcmZsb3dlZCBhbmQgQ1JFQVRFX1JFUExJQ0FUT0lO
X1NMT1QgY29tbWFuZCBlbmRzIHdpdGggYW4gZXJyb3IuDQoNClRvIGZpeCB0aGlzLCB0aGlzIHBh
dGNoIGlzIGRvaW5nIHRoZSBmb2xsb3dpbmcgdGhpbmdzLg0KDQotIFVzZSBzdWJ4aXAgYXJyYXkg
aW5zdGVhZCBvZiB4aXAgYXJyYXkgdG8gYWxsb3cgdXMgaGF2ZSBsYXJnZXIgYXJyYXkNCiAgZm9y
IHhpZHMuICBTbyB0aGUgc25hcHNob3QgaXMgbWFya2VkIGFzIHRha2VuRHVyaW5nUmVjb3Zlcnks
IHdoaWNoDQogIGlzIGEga2luZCBvZiBhYnVzZSBidXQgbGFyZ2VseSByZWR1Y2VzIHRoZSBjaGFu
Y2Ugb2YgZ2V0dGluZw0KICAiaW5pdGlhbCBzbG90IHNuYXBzaG90IHRvbyBsYXJnZSIgZXJyb3Iu
DQoNCi0gU3RpbGwgaWYgc3VieGlwIGlzIG92ZXJmbG93ZWQsIHJldHJ5IHdpdGggZXhjbHVkaW5n
IHN1YnRyYW5zYWN0aW9ucw0KICB0aGVuIHNldCBzdWJvdmVyZmxvd2VkLiAgVGhpcyBjYXVzZXMg
WGlkSW5NVkNDU25hcHNob3QgKGZpbmFsbHkpDQogIHNjYW5zIG92ZXIgc3VieGlwIGFycmF5IGZv
ciB0YXJnZXR0ZWQgdG9wLWxldmVsIHhpZC4NCg0KV2UgY291bGQgdGFrZSBhbm90aGVyIHdheTog
bWFrZSBhICF0YWtlbkR1cmluZ1JlY292ZXJ5IHNuYXBzaG90IGJ5DQp1c2luZyB4aXAgaW5zdGVh
ZCBvZiBzdWJ4aXAuIEl0IGlzIGNsZWFuZXIgYnV0IGl0IGhhcyBmYXIgbGFyZ2VyDQpjaGFuY2Ug
b2YgbmVlZGluZyB0byByZXRyeS4NCg0KKHJlbmFtZWQgdGhlIHBhdGNoIHNpbmNlIGl0IHJlcHJl
c2VudGVkIGEgcGFydCBvZiB0aGUgcGF0Y2gpDQoNCnJlZ2FyZHMuDQoNCi0tIA0KS3lvdGFybyBI
b3JpZ3VjaGkNCk5UVCBPcGVuIFNvdXJjZSBTb2Z0d2FyZSBDZW50ZXINCg==

----Next_Part(Mon_Jan_31_15_20_11_2022_830)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v4-0001-Avoid-an-error-while-creating-logical-replication.patch"

From 1e3ec40d70c2e7f8482632333001fe52527ef17a Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Mon, 31 Jan 2022 15:03:33 +0900
Subject: [PATCH v4] Avoid an error while 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 easily fails with "initial slot snapshot too large" error.

Instead, create a "takenDuringRecovery" snapshot, which stores all
XIDs in subxip array. This alone largely reduces the chance of getting
the error. But to eliminate the chance of the error to occur, allow to
create an overflowed snapshot by adding to reorder buffer a function
to tell whether an XID is a top-level or not.

Author: Dilip Kumar and Kyotaro Horiguchi
---
 .../replication/logical/reorderbuffer.c       | 20 +++++++
 src/backend/replication/logical/snapbuild.c   | 54 ++++++++++++++-----
 src/include/replication/reorderbuffer.h       |  1 +
 3 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 19b2ba2000..429e6296ab 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -3326,6 +3326,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 83fca8a77d..21f30fa1d3 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			newsubxcnt;
+	bool		overflowed = false;
 
 	Assert(!FirstSnapshotSet);
 	Assert(XactIsoLevel == XACT_REPEATABLE_READ);
@@ -568,9 +569,12 @@ 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
+	 * below and GetSnapshotData for details.
+	 */
+	newsubxip = (TransactionId *)
+		palloc(sizeof(TransactionId) * GetMaxSnapshotSubxidCount());
 
 	/*
 	 * snapbuild.c builds transactions in an "inverted" manner, which means it
@@ -578,6 +582,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 +598,28 @@ SnapBuildInitialSnapshot(SnapBuild *builder)
 
 		if (test == NULL)
 		{
-			if (newxcnt >= GetMaxSnapshotXidCount())
-				ereport(ERROR,
-						(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
-						 errmsg("initial slot snapshot too large")));
+			/*
+			 * If subtransactions fit the subxid array, we fill the array and
+			 * call it a day. Otherwise we store only top-level transactions
+			 * into the same subxip array.
+			 */
+			if (!overflowed ||
+				!ReorderBufferXidIsKnownSubXact(builder->reorder, xid))
+			{
+				if (newsubxcnt >= GetMaxSnapshotSubxidCount())
+				{
+					if (overflowed)
+						ereport(ERROR,
+								(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
+								 errmsg("initial slot snapshot too large")));
 
-			newxip[newxcnt++] = xid;
+					/* retry excluding subxids */
+					overflowed = true;
+					goto retry;
+				}
+
+				newsubxip[newsubxcnt++] = xid;
+			}
 		}
 
 		TransactionIdAdvance(xid);
@@ -604,8 +627,15 @@ 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 aa0a73382f..c05cf3078c 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,
-- 
2.27.0


----Next_Part(Mon_Jan_31_15_20_11_2022_830)----