v79-0004-Address-review-comments-on-tidstore.patch

From 0d0d5954efd5a08bb9a2ee22e615b8985b6afe80 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Mon, 25 Mar 2024 16:36:11 +0900
Subject: [PATCH v79 4/6] Address review comments on tidstore.

---
 src/backend/access/common/tidstore.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/src/backend/access/common/tidstore.c b/src/backend/access/common/tidstore.c
index e3e37f718b..c969da4235 100644
--- a/src/backend/access/common/tidstore.c
+++ b/src/backend/access/common/tidstore.c
@@ -111,9 +111,12 @@ static void tidstore_iter_extract_tids(TidStoreIter *iter, BlockNumber blkno,
 /*
  * Create a TidStore. The TidStore will live in the memory context that is
  * CurrentMemoryContext at the time of this call. The TID storage, backed
- * by a radix tree, will live in its child memory context, rt_context. The
- * TidStore will be limited to (approximately) max_bytes total memory
- * consumption.
+ * by a radix tree, will live in its child memory context, rt_context.
+ *
+ * max_bytes is not a limit; it's used to choose the memory block sizes of
+ * a memory context for TID storage in order for the total memory consumption
+ * not to be overshot a lot. The caller can use the max_bytes as the criteria
+ * for reporting whether it's full or not.
  */
 TidStore *
 TidStoreCreateLocal(size_t max_bytes)
@@ -158,7 +161,7 @@ TidStoreCreateShared(size_t max_bytes, int tranche_id)
 	TidStore   *ts;
 	dsa_area   *area;
 	size_t		dsa_init_size = DSA_DEFAULT_INIT_SEGMENT_SIZE;
-	size_t		dsa_max_size = DSA_MAX_SEGMENT_SIZE;;
+	size_t		dsa_max_size = DSA_MAX_SEGMENT_SIZE;
 
 	ts = palloc0(sizeof(TidStore));
 	ts->context = CurrentMemoryContext;
@@ -169,23 +172,17 @@ TidStoreCreateShared(size_t max_bytes, int tranche_id)
 
 	/*
 	 * Choose the initial and maximum DSA segment sizes to be no longer
-	 * than 1/16 and 1/8 of max_bytes, respectively. If the initial
-	 * segment size is low, we end up having many segments, which risks
-	 * exceeding the total number of segments the platform can have.
-	 * if the maximum segment size is high, there is a risk that the
-	 * total segment size overshoots the max_bytes a lot.
+	 * than 1/8 of max_bytes.
 	 */
-
-	while (16 * dsa_init_size > max_bytes)
-		dsa_init_size >>= 1;
 	while (8 * dsa_max_size > max_bytes)
 		dsa_max_size >>= 1;
 
-	if (dsa_init_size < DSA_MIN_SEGMENT_SIZE)
-		dsa_init_size = DSA_MIN_SEGMENT_SIZE;
 	if (dsa_max_size < DSA_MIN_SEGMENT_SIZE)
 		dsa_max_size = DSA_MIN_SEGMENT_SIZE;
 
+	if (dsa_init_size > dsa_max_size)
+		dsa_init_size = dsa_max_size;
+
 	area = dsa_create_ext(tranche_id, dsa_init_size, dsa_max_size);
 	ts->tree.shared = shared_ts_create(ts->rt_context, area,
 									   tranche_id);
@@ -276,9 +273,9 @@ TidStoreUnlock(TidStore *ts)
 void
 TidStoreDestroy(TidStore *ts)
 {
+	/* Destroy underlying radix tree */
 	if (TidStoreIsShared(ts))
 	{
-		/* Destroy underlying radix tree */
 		shared_ts_free(ts->tree.shared);
 
 		dsa_detach(ts->area);
-- 
2.39.3