Use INT_MAX for wal size related gucs's max value

Started by Junwang Zhaoover 2 years ago3 messages
#1Junwang Zhao
zhjwpku@gmail.com
1 attachment(s)

The wal size related gucs use the MB unit, so we should just use
INT_MAX instead of MAX_KILOBYTES as the max value.

--
Regards
Junwang Zhao

Attachments:

0001-use-INT_MAX-for-wal-size-related-max-value.patchapplication/octet-stream; name=0001-use-INT_MAX-for-wal-size-related-max-value.patchDownload
From ee93f34e420ed84deb3ee94925067ab4f91835a3 Mon Sep 17 00:00:00 2001
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Wed, 19 Apr 2023 11:15:22 +0800
Subject: [PATCH v1] use INT_MAX for wal size related max value

For these WAL size related gucs, the config_generic part set the
GUC_UNIT_MB flag, so use INT_MAX instead of MAX_KILOBYTES for
their max value.

Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
 src/backend/utils/misc/guc_tables.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index cab3ddbe11..f6b8ca3266 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2695,7 +2695,7 @@ struct config_int ConfigureNamesInt[] =
 			GUC_UNIT_MB
 		},
 		&wal_keep_size_mb,
-		0, 0, MAX_KILOBYTES,
+		0, 0, INT_MAX,
 		NULL, NULL, NULL
 	},
 
@@ -2707,7 +2707,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&min_wal_size_mb,
 		DEFAULT_MIN_WAL_SEGS * (DEFAULT_XLOG_SEG_SIZE / (1024 * 1024)),
-		2, MAX_KILOBYTES,
+		2, INT_MAX,
 		NULL, NULL, NULL
 	},
 
@@ -2719,7 +2719,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&max_wal_size_mb,
 		DEFAULT_MAX_WAL_SEGS * (DEFAULT_XLOG_SEG_SIZE / (1024 * 1024)),
-		2, MAX_KILOBYTES,
+		2, INT_MAX,
 		NULL, assign_max_wal_size, NULL
 	},
 
@@ -2834,7 +2834,7 @@ struct config_int ConfigureNamesInt[] =
 			GUC_UNIT_MB
 		},
 		&max_slot_wal_keep_size_mb,
-		-1, -1, MAX_KILOBYTES,
+		-1, -1, INT_MAX,
 		NULL, NULL, NULL
 	},
 
-- 
2.33.0

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Junwang Zhao (#1)
Re: Use INT_MAX for wal size related gucs's max value

Junwang Zhao <zhjwpku@gmail.com> writes:

The wal size related gucs use the MB unit, so we should just use
INT_MAX instead of MAX_KILOBYTES as the max value.

The point of MAX_KILOBYTES is to avoid overflow when the value
is multiplied by 1kB. It does seem like that might not be
appropriate for these values, but that doesn't mean that we can
blithely go to INT_MAX. Have you chased down how they are used?

regards, tom lane

#3Junwang Zhao
zhjwpku@gmail.com
In reply to: Tom Lane (#2)
Re: Use INT_MAX for wal size related gucs's max value

These gucs are always used with ConvertToXSegs, to calculate the count of
wal segments(see the following code snip), and wal_segment_size can be
configured by initdb as a value of a power of 2 between 1 and 1024 (megabytes),
so I think INT_MAX should be safe here.

/*
* Convert values of GUCs measured in megabytes to equiv. segment count.
* Rounds down.
*/
#define ConvertToXSegs(x, segsize) XLogMBVarToSegs((x), (segsize))

/*
* Convert values of GUCs measured in megabytes to equiv. segment count.
* Rounds down.
*/
#define XLogMBVarToSegs(mbvar, wal_segsz_bytes) \
((mbvar) / ((wal_segsz_bytes) / (1024 * 1024)))

On Wed, Apr 19, 2023 at 11:33 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Junwang Zhao <zhjwpku@gmail.com> writes:

The wal size related gucs use the MB unit, so we should just use
INT_MAX instead of MAX_KILOBYTES as the max value.

The point of MAX_KILOBYTES is to avoid overflow when the value
is multiplied by 1kB. It does seem like that might not be
appropriate for these values, but that doesn't mean that we can
blithely go to INT_MAX. Have you chased down how they are used?

regards, tom lane

--
Regards
Junwang Zhao