public inbox for [email protected]  

help / color / mirror / Atom feed

Re: [PATCH] Refactor SLRU to always use long file names
9+ messages / 4 participants
[nested] [flat]

* Re: [PATCH] Refactor SLRU to always use long file names
@ 2025-01-15 12:42 Aleksander Alekseev <[email protected]>
  2025-02-25 14:18 ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Aleksander Alekseev @ 2025-01-15 12:42 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>; +Cc: Michael Paquier <[email protected]>

Hi,

> Here is an updated patch. The steps to test it manually are as follows.
>
> Compile and install PostgreSQL from the REL_17_STABLE branch:
>
> [...]
>
> As always, your feedback and suggestions are most welcomed.

Rebased.

--
Best regards,
Aleksander Alekseev


Attachments:

  [application/octet-stream] v3-0001-Always-use-long-SLRU-segment-file-names.patch (14.9K, 2-v3-0001-Always-use-long-SLRU-segment-file-names.patch)
  download | inline diff:
From 28e1dc9564256f2befe991dc9e89390f0f0f5c7c Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v3] Always use long SLRU segment file names

PG17 introduced long SLRU segment file names (commit 4ed8f0913bfd). We used
short or long file names depending on SlruCtl->long_segment_names. This commit
refactors SLRU to always use long file names in order to simplify the code.

Aleksander Alekseev, reviewed by Michael Paquier
Discussion: https://postgr.es/m/CAJ7c6TOy7fUW9MuNeOWor3cSFnQg9tgz=mjXHDb94GORtM_Eyg@mail.gmail.com

(!!!) bump catversion and change the corresponding TODO FIXME line in pg_upgrade.h
---
 src/backend/access/transam/clog.c           |  2 +-
 src/backend/access/transam/commit_ts.c      |  3 +-
 src/backend/access/transam/multixact.c      |  6 +-
 src/backend/access/transam/slru.c           | 73 ++++----------------
 src/backend/access/transam/subtrans.c       |  2 +-
 src/backend/commands/async.c                |  2 +-
 src/backend/storage/lmgr/predicate.c        |  2 +-
 src/bin/pg_upgrade/pg_upgrade.c             | 74 +++++++++++++++++++++
 src/bin/pg_upgrade/pg_upgrade.h             |  6 ++
 src/bin/pg_verifybackup/t/003_corruption.pl |  2 +-
 src/include/access/slru.h                   | 10 +--
 src/test/modules/test_slru/test_slru.c      |  8 +--
 12 files changed, 104 insertions(+), 86 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 0d556c00b8c..7a238efc227 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -810,7 +810,7 @@ CLOGShmemInit(void)
 	XactCtl->PagePrecedes = CLOGPagePrecedes;
 	SimpleLruInit(XactCtl, "transaction", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
 				  "pg_xact", LWTRANCHE_XACT_BUFFER,
-				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG, false);
+				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG);
 	SlruPagePrecedesUnitTests(XactCtl, CLOG_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 95049acd0b5..99252cd9b87 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -556,8 +556,7 @@ CommitTsShmemInit(void)
 	SimpleLruInit(CommitTsCtl, "commit_timestamp", CommitTsShmemBuffers(), 0,
 				  "pg_commit_ts", LWTRANCHE_COMMITTS_BUFFER,
 				  LWTRANCHE_COMMITTS_SLRU,
-				  SYNC_HANDLER_COMMIT_TS,
-				  false);
+				  SYNC_HANDLER_COMMIT_TS);
 	SlruPagePrecedesUnitTests(CommitTsCtl, COMMIT_TS_XACTS_PER_PAGE);
 
 	commitTsShared = ShmemInitStruct("CommitTs shared",
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 27ccdf9500f..2cc64289054 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1974,15 +1974,13 @@ MultiXactShmemInit(void)
 				  "multixact_offset", multixact_offset_buffers, 0,
 				  "pg_multixact/offsets", LWTRANCHE_MULTIXACTOFFSET_BUFFER,
 				  LWTRANCHE_MULTIXACTOFFSET_SLRU,
-				  SYNC_HANDLER_MULTIXACT_OFFSET,
-				  false);
+				  SYNC_HANDLER_MULTIXACT_OFFSET);
 	SlruPagePrecedesUnitTests(MultiXactOffsetCtl, MULTIXACT_OFFSETS_PER_PAGE);
 	SimpleLruInit(MultiXactMemberCtl,
 				  "multixact_member", multixact_member_buffers, 0,
 				  "pg_multixact/members", LWTRANCHE_MULTIXACTMEMBER_BUFFER,
 				  LWTRANCHE_MULTIXACTMEMBER_SLRU,
-				  SYNC_HANDLER_MULTIXACT_MEMBER,
-				  false);
+				  SYNC_HANDLER_MULTIXACT_MEMBER);
 	/* doesn't call SimpleLruTruncate() or meet criteria for unit tests */
 
 	/* Initialize our shared state struct */
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 9ce628e62a5..cc078d2e6ce 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -77,42 +77,22 @@
  *
  * "path" should point to a buffer at least MAXPGPATH characters long.
  *
- * If ctl->long_segment_names is true, segno can be in the range [0, 2^60-1].
- * The resulting file name is made of 15 characters, e.g. dir/123456789ABCDEF.
- *
- * If ctl->long_segment_names is false, segno can be in the range [0, 2^24-1].
- * The resulting file name is made of 4 to 6 characters, as of:
- *
- *  dir/1234   for [0, 2^16-1]
- *  dir/12345  for [2^16, 2^20-1]
- *  dir/123456 for [2^20, 2^24-1]
+ * segno can be in the range [0, 2^60-1]. The resulting file name is made
+ * of 15 characters, e.g. dir/123456789ABCDEF.
  */
 static inline int
 SlruFileName(SlruCtl ctl, char *path, int64 segno)
 {
-	if (ctl->long_segment_names)
-	{
-		/*
-		 * We could use 16 characters here but the disadvantage would be that
-		 * the SLRU segments will be hard to distinguish from WAL segments.
-		 *
-		 * For this reason we use 15 characters. It is enough but also means
-		 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%015llX", ctl->Dir,
-						(long long) segno);
-	}
-	else
-	{
-		/*
-		 * Despite the fact that %04X format string is used up to 24 bit
-		 * integers are allowed. See SlruCorrectSegmentFilenameLength()
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir,
-						(unsigned int) segno);
-	}
+	/*
+	 * We could use 16 characters here but the disadvantage would be that
+	 * the SLRU segments will be hard to distinguish from WAL segments.
+	 *
+	 * For this reason we use 15 characters. It is enough but also means
+	 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
+	 */
+	Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
+	return snprintf(path, MAXPGPATH, "%s/%015llX", ctl->Dir,
+					(long long) segno);
 }
 
 /*
@@ -251,7 +231,7 @@ SimpleLruAutotuneBuffers(int divisor, int max)
 void
 SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 			  const char *subdir, int buffer_tranche_id, int bank_tranche_id,
-			  SyncRequestHandler sync_handler, bool long_segment_names)
+			  SyncRequestHandler sync_handler)
 {
 	SlruShared	shared;
 	bool		found;
@@ -342,7 +322,6 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 	 */
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
-	ctl->long_segment_names = long_segment_names;
 	ctl->nbanks = nbanks;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
@@ -1748,30 +1727,6 @@ SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int64 segpage, void *data)
 	return false;				/* keep going */
 }
 
-/*
- * An internal function used by SlruScanDirectory().
- *
- * Returns true if a file with a name of a given length may be a correct
- * SLRU segment.
- */
-static inline bool
-SlruCorrectSegmentFilenameLength(SlruCtl ctl, size_t len)
-{
-	if (ctl->long_segment_names)
-		return (len == 15);		/* see SlruFileName() */
-	else
-
-		/*
-		 * Commit 638cf09e76d allowed 5-character lengths. Later commit
-		 * 73c986adde5 allowed 6-character length.
-		 *
-		 * Note: There is an ongoing plan to migrate all SLRUs to 64-bit page
-		 * numbers, and the corresponding 15-character file names, which may
-		 * eventually deprecate the support for 4, 5, and 6-character names.
-		 */
-		return (len == 4 || len == 5 || len == 6);
-}
-
 /*
  * Scan the SimpleLru directory and apply a callback to each file found in it.
  *
@@ -1803,7 +1758,7 @@ SlruScanDirectory(SlruCtl ctl, SlruScanCallback callback, void *data)
 
 		len = strlen(clde->d_name);
 
-		if (SlruCorrectSegmentFilenameLength(ctl, len) &&
+		if ((len == 15) &&
 			strspn(clde->d_name, "0123456789ABCDEF") == len)
 		{
 			segno = strtoi64(clde->d_name, NULL, 16);
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index 15153618fad..58a5ef657ea 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -243,7 +243,7 @@ SUBTRANSShmemInit(void)
 	SubTransCtl->PagePrecedes = SubTransPagePrecedes;
 	SimpleLruInit(SubTransCtl, "subtransaction", SUBTRANSShmemBuffers(), 0,
 				  "pg_subtrans", LWTRANCHE_SUBTRANS_BUFFER,
-				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE, false);
+				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE);
 	SlruPagePrecedesUnitTests(SubTransCtl, SUBTRANS_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 4bd37d5beb5..373b0357fad 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -537,7 +537,7 @@ AsyncShmemInit(void)
 	NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
 	SimpleLruInit(NotifyCtl, "notify", notify_buffers, 0,
 				  "pg_notify", LWTRANCHE_NOTIFY_BUFFER, LWTRANCHE_NOTIFY_SLRU,
-				  SYNC_HANDLER_NONE, true);
+				  SYNC_HANDLER_NONE);
 
 	if (!found)
 	{
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 5b21a053981..bc83e8e859d 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -814,7 +814,7 @@ SerialInit(void)
 	SimpleLruInit(SerialSlruCtl, "serializable",
 				  serializable_buffers, 0, "pg_serial",
 				  LWTRANCHE_SERIAL_BUFFER, LWTRANCHE_SERIAL_SLRU,
-				  SYNC_HANDLER_NONE, false);
+				  SYNC_HANDLER_NONE);
 #ifdef USE_ASSERT_CHECKING
 	SerialPagePrecedesLogicallyUnitTests();
 #endif
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 36c7f3879d5..6d3dcc63d2b 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -38,6 +38,7 @@
 
 #include "postgres_fe.h"
 
+#include <dirent.h>
 #include <time.h>
 
 #include "catalog/pg_class_d.h"
@@ -59,6 +60,8 @@ static void prepare_new_cluster(void);
 static void prepare_new_globals(void);
 static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
+static void check_slru_segment_filenames(void);
+static void rename_slru_segments(const char *dirname);
 static void set_frozenxids(bool minmxid_only);
 static void make_outputdirs(char *pgdata);
 static void setup(char *argv0);
@@ -154,6 +157,7 @@ main(int argc, char **argv)
 	 */
 
 	copy_xact_xlog_xid();
+	check_slru_segment_filenames();
 
 	/* New now using xids of the old system */
 
@@ -806,6 +810,76 @@ copy_xact_xlog_xid(void)
 	check_ok();
 }
 
+static void
+rename_slru_segments(const char* dirname)
+{
+	DIR		   *dir;
+	struct dirent *de;
+	int 		len;
+	int64 		segno;
+	char		dir_path[MAXPGPATH];
+	char		old_path[MAXPGPATH];
+	char		new_path[MAXPGPATH];
+
+	prep_status("Renaming SLRU segments in %s", dirname);
+	snprintf(dir_path, sizeof(dir_path), "%s/%s", new_cluster.pgdata, dirname);
+
+	dir = opendir(dir_path);
+	if (dir == NULL)
+		pg_fatal("could not open directory \"%s\": %m", dir_path);
+
+	while (errno = 0, (de = readdir(dir)) != NULL)
+	{
+		/*
+		 * ignore '.', '..' and everything else that doesn't look
+		 * like an SLRU segment with a short file name
+		 */
+
+		len = strlen(de->d_name);
+		if(len != 4 && len != 5 && len != 6)
+			continue;
+
+		if(strspn(de->d_name, "0123456789ABCDEF") != len)
+			continue;
+
+		segno = strtoi64(de->d_name, NULL, 16);
+		snprintf(new_path, MAXPGPATH, "%s/%015llX", dir_path,
+					(long long) segno);
+		snprintf(old_path, MAXPGPATH, "%s/%s", dir_path, de->d_name);
+
+		if (pg_mv_file(old_path, new_path) != 0)
+			pg_fatal("could not rename file \"%s\" to \"%s\": %m",
+					 old_path, new_path);
+	}
+
+	if (errno)
+		pg_fatal("could not read directory \"%s\": %m", dir_path);
+
+	if (closedir(dir))
+		pg_fatal("could not close directory \"%s\": %m", dir_path);
+
+	check_ok();
+}
+
+static void
+check_slru_segment_filenames(void)
+{
+	int i;
+	static const char* dirs[] = {
+		"pg_xact",
+		"pg_commit_ts",
+		"pg_multixact/offsets",
+		"pg_multixact/members",
+		"pg_subtrans",
+		"pg_serial",
+	};
+
+	if(old_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER)
+		return;
+
+	for (i = 0; i < sizeof(dirs)/sizeof(dirs[0]); i++)
+		rename_slru_segments(dirs[i]);
+}
 
 /*
  *	set_frozenxids()
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 0cdd675e4f1..a839f19e310 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -125,6 +125,12 @@ extern char *output_files[];
  */
 #define JSONB_FORMAT_CHANGE_CAT_VER 201409291
 
+/*
+ * change of SLRU segment filenames length in 18.0
+ * TODO FIXME CHANGE TO THE ACTUAL VALUE BEFORE COMMITTING
+ */
+#define SLRU_SEG_FILENAMES_CHANGE_CAT_VER 202412201
+
 
 /*
  * Each relation is represented by a relinfo structure.
diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl
index 1111b09637d..9d1dbb93d72 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -237,7 +237,7 @@ sub mutilate_extra_tablespace_file
 sub mutilate_missing_file
 {
 	my ($backup_path) = @_;
-	my $pathname = "$backup_path/pg_xact/0000";
+	my $pathname = "$backup_path/pg_xact/000000000000000";
 	unlink($pathname) || die "$pathname: $!";
 	return;
 }
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index e142800aab2..ecf2ca79692 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -131,13 +131,6 @@ typedef struct SlruCtlData
 	/* Number of banks in this SLRU. */
 	uint16		nbanks;
 
-	/*
-	 * If true, use long segment file names.  Otherwise, use short file names.
-	 *
-	 * For details about the file name format, see SlruFileName().
-	 */
-	bool		long_segment_names;
-
 	/*
 	 * Which sync handler function to use when handing sync requests over to
 	 * the checkpointer.  SYNC_HANDLER_NONE to disable fsync (eg pg_notify).
@@ -184,8 +177,7 @@ extern Size SimpleLruShmemSize(int nslots, int nlsns);
 extern int	SimpleLruAutotuneBuffers(int divisor, int max);
 extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 						  const char *subdir, int buffer_tranche_id,
-						  int bank_tranche_id, SyncRequestHandler sync_handler,
-						  bool long_segment_names);
+						  int bank_tranche_id, SyncRequestHandler sync_handler);
 extern int	SimpleLruZeroPage(SlruCtl ctl, int64 pageno);
 extern int	SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
 							  TransactionId xid);
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 3ea5ceb8552..cbd5173015a 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -213,11 +213,6 @@ test_slru_page_precedes_logically(int64 page1, int64 page2)
 static void
 test_slru_shmem_startup(void)
 {
-	/*
-	 * Short segments names are well tested elsewhere so in this test we are
-	 * focusing on long names.
-	 */
-	const bool	long_segment_names = true;
 	const char	slru_dir_name[] = "pg_test_slru";
 	int			test_tranche_id;
 	int			test_buffer_tranche_id;
@@ -241,8 +236,7 @@ test_slru_shmem_startup(void)
 	TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
 	SimpleLruInit(TestSlruCtl, "TestSLRU",
 				  NUM_TEST_BUFFERS, 0, slru_dir_name,
-				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE,
-				  long_segment_names);
+				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE);
 }
 
 void
-- 
2.47.1



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: [PATCH] Refactor SLRU to always use long file names
  2025-01-15 12:42 Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
@ 2025-02-25 14:18 ` Aleksander Alekseev <[email protected]>
  2025-03-28 20:27   ` Re: [PATCH] Refactor SLRU to always use long file names Rustam ALLAKOV <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Aleksander Alekseev @ 2025-02-25 14:18 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>; +Cc: Michael Paquier <[email protected]>

Hi,

> > Here is an updated patch. The steps to test it manually are as follows.
> >
> > Compile and install PostgreSQL from the REL_17_STABLE branch:
> >
> > [...]
> >
> > As always, your feedback and suggestions are most welcomed.
>
> Rebased.

Rebased.

--
Best regards,
Aleksander Alekseev


Attachments:

  [application/x-patch] v4-0001-Always-use-long-SLRU-segment-file-names.patch (14.9K, 2-v4-0001-Always-use-long-SLRU-segment-file-names.patch)
  download | inline diff:
From 43e7cdccf1aa96e6522a41518bf0437dda23cc4d Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v4] Always use long SLRU segment file names

PG17 introduced long SLRU segment file names (commit 4ed8f0913bfd). We used
short or long file names depending on SlruCtl->long_segment_names. This commit
refactors SLRU to always use long file names in order to simplify the code.

Aleksander Alekseev, reviewed by Michael Paquier
Discussion: https://postgr.es/m/CAJ7c6TOy7fUW9MuNeOWor3cSFnQg9tgz=mjXHDb94GORtM_Eyg@mail.gmail.com

(!!!) bump catversion and change the corresponding TODO FIXME line in pg_upgrade.h
---
 src/backend/access/transam/clog.c           |  2 +-
 src/backend/access/transam/commit_ts.c      |  3 +-
 src/backend/access/transam/multixact.c      |  6 +-
 src/backend/access/transam/slru.c           | 73 ++++----------------
 src/backend/access/transam/subtrans.c       |  2 +-
 src/backend/commands/async.c                |  2 +-
 src/backend/storage/lmgr/predicate.c        |  2 +-
 src/bin/pg_upgrade/pg_upgrade.c             | 74 +++++++++++++++++++++
 src/bin/pg_upgrade/pg_upgrade.h             |  6 ++
 src/bin/pg_verifybackup/t/003_corruption.pl |  2 +-
 src/include/access/slru.h                   | 10 +--
 src/test/modules/test_slru/test_slru.c      |  8 +--
 12 files changed, 104 insertions(+), 86 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 48f10bec91e..f130403ac4b 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -810,7 +810,7 @@ CLOGShmemInit(void)
 	XactCtl->PagePrecedes = CLOGPagePrecedes;
 	SimpleLruInit(XactCtl, "transaction", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
 				  "pg_xact", LWTRANCHE_XACT_BUFFER,
-				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG, false);
+				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG);
 	SlruPagePrecedesUnitTests(XactCtl, CLOG_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 113fae1437a..59535526823 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -556,8 +556,7 @@ CommitTsShmemInit(void)
 	SimpleLruInit(CommitTsCtl, "commit_timestamp", CommitTsShmemBuffers(), 0,
 				  "pg_commit_ts", LWTRANCHE_COMMITTS_BUFFER,
 				  LWTRANCHE_COMMITTS_SLRU,
-				  SYNC_HANDLER_COMMIT_TS,
-				  false);
+				  SYNC_HANDLER_COMMIT_TS);
 	SlruPagePrecedesUnitTests(CommitTsCtl, COMMIT_TS_XACTS_PER_PAGE);
 
 	commitTsShared = ShmemInitStruct("CommitTs shared",
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index c1e2c42e1bb..b3b11c1a0ad 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1974,15 +1974,13 @@ MultiXactShmemInit(void)
 				  "multixact_offset", multixact_offset_buffers, 0,
 				  "pg_multixact/offsets", LWTRANCHE_MULTIXACTOFFSET_BUFFER,
 				  LWTRANCHE_MULTIXACTOFFSET_SLRU,
-				  SYNC_HANDLER_MULTIXACT_OFFSET,
-				  false);
+				  SYNC_HANDLER_MULTIXACT_OFFSET);
 	SlruPagePrecedesUnitTests(MultiXactOffsetCtl, MULTIXACT_OFFSETS_PER_PAGE);
 	SimpleLruInit(MultiXactMemberCtl,
 				  "multixact_member", multixact_member_buffers, 0,
 				  "pg_multixact/members", LWTRANCHE_MULTIXACTMEMBER_BUFFER,
 				  LWTRANCHE_MULTIXACTMEMBER_SLRU,
-				  SYNC_HANDLER_MULTIXACT_MEMBER,
-				  false);
+				  SYNC_HANDLER_MULTIXACT_MEMBER);
 	/* doesn't call SimpleLruTruncate() or meet criteria for unit tests */
 
 	/* Initialize our shared state struct */
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 9ce628e62a5..cc078d2e6ce 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -77,42 +77,22 @@
  *
  * "path" should point to a buffer at least MAXPGPATH characters long.
  *
- * If ctl->long_segment_names is true, segno can be in the range [0, 2^60-1].
- * The resulting file name is made of 15 characters, e.g. dir/123456789ABCDEF.
- *
- * If ctl->long_segment_names is false, segno can be in the range [0, 2^24-1].
- * The resulting file name is made of 4 to 6 characters, as of:
- *
- *  dir/1234   for [0, 2^16-1]
- *  dir/12345  for [2^16, 2^20-1]
- *  dir/123456 for [2^20, 2^24-1]
+ * segno can be in the range [0, 2^60-1]. The resulting file name is made
+ * of 15 characters, e.g. dir/123456789ABCDEF.
  */
 static inline int
 SlruFileName(SlruCtl ctl, char *path, int64 segno)
 {
-	if (ctl->long_segment_names)
-	{
-		/*
-		 * We could use 16 characters here but the disadvantage would be that
-		 * the SLRU segments will be hard to distinguish from WAL segments.
-		 *
-		 * For this reason we use 15 characters. It is enough but also means
-		 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%015llX", ctl->Dir,
-						(long long) segno);
-	}
-	else
-	{
-		/*
-		 * Despite the fact that %04X format string is used up to 24 bit
-		 * integers are allowed. See SlruCorrectSegmentFilenameLength()
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir,
-						(unsigned int) segno);
-	}
+	/*
+	 * We could use 16 characters here but the disadvantage would be that
+	 * the SLRU segments will be hard to distinguish from WAL segments.
+	 *
+	 * For this reason we use 15 characters. It is enough but also means
+	 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
+	 */
+	Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
+	return snprintf(path, MAXPGPATH, "%s/%015llX", ctl->Dir,
+					(long long) segno);
 }
 
 /*
@@ -251,7 +231,7 @@ SimpleLruAutotuneBuffers(int divisor, int max)
 void
 SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 			  const char *subdir, int buffer_tranche_id, int bank_tranche_id,
-			  SyncRequestHandler sync_handler, bool long_segment_names)
+			  SyncRequestHandler sync_handler)
 {
 	SlruShared	shared;
 	bool		found;
@@ -342,7 +322,6 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 	 */
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
-	ctl->long_segment_names = long_segment_names;
 	ctl->nbanks = nbanks;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
@@ -1748,30 +1727,6 @@ SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int64 segpage, void *data)
 	return false;				/* keep going */
 }
 
-/*
- * An internal function used by SlruScanDirectory().
- *
- * Returns true if a file with a name of a given length may be a correct
- * SLRU segment.
- */
-static inline bool
-SlruCorrectSegmentFilenameLength(SlruCtl ctl, size_t len)
-{
-	if (ctl->long_segment_names)
-		return (len == 15);		/* see SlruFileName() */
-	else
-
-		/*
-		 * Commit 638cf09e76d allowed 5-character lengths. Later commit
-		 * 73c986adde5 allowed 6-character length.
-		 *
-		 * Note: There is an ongoing plan to migrate all SLRUs to 64-bit page
-		 * numbers, and the corresponding 15-character file names, which may
-		 * eventually deprecate the support for 4, 5, and 6-character names.
-		 */
-		return (len == 4 || len == 5 || len == 6);
-}
-
 /*
  * Scan the SimpleLru directory and apply a callback to each file found in it.
  *
@@ -1803,7 +1758,7 @@ SlruScanDirectory(SlruCtl ctl, SlruScanCallback callback, void *data)
 
 		len = strlen(clde->d_name);
 
-		if (SlruCorrectSegmentFilenameLength(ctl, len) &&
+		if ((len == 15) &&
 			strspn(clde->d_name, "0123456789ABCDEF") == len)
 		{
 			segno = strtoi64(clde->d_name, NULL, 16);
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index 15153618fad..58a5ef657ea 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -243,7 +243,7 @@ SUBTRANSShmemInit(void)
 	SubTransCtl->PagePrecedes = SubTransPagePrecedes;
 	SimpleLruInit(SubTransCtl, "subtransaction", SUBTRANSShmemBuffers(), 0,
 				  "pg_subtrans", LWTRANCHE_SUBTRANS_BUFFER,
-				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE, false);
+				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE);
 	SlruPagePrecedesUnitTests(SubTransCtl, SUBTRANS_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 4bd37d5beb5..373b0357fad 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -537,7 +537,7 @@ AsyncShmemInit(void)
 	NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
 	SimpleLruInit(NotifyCtl, "notify", notify_buffers, 0,
 				  "pg_notify", LWTRANCHE_NOTIFY_BUFFER, LWTRANCHE_NOTIFY_SLRU,
-				  SYNC_HANDLER_NONE, true);
+				  SYNC_HANDLER_NONE);
 
 	if (!found)
 	{
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 5b21a053981..bc83e8e859d 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -814,7 +814,7 @@ SerialInit(void)
 	SimpleLruInit(SerialSlruCtl, "serializable",
 				  serializable_buffers, 0, "pg_serial",
 				  LWTRANCHE_SERIAL_BUFFER, LWTRANCHE_SERIAL_SLRU,
-				  SYNC_HANDLER_NONE, false);
+				  SYNC_HANDLER_NONE);
 #ifdef USE_ASSERT_CHECKING
 	SerialPagePrecedesLogicallyUnitTests();
 #endif
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index d95c491fb57..8b10908e1f2 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -38,6 +38,7 @@
 
 #include "postgres_fe.h"
 
+#include <dirent.h>
 #include <time.h>
 
 #include "catalog/pg_class_d.h"
@@ -60,6 +61,8 @@ static void prepare_new_cluster(void);
 static void prepare_new_globals(void);
 static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
+static void check_slru_segment_filenames(void);
+static void rename_slru_segments(const char *dirname);
 static void set_frozenxids(bool minmxid_only);
 static void make_outputdirs(char *pgdata);
 static void setup(char *argv0);
@@ -156,6 +159,7 @@ main(int argc, char **argv)
 
 	copy_xact_xlog_xid();
 	set_new_cluster_char_signedness();
+	check_slru_segment_filenames();
 
 	/* New now using xids of the old system */
 
@@ -840,6 +844,76 @@ copy_xact_xlog_xid(void)
 	check_ok();
 }
 
+static void
+rename_slru_segments(const char* dirname)
+{
+	DIR		   *dir;
+	struct dirent *de;
+	int 		len;
+	int64 		segno;
+	char		dir_path[MAXPGPATH];
+	char		old_path[MAXPGPATH];
+	char		new_path[MAXPGPATH];
+
+	prep_status("Renaming SLRU segments in %s", dirname);
+	snprintf(dir_path, sizeof(dir_path), "%s/%s", new_cluster.pgdata, dirname);
+
+	dir = opendir(dir_path);
+	if (dir == NULL)
+		pg_fatal("could not open directory \"%s\": %m", dir_path);
+
+	while (errno = 0, (de = readdir(dir)) != NULL)
+	{
+		/*
+		 * ignore '.', '..' and everything else that doesn't look
+		 * like an SLRU segment with a short file name
+		 */
+
+		len = strlen(de->d_name);
+		if(len != 4 && len != 5 && len != 6)
+			continue;
+
+		if(strspn(de->d_name, "0123456789ABCDEF") != len)
+			continue;
+
+		segno = strtoi64(de->d_name, NULL, 16);
+		snprintf(new_path, MAXPGPATH, "%s/%015llX", dir_path,
+					(long long) segno);
+		snprintf(old_path, MAXPGPATH, "%s/%s", dir_path, de->d_name);
+
+		if (pg_mv_file(old_path, new_path) != 0)
+			pg_fatal("could not rename file \"%s\" to \"%s\": %m",
+					 old_path, new_path);
+	}
+
+	if (errno)
+		pg_fatal("could not read directory \"%s\": %m", dir_path);
+
+	if (closedir(dir))
+		pg_fatal("could not close directory \"%s\": %m", dir_path);
+
+	check_ok();
+}
+
+static void
+check_slru_segment_filenames(void)
+{
+	int i;
+	static const char* dirs[] = {
+		"pg_xact",
+		"pg_commit_ts",
+		"pg_multixact/offsets",
+		"pg_multixact/members",
+		"pg_subtrans",
+		"pg_serial",
+	};
+
+	if(old_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER)
+		return;
+
+	for (i = 0; i < sizeof(dirs)/sizeof(dirs[0]); i++)
+		rename_slru_segments(dirs[i]);
+}
 
 /*
  *	set_frozenxids()
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index f4e375d27c7..0340a20a391 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -131,6 +131,12 @@ extern char *output_files[];
  */
 #define DEFAULT_CHAR_SIGNEDNESS_CAT_VER 202502212
 
+/*
+ * change of SLRU segment filenames length in 18.0
+ * TODO FIXME CHANGE TO THE ACTUAL VALUE BEFORE COMMITTING
+ */
+#define SLRU_SEG_FILENAMES_CHANGE_CAT_VER 202412201
+
 /*
  * Each relation is represented by a relinfo structure.
  */
diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl
index 8ef7f8a4e7a..367212af529 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -246,7 +246,7 @@ sub mutilate_extra_tablespace_file
 sub mutilate_missing_file
 {
 	my ($backup_path) = @_;
-	my $pathname = "$backup_path/pg_xact/0000";
+	my $pathname = "$backup_path/pg_xact/000000000000000";
 	unlink($pathname) || die "$pathname: $!";
 	return;
 }
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index e142800aab2..ecf2ca79692 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -131,13 +131,6 @@ typedef struct SlruCtlData
 	/* Number of banks in this SLRU. */
 	uint16		nbanks;
 
-	/*
-	 * If true, use long segment file names.  Otherwise, use short file names.
-	 *
-	 * For details about the file name format, see SlruFileName().
-	 */
-	bool		long_segment_names;
-
 	/*
 	 * Which sync handler function to use when handing sync requests over to
 	 * the checkpointer.  SYNC_HANDLER_NONE to disable fsync (eg pg_notify).
@@ -184,8 +177,7 @@ extern Size SimpleLruShmemSize(int nslots, int nlsns);
 extern int	SimpleLruAutotuneBuffers(int divisor, int max);
 extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 						  const char *subdir, int buffer_tranche_id,
-						  int bank_tranche_id, SyncRequestHandler sync_handler,
-						  bool long_segment_names);
+						  int bank_tranche_id, SyncRequestHandler sync_handler);
 extern int	SimpleLruZeroPage(SlruCtl ctl, int64 pageno);
 extern int	SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
 							  TransactionId xid);
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 3ea5ceb8552..cbd5173015a 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -213,11 +213,6 @@ test_slru_page_precedes_logically(int64 page1, int64 page2)
 static void
 test_slru_shmem_startup(void)
 {
-	/*
-	 * Short segments names are well tested elsewhere so in this test we are
-	 * focusing on long names.
-	 */
-	const bool	long_segment_names = true;
 	const char	slru_dir_name[] = "pg_test_slru";
 	int			test_tranche_id;
 	int			test_buffer_tranche_id;
@@ -241,8 +236,7 @@ test_slru_shmem_startup(void)
 	TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
 	SimpleLruInit(TestSlruCtl, "TestSLRU",
 				  NUM_TEST_BUFFERS, 0, slru_dir_name,
-				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE,
-				  long_segment_names);
+				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE);
 }
 
 void
-- 
2.48.1



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: [PATCH] Refactor SLRU to always use long file names
  2025-01-15 12:42 Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-02-25 14:18 ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
@ 2025-03-28 20:27   ` Rustam ALLAKOV <[email protected]>
  2025-04-03 14:25     ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Rustam ALLAKOV @ 2025-03-28 20:27 UTC (permalink / raw)
  To: [email protected]; +Cc: Aleksander Alekseev <[email protected]>

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

Hi Aleksander, 
I have reviewed your patch. It looks good to me.

Kindest regards.
Rustam Allakov

The new status of this patch is: Ready for Committer


^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: [PATCH] Refactor SLRU to always use long file names
  2025-01-15 12:42 Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-02-25 14:18 ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-03-28 20:27   ` Re: [PATCH] Refactor SLRU to always use long file names Rustam ALLAKOV <[email protected]>
@ 2025-04-03 14:25     ` Aleksander Alekseev <[email protected]>
  2025-08-13 11:54       ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Aleksander Alekseev @ 2025-04-03 14:25 UTC (permalink / raw)
  To: [email protected]; +Cc: Rustam ALLAKOV <[email protected]>; Michael Paquier <[email protected]>

Hi Rustam,

> Hi Aleksander,
> I have reviewed your patch. It looks good to me.
>
> Kindest regards.
> Rustam Allakov
>
> The new status of this patch is: Ready for Committer

Thanks for testing!

Here is the rebased patch.

-- 
Best regards,
Aleksander Alekseev


Attachments:

  [application/octet-stream] v5-0001-Always-use-long-SLRU-segment-file-names.patch (14.9K, 2-v5-0001-Always-use-long-SLRU-segment-file-names.patch)
  download | inline diff:
From b0b98310672de9181e7006fd28a6ed2e12e11851 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v5] Always use long SLRU segment file names

PG17 introduced long SLRU segment file names (commit 4ed8f0913bfd). We used
short or long file names depending on SlruCtl->long_segment_names. This commit
refactors SLRU to always use long file names in order to simplify the code.

Aleksander Alekseev, reviewed by Michael Paquier, Rustam Allakov
Discussion: https://postgr.es/m/CAJ7c6TOy7fUW9MuNeOWor3cSFnQg9tgz=mjXHDb94GORtM_Eyg@mail.gmail.com

(!!!) bump catversion and change the corresponding TODO FIXME line in pg_upgrade.h
---
 src/backend/access/transam/clog.c           |  2 +-
 src/backend/access/transam/commit_ts.c      |  3 +-
 src/backend/access/transam/multixact.c      |  6 +-
 src/backend/access/transam/slru.c           | 71 ++++----------------
 src/backend/access/transam/subtrans.c       |  2 +-
 src/backend/commands/async.c                |  2 +-
 src/backend/storage/lmgr/predicate.c        |  2 +-
 src/bin/pg_upgrade/pg_upgrade.c             | 74 +++++++++++++++++++++
 src/bin/pg_upgrade/pg_upgrade.h             |  6 ++
 src/bin/pg_verifybackup/t/003_corruption.pl |  2 +-
 src/include/access/slru.h                   | 10 +--
 src/test/modules/test_slru/test_slru.c      |  8 +--
 12 files changed, 103 insertions(+), 85 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 48f10bec91e..f130403ac4b 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -810,7 +810,7 @@ CLOGShmemInit(void)
 	XactCtl->PagePrecedes = CLOGPagePrecedes;
 	SimpleLruInit(XactCtl, "transaction", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
 				  "pg_xact", LWTRANCHE_XACT_BUFFER,
-				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG, false);
+				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG);
 	SlruPagePrecedesUnitTests(XactCtl, CLOG_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 113fae1437a..59535526823 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -556,8 +556,7 @@ CommitTsShmemInit(void)
 	SimpleLruInit(CommitTsCtl, "commit_timestamp", CommitTsShmemBuffers(), 0,
 				  "pg_commit_ts", LWTRANCHE_COMMITTS_BUFFER,
 				  LWTRANCHE_COMMITTS_SLRU,
-				  SYNC_HANDLER_COMMIT_TS,
-				  false);
+				  SYNC_HANDLER_COMMIT_TS);
 	SlruPagePrecedesUnitTests(CommitTsCtl, COMMIT_TS_XACTS_PER_PAGE);
 
 	commitTsShared = ShmemInitStruct("CommitTs shared",
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 9d25a7df0d3..8d8f7d28a06 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1974,15 +1974,13 @@ MultiXactShmemInit(void)
 				  "multixact_offset", multixact_offset_buffers, 0,
 				  "pg_multixact/offsets", LWTRANCHE_MULTIXACTOFFSET_BUFFER,
 				  LWTRANCHE_MULTIXACTOFFSET_SLRU,
-				  SYNC_HANDLER_MULTIXACT_OFFSET,
-				  false);
+				  SYNC_HANDLER_MULTIXACT_OFFSET);
 	SlruPagePrecedesUnitTests(MultiXactOffsetCtl, MULTIXACT_OFFSETS_PER_PAGE);
 	SimpleLruInit(MultiXactMemberCtl,
 				  "multixact_member", multixact_member_buffers, 0,
 				  "pg_multixact/members", LWTRANCHE_MULTIXACTMEMBER_BUFFER,
 				  LWTRANCHE_MULTIXACTMEMBER_SLRU,
-				  SYNC_HANDLER_MULTIXACT_MEMBER,
-				  false);
+				  SYNC_HANDLER_MULTIXACT_MEMBER);
 	/* doesn't call SimpleLruTruncate() or meet criteria for unit tests */
 
 	/* Initialize our shared state struct */
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index fe56286d9a9..752aa71fa87 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -77,41 +77,21 @@
  *
  * "path" should point to a buffer at least MAXPGPATH characters long.
  *
- * If ctl->long_segment_names is true, segno can be in the range [0, 2^60-1].
- * The resulting file name is made of 15 characters, e.g. dir/123456789ABCDEF.
- *
- * If ctl->long_segment_names is false, segno can be in the range [0, 2^24-1].
- * The resulting file name is made of 4 to 6 characters, as of:
- *
- *  dir/1234   for [0, 2^16-1]
- *  dir/12345  for [2^16, 2^20-1]
- *  dir/123456 for [2^20, 2^24-1]
+ * segno can be in the range [0, 2^60-1]. The resulting file name is made
+ * of 15 characters, e.g. dir/123456789ABCDEF.
  */
 static inline int
 SlruFileName(SlruCtl ctl, char *path, int64 segno)
 {
-	if (ctl->long_segment_names)
-	{
-		/*
-		 * We could use 16 characters here but the disadvantage would be that
-		 * the SLRU segments will be hard to distinguish from WAL segments.
-		 *
-		 * For this reason we use 15 characters. It is enough but also means
-		 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%015" PRIX64, ctl->Dir, segno);
-	}
-	else
-	{
-		/*
-		 * Despite the fact that %04X format string is used up to 24 bit
-		 * integers are allowed. See SlruCorrectSegmentFilenameLength()
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir,
-						(unsigned int) segno);
-	}
+	/*
+	 * We could use 16 characters here but the disadvantage would be that
+	 * the SLRU segments will be hard to distinguish from WAL segments.
+	 *
+	 * For this reason we use 15 characters. It is enough but also means
+	 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
+	 */
+	Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
+	return snprintf(path, MAXPGPATH, "%s/%015" PRIX64, ctl->Dir, segno);
 }
 
 /*
@@ -250,7 +230,7 @@ SimpleLruAutotuneBuffers(int divisor, int max)
 void
 SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 			  const char *subdir, int buffer_tranche_id, int bank_tranche_id,
-			  SyncRequestHandler sync_handler, bool long_segment_names)
+			  SyncRequestHandler sync_handler)
 {
 	SlruShared	shared;
 	bool		found;
@@ -341,7 +321,6 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 	 */
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
-	ctl->long_segment_names = long_segment_names;
 	ctl->nbanks = nbanks;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
@@ -1747,30 +1726,6 @@ SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int64 segpage, void *data)
 	return false;				/* keep going */
 }
 
-/*
- * An internal function used by SlruScanDirectory().
- *
- * Returns true if a file with a name of a given length may be a correct
- * SLRU segment.
- */
-static inline bool
-SlruCorrectSegmentFilenameLength(SlruCtl ctl, size_t len)
-{
-	if (ctl->long_segment_names)
-		return (len == 15);		/* see SlruFileName() */
-	else
-
-		/*
-		 * Commit 638cf09e76d allowed 5-character lengths. Later commit
-		 * 73c986adde5 allowed 6-character length.
-		 *
-		 * Note: There is an ongoing plan to migrate all SLRUs to 64-bit page
-		 * numbers, and the corresponding 15-character file names, which may
-		 * eventually deprecate the support for 4, 5, and 6-character names.
-		 */
-		return (len == 4 || len == 5 || len == 6);
-}
-
 /*
  * Scan the SimpleLru directory and apply a callback to each file found in it.
  *
@@ -1802,7 +1757,7 @@ SlruScanDirectory(SlruCtl ctl, SlruScanCallback callback, void *data)
 
 		len = strlen(clde->d_name);
 
-		if (SlruCorrectSegmentFilenameLength(ctl, len) &&
+		if ((len == 15) &&
 			strspn(clde->d_name, "0123456789ABCDEF") == len)
 		{
 			segno = strtoi64(clde->d_name, NULL, 16);
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index 15153618fad..58a5ef657ea 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -243,7 +243,7 @@ SUBTRANSShmemInit(void)
 	SubTransCtl->PagePrecedes = SubTransPagePrecedes;
 	SimpleLruInit(SubTransCtl, "subtransaction", SUBTRANSShmemBuffers(), 0,
 				  "pg_subtrans", LWTRANCHE_SUBTRANS_BUFFER,
-				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE, false);
+				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE);
 	SlruPagePrecedesUnitTests(SubTransCtl, SUBTRANS_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 4bd37d5beb5..373b0357fad 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -537,7 +537,7 @@ AsyncShmemInit(void)
 	NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
 	SimpleLruInit(NotifyCtl, "notify", notify_buffers, 0,
 				  "pg_notify", LWTRANCHE_NOTIFY_BUFFER, LWTRANCHE_NOTIFY_SLRU,
-				  SYNC_HANDLER_NONE, true);
+				  SYNC_HANDLER_NONE);
 
 	if (!found)
 	{
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index d82114ffca1..b029b12601e 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -814,7 +814,7 @@ SerialInit(void)
 	SimpleLruInit(SerialSlruCtl, "serializable",
 				  serializable_buffers, 0, "pg_serial",
 				  LWTRANCHE_SERIAL_BUFFER, LWTRANCHE_SERIAL_SLRU,
-				  SYNC_HANDLER_NONE, false);
+				  SYNC_HANDLER_NONE);
 #ifdef USE_ASSERT_CHECKING
 	SerialPagePrecedesLogicallyUnitTests();
 #endif
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 9295e46aed3..df0bb7df1aa 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -38,6 +38,7 @@
 
 #include "postgres_fe.h"
 
+#include <dirent.h>
 #include <time.h>
 
 #include "catalog/pg_class_d.h"
@@ -60,6 +61,8 @@ static void prepare_new_cluster(void);
 static void prepare_new_globals(void);
 static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
+static void check_slru_segment_filenames(void);
+static void rename_slru_segments(const char *dirname);
 static void set_frozenxids(bool minmxid_only);
 static void make_outputdirs(char *pgdata);
 static void setup(char *argv0);
@@ -156,6 +159,7 @@ main(int argc, char **argv)
 
 	copy_xact_xlog_xid();
 	set_new_cluster_char_signedness();
+	check_slru_segment_filenames();
 
 	/* New now using xids of the old system */
 
@@ -843,6 +847,76 @@ copy_xact_xlog_xid(void)
 	check_ok();
 }
 
+static void
+rename_slru_segments(const char* dirname)
+{
+	DIR		   *dir;
+	struct dirent *de;
+	int 		len;
+	int64 		segno;
+	char		dir_path[MAXPGPATH];
+	char		old_path[MAXPGPATH];
+	char		new_path[MAXPGPATH];
+
+	prep_status("Renaming SLRU segments in %s", dirname);
+	snprintf(dir_path, sizeof(dir_path), "%s/%s", new_cluster.pgdata, dirname);
+
+	dir = opendir(dir_path);
+	if (dir == NULL)
+		pg_fatal("could not open directory \"%s\": %m", dir_path);
+
+	while (errno = 0, (de = readdir(dir)) != NULL)
+	{
+		/*
+		 * ignore '.', '..' and everything else that doesn't look
+		 * like an SLRU segment with a short file name
+		 */
+
+		len = strlen(de->d_name);
+		if(len != 4 && len != 5 && len != 6)
+			continue;
+
+		if(strspn(de->d_name, "0123456789ABCDEF") != len)
+			continue;
+
+		segno = strtoi64(de->d_name, NULL, 16);
+		snprintf(new_path, MAXPGPATH, "%s/%015llX", dir_path,
+					(long long) segno);
+		snprintf(old_path, MAXPGPATH, "%s/%s", dir_path, de->d_name);
+
+		if (pg_mv_file(old_path, new_path) != 0)
+			pg_fatal("could not rename file \"%s\" to \"%s\": %m",
+					 old_path, new_path);
+	}
+
+	if (errno)
+		pg_fatal("could not read directory \"%s\": %m", dir_path);
+
+	if (closedir(dir))
+		pg_fatal("could not close directory \"%s\": %m", dir_path);
+
+	check_ok();
+}
+
+static void
+check_slru_segment_filenames(void)
+{
+	int i;
+	static const char* dirs[] = {
+		"pg_xact",
+		"pg_commit_ts",
+		"pg_multixact/offsets",
+		"pg_multixact/members",
+		"pg_subtrans",
+		"pg_serial",
+	};
+
+	if(old_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER)
+		return;
+
+	for (i = 0; i < sizeof(dirs)/sizeof(dirs[0]); i++)
+		rename_slru_segments(dirs[i]);
+}
 
 /*
  *	set_frozenxids()
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 69c965bb7d0..32c6ac49138 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -131,6 +131,12 @@ extern char *output_files[];
  */
 #define DEFAULT_CHAR_SIGNEDNESS_CAT_VER 202502212
 
+/*
+ * change of SLRU segment filenames length in 18.0
+ * TODO FIXME CHANGE TO THE ACTUAL VALUE BEFORE COMMITTING
+ */
+#define SLRU_SEG_FILENAMES_CHANGE_CAT_VER 202412201
+
 /*
  * Each relation is represented by a relinfo structure.
  */
diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl
index 84f23b8bc3d..df215378e74 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -246,7 +246,7 @@ sub mutilate_extra_tablespace_file
 sub mutilate_missing_file
 {
 	my ($backup_path) = @_;
-	my $pathname = "$backup_path/pg_xact/0000";
+	my $pathname = "$backup_path/pg_xact/000000000000000";
 	unlink($pathname) || die "$pathname: $!";
 	return;
 }
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index e142800aab2..ecf2ca79692 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -131,13 +131,6 @@ typedef struct SlruCtlData
 	/* Number of banks in this SLRU. */
 	uint16		nbanks;
 
-	/*
-	 * If true, use long segment file names.  Otherwise, use short file names.
-	 *
-	 * For details about the file name format, see SlruFileName().
-	 */
-	bool		long_segment_names;
-
 	/*
 	 * Which sync handler function to use when handing sync requests over to
 	 * the checkpointer.  SYNC_HANDLER_NONE to disable fsync (eg pg_notify).
@@ -184,8 +177,7 @@ extern Size SimpleLruShmemSize(int nslots, int nlsns);
 extern int	SimpleLruAutotuneBuffers(int divisor, int max);
 extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 						  const char *subdir, int buffer_tranche_id,
-						  int bank_tranche_id, SyncRequestHandler sync_handler,
-						  bool long_segment_names);
+						  int bank_tranche_id, SyncRequestHandler sync_handler);
 extern int	SimpleLruZeroPage(SlruCtl ctl, int64 pageno);
 extern int	SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
 							  TransactionId xid);
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 2e4492900af..171076d9209 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -213,11 +213,6 @@ test_slru_page_precedes_logically(int64 page1, int64 page2)
 static void
 test_slru_shmem_startup(void)
 {
-	/*
-	 * Short segments names are well tested elsewhere so in this test we are
-	 * focusing on long names.
-	 */
-	const bool	long_segment_names = true;
 	const char	slru_dir_name[] = "pg_test_slru";
 	int			test_tranche_id;
 	int			test_buffer_tranche_id;
@@ -241,8 +236,7 @@ test_slru_shmem_startup(void)
 	TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
 	SimpleLruInit(TestSlruCtl, "TestSLRU",
 				  NUM_TEST_BUFFERS, 0, slru_dir_name,
-				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE,
-				  long_segment_names);
+				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE);
 }
 
 void
-- 
2.49.0



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: [PATCH] Refactor SLRU to always use long file names
  2025-01-15 12:42 Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-02-25 14:18 ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-03-28 20:27   ` Re: [PATCH] Refactor SLRU to always use long file names Rustam ALLAKOV <[email protected]>
  2025-04-03 14:25     ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
@ 2025-08-13 11:54       ` Aleksander Alekseev <[email protected]>
  2026-01-06 15:18         ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Aleksander Alekseev @ 2025-08-13 11:54 UTC (permalink / raw)
  To: [email protected]; +Cc: Rustam ALLAKOV <[email protected]>; Michael Paquier <[email protected]>

Hi,

> > The new status of this patch is: Ready for Committer
>
> Thanks for testing!
>
> Here is the rebased patch.

Rebased.


Attachments:

  [text/x-patch] v6-0001-Always-use-long-SLRU-segment-file-names.patch (15.0K, 2-v6-0001-Always-use-long-SLRU-segment-file-names.patch)
  download | inline diff:
From 4d83acd61eee36784840e2d1228d0c854807bac0 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v6] Always use long SLRU segment file names

PG17 introduced long SLRU segment file names (commit 4ed8f0913bfd). We used
short or long file names depending on SlruCtl->long_segment_names. This commit
refactors SLRU to always use long file names in order to simplify the code.

Aleksander Alekseev, reviewed by Michael Paquier, Rustam Allakov
Discussion: https://postgr.es/m/CAJ7c6TOy7fUW9MuNeOWor3cSFnQg9tgz=mjXHDb94GORtM_Eyg@mail.gmail.com

(!!!) bump catversion and change the corresponding TODO FIXME line in pg_upgrade.h
---
 src/backend/access/transam/clog.c           |  2 +-
 src/backend/access/transam/commit_ts.c      |  3 +-
 src/backend/access/transam/multixact.c      |  6 +-
 src/backend/access/transam/slru.c           | 71 ++++----------------
 src/backend/access/transam/subtrans.c       |  2 +-
 src/backend/commands/async.c                |  2 +-
 src/backend/storage/lmgr/predicate.c        |  2 +-
 src/bin/pg_upgrade/pg_upgrade.c             | 74 +++++++++++++++++++++
 src/bin/pg_upgrade/pg_upgrade.h             |  6 ++
 src/bin/pg_verifybackup/t/003_corruption.pl |  2 +-
 src/include/access/slru.h                   | 10 +--
 src/test/modules/test_slru/test_slru.c      |  8 +--
 12 files changed, 103 insertions(+), 85 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index e80fbe109cf..b6603cacf82 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -808,7 +808,7 @@ CLOGShmemInit(void)
 	XactCtl->PagePrecedes = CLOGPagePrecedes;
 	SimpleLruInit(XactCtl, "transaction", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
 				  "pg_xact", LWTRANCHE_XACT_BUFFER,
-				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG, false);
+				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG);
 	SlruPagePrecedesUnitTests(XactCtl, CLOG_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 370b38e048b..7798f217e75 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -554,8 +554,7 @@ CommitTsShmemInit(void)
 	SimpleLruInit(CommitTsCtl, "commit_timestamp", CommitTsShmemBuffers(), 0,
 				  "pg_commit_ts", LWTRANCHE_COMMITTS_BUFFER,
 				  LWTRANCHE_COMMITTS_SLRU,
-				  SYNC_HANDLER_COMMIT_TS,
-				  false);
+				  SYNC_HANDLER_COMMIT_TS);
 	SlruPagePrecedesUnitTests(CommitTsCtl, COMMIT_TS_XACTS_PER_PAGE);
 
 	commitTsShared = ShmemInitStruct("CommitTs shared",
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 3cb09c3d598..d1c04948290 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1971,15 +1971,13 @@ MultiXactShmemInit(void)
 				  "multixact_offset", multixact_offset_buffers, 0,
 				  "pg_multixact/offsets", LWTRANCHE_MULTIXACTOFFSET_BUFFER,
 				  LWTRANCHE_MULTIXACTOFFSET_SLRU,
-				  SYNC_HANDLER_MULTIXACT_OFFSET,
-				  false);
+				  SYNC_HANDLER_MULTIXACT_OFFSET);
 	SlruPagePrecedesUnitTests(MultiXactOffsetCtl, MULTIXACT_OFFSETS_PER_PAGE);
 	SimpleLruInit(MultiXactMemberCtl,
 				  "multixact_member", multixact_member_buffers, 0,
 				  "pg_multixact/members", LWTRANCHE_MULTIXACTMEMBER_BUFFER,
 				  LWTRANCHE_MULTIXACTMEMBER_SLRU,
-				  SYNC_HANDLER_MULTIXACT_MEMBER,
-				  false);
+				  SYNC_HANDLER_MULTIXACT_MEMBER);
 	/* doesn't call SimpleLruTruncate() or meet criteria for unit tests */
 
 	/* Initialize our shared state struct */
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 10ec259f382..3006def1f83 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -77,41 +77,21 @@
  *
  * "path" should point to a buffer at least MAXPGPATH characters long.
  *
- * If ctl->long_segment_names is true, segno can be in the range [0, 2^60-1].
- * The resulting file name is made of 15 characters, e.g. dir/123456789ABCDEF.
- *
- * If ctl->long_segment_names is false, segno can be in the range [0, 2^24-1].
- * The resulting file name is made of 4 to 6 characters, as of:
- *
- *  dir/1234   for [0, 2^16-1]
- *  dir/12345  for [2^16, 2^20-1]
- *  dir/123456 for [2^20, 2^24-1]
+ * segno can be in the range [0, 2^60-1]. The resulting file name is made
+ * of 15 characters, e.g. dir/123456789ABCDEF.
  */
 static inline int
 SlruFileName(SlruCtl ctl, char *path, int64 segno)
 {
-	if (ctl->long_segment_names)
-	{
-		/*
-		 * We could use 16 characters here but the disadvantage would be that
-		 * the SLRU segments will be hard to distinguish from WAL segments.
-		 *
-		 * For this reason we use 15 characters. It is enough but also means
-		 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%015" PRIX64, ctl->Dir, segno);
-	}
-	else
-	{
-		/*
-		 * Despite the fact that %04X format string is used up to 24 bit
-		 * integers are allowed. See SlruCorrectSegmentFilenameLength()
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir,
-						(unsigned int) segno);
-	}
+	/*
+	 * We could use 16 characters here but the disadvantage would be that
+	 * the SLRU segments will be hard to distinguish from WAL segments.
+	 *
+	 * For this reason we use 15 characters. It is enough but also means
+	 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
+	 */
+	Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
+	return snprintf(path, MAXPGPATH, "%s/%015" PRIX64, ctl->Dir, segno);
 }
 
 /*
@@ -250,7 +230,7 @@ SimpleLruAutotuneBuffers(int divisor, int max)
 void
 SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 			  const char *subdir, int buffer_tranche_id, int bank_tranche_id,
-			  SyncRequestHandler sync_handler, bool long_segment_names)
+			  SyncRequestHandler sync_handler)
 {
 	SlruShared	shared;
 	bool		found;
@@ -341,7 +321,6 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 	 */
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
-	ctl->long_segment_names = long_segment_names;
 	ctl->nbanks = nbanks;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
@@ -1772,30 +1751,6 @@ SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int64 segpage, void *data)
 	return false;				/* keep going */
 }
 
-/*
- * An internal function used by SlruScanDirectory().
- *
- * Returns true if a file with a name of a given length may be a correct
- * SLRU segment.
- */
-static inline bool
-SlruCorrectSegmentFilenameLength(SlruCtl ctl, size_t len)
-{
-	if (ctl->long_segment_names)
-		return (len == 15);		/* see SlruFileName() */
-	else
-
-		/*
-		 * Commit 638cf09e76d allowed 5-character lengths. Later commit
-		 * 73c986adde5 allowed 6-character length.
-		 *
-		 * Note: There is an ongoing plan to migrate all SLRUs to 64-bit page
-		 * numbers, and the corresponding 15-character file names, which may
-		 * eventually deprecate the support for 4, 5, and 6-character names.
-		 */
-		return (len == 4 || len == 5 || len == 6);
-}
-
 /*
  * Scan the SimpleLru directory and apply a callback to each file found in it.
  *
@@ -1827,7 +1782,7 @@ SlruScanDirectory(SlruCtl ctl, SlruScanCallback callback, void *data)
 
 		len = strlen(clde->d_name);
 
-		if (SlruCorrectSegmentFilenameLength(ctl, len) &&
+		if ((len == 15) &&
 			strspn(clde->d_name, "0123456789ABCDEF") == len)
 		{
 			segno = strtoi64(clde->d_name, NULL, 16);
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index 09aace9e09f..517089f0666 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -242,7 +242,7 @@ SUBTRANSShmemInit(void)
 	SubTransCtl->PagePrecedes = SubTransPagePrecedes;
 	SimpleLruInit(SubTransCtl, "subtransaction", SUBTRANSShmemBuffers(), 0,
 				  "pg_subtrans", LWTRANCHE_SUBTRANS_BUFFER,
-				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE, false);
+				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE);
 	SlruPagePrecedesUnitTests(SubTransCtl, SUBTRANS_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 4bd37d5beb5..373b0357fad 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -537,7 +537,7 @@ AsyncShmemInit(void)
 	NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
 	SimpleLruInit(NotifyCtl, "notify", notify_buffers, 0,
 				  "pg_notify", LWTRANCHE_NOTIFY_BUFFER, LWTRANCHE_NOTIFY_SLRU,
-				  SYNC_HANDLER_NONE, true);
+				  SYNC_HANDLER_NONE);
 
 	if (!found)
 	{
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index c07fb588355..d6117eb72f6 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -814,7 +814,7 @@ SerialInit(void)
 	SimpleLruInit(SerialSlruCtl, "serializable",
 				  serializable_buffers, 0, "pg_serial",
 				  LWTRANCHE_SERIAL_BUFFER, LWTRANCHE_SERIAL_SLRU,
-				  SYNC_HANDLER_NONE, false);
+				  SYNC_HANDLER_NONE);
 #ifdef USE_ASSERT_CHECKING
 	SerialPagePrecedesLogicallyUnitTests();
 #endif
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index d5cd5bf0b3a..52f6a1cdfec 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -41,6 +41,7 @@
 
 #include "postgres_fe.h"
 
+#include <dirent.h>
 #include <time.h>
 
 #include "catalog/pg_class_d.h"
@@ -63,6 +64,8 @@ static void prepare_new_cluster(void);
 static void prepare_new_globals(void);
 static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
+static void check_slru_segment_filenames(void);
+static void rename_slru_segments(const char *dirname);
 static void set_frozenxids(bool minmxid_only);
 static void make_outputdirs(char *pgdata);
 static void setup(char *argv0);
@@ -161,6 +164,7 @@ main(int argc, char **argv)
 
 	copy_xact_xlog_xid();
 	set_new_cluster_char_signedness();
+	check_slru_segment_filenames();
 
 	/* New now using xids of the old system */
 
@@ -872,6 +876,76 @@ copy_xact_xlog_xid(void)
 	check_ok();
 }
 
+static void
+rename_slru_segments(const char* dirname)
+{
+	DIR		   *dir;
+	struct dirent *de;
+	int 		len;
+	int64 		segno;
+	char		dir_path[MAXPGPATH];
+	char		old_path[MAXPGPATH];
+	char		new_path[MAXPGPATH];
+
+	prep_status("Renaming SLRU segments in %s", dirname);
+	snprintf(dir_path, sizeof(dir_path), "%s/%s", new_cluster.pgdata, dirname);
+
+	dir = opendir(dir_path);
+	if (dir == NULL)
+		pg_fatal("could not open directory \"%s\": %m", dir_path);
+
+	while (errno = 0, (de = readdir(dir)) != NULL)
+	{
+		/*
+		 * ignore '.', '..' and everything else that doesn't look
+		 * like an SLRU segment with a short file name
+		 */
+
+		len = strlen(de->d_name);
+		if(len != 4 && len != 5 && len != 6)
+			continue;
+
+		if(strspn(de->d_name, "0123456789ABCDEF") != len)
+			continue;
+
+		segno = strtoi64(de->d_name, NULL, 16);
+		snprintf(new_path, MAXPGPATH, "%s/%015llX", dir_path,
+					(long long) segno);
+		snprintf(old_path, MAXPGPATH, "%s/%s", dir_path, de->d_name);
+
+		if (pg_mv_file(old_path, new_path) != 0)
+			pg_fatal("could not rename file \"%s\" to \"%s\": %m",
+					 old_path, new_path);
+	}
+
+	if (errno)
+		pg_fatal("could not read directory \"%s\": %m", dir_path);
+
+	if (closedir(dir))
+		pg_fatal("could not close directory \"%s\": %m", dir_path);
+
+	check_ok();
+}
+
+static void
+check_slru_segment_filenames(void)
+{
+	int i;
+	static const char* dirs[] = {
+		"pg_xact",
+		"pg_commit_ts",
+		"pg_multixact/offsets",
+		"pg_multixact/members",
+		"pg_subtrans",
+		"pg_serial",
+	};
+
+	if(old_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER)
+		return;
+
+	for (i = 0; i < sizeof(dirs)/sizeof(dirs[0]); i++)
+		rename_slru_segments(dirs[i]);
+}
 
 /*
  *	set_frozenxids()
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 0ef47be0dc1..73d102529c3 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -131,6 +131,12 @@ extern char *output_files[];
  */
 #define DEFAULT_CHAR_SIGNEDNESS_CAT_VER 202502212
 
+/*
+ * change of SLRU segment filenames length in 18.0
+ * TODO FIXME CHANGE TO THE ACTUAL VALUE BEFORE COMMITTING
+ */
+#define SLRU_SEG_FILENAMES_CHANGE_CAT_VER 202412201
+
 /*
  * Each relation is represented by a relinfo structure.
  */
diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl
index 1dd60f709cf..deb8c11ca9e 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -246,7 +246,7 @@ sub mutilate_extra_tablespace_file
 sub mutilate_missing_file
 {
 	my ($backup_path) = @_;
-	my $pathname = "$backup_path/pg_xact/0000";
+	my $pathname = "$backup_path/pg_xact/000000000000000";
 	unlink($pathname) || die "$pathname: $!";
 	return;
 }
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 20dbd1e0070..186d9cb1a10 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -131,13 +131,6 @@ typedef struct SlruCtlData
 	/* Number of banks in this SLRU. */
 	uint16		nbanks;
 
-	/*
-	 * If true, use long segment file names.  Otherwise, use short file names.
-	 *
-	 * For details about the file name format, see SlruFileName().
-	 */
-	bool		long_segment_names;
-
 	/*
 	 * Which sync handler function to use when handing sync requests over to
 	 * the checkpointer.  SYNC_HANDLER_NONE to disable fsync (eg pg_notify).
@@ -184,8 +177,7 @@ extern Size SimpleLruShmemSize(int nslots, int nlsns);
 extern int	SimpleLruAutotuneBuffers(int divisor, int max);
 extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 						  const char *subdir, int buffer_tranche_id,
-						  int bank_tranche_id, SyncRequestHandler sync_handler,
-						  bool long_segment_names);
+						  int bank_tranche_id, SyncRequestHandler sync_handler);
 extern int	SimpleLruZeroPage(SlruCtl ctl, int64 pageno);
 extern void SimpleLruZeroAndWritePage(SlruCtl ctl, int64 pageno);
 extern int	SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 32750930e43..483c06edfc7 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -213,11 +213,6 @@ test_slru_page_precedes_logically(int64 page1, int64 page2)
 static void
 test_slru_shmem_startup(void)
 {
-	/*
-	 * Short segments names are well tested elsewhere so in this test we are
-	 * focusing on long names.
-	 */
-	const bool	long_segment_names = true;
 	const char	slru_dir_name[] = "pg_test_slru";
 	int			test_tranche_id;
 	int			test_buffer_tranche_id;
@@ -241,8 +236,7 @@ test_slru_shmem_startup(void)
 	TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
 	SimpleLruInit(TestSlruCtl, "TestSLRU",
 				  NUM_TEST_BUFFERS, 0, slru_dir_name,
-				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE,
-				  long_segment_names);
+				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE);
 }
 
 void
-- 
2.43.0



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: [PATCH] Refactor SLRU to always use long file names
  2025-01-15 12:42 Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-02-25 14:18 ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-03-28 20:27   ` Re: [PATCH] Refactor SLRU to always use long file names Rustam ALLAKOV <[email protected]>
  2025-04-03 14:25     ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-08-13 11:54       ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
@ 2026-01-06 15:18         ` Aleksander Alekseev <[email protected]>
  2026-01-06 15:28           ` Re: [PATCH] Refactor SLRU to always use long file names Heikki Linnakangas <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Aleksander Alekseev @ 2026-01-06 15:18 UTC (permalink / raw)
  To: [email protected]; +Cc: Rustam ALLAKOV <[email protected]>; Michael Paquier <[email protected]>

Hi,

> > > The new status of this patch is: Ready for Committer
> >
> > Thanks for testing!
> >
> > Here is the rebased patch.
>
> Rebased.

Rebased.

-- 
Best regards,
Aleksander Alekseev


Attachments:

  [text/x-patch] v7-0001-Always-use-long-SLRU-segment-file-names.patch (16.3K, 2-v7-0001-Always-use-long-SLRU-segment-file-names.patch)
  download | inline diff:
From 89ee21b95411af09aa5ec546cb6a59369653ff6d Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v7] Always use long SLRU segment file names

PG17 introduced long SLRU segment file names (commit 4ed8f0913bfd). We used
short or long file names depending on SlruCtl->long_segment_names. This commit
refactors SLRU to always use long file names in order to simplify the code.

Aleksander Alekseev, reviewed by Michael Paquier, Rustam Allakov
Discussion: https://postgr.es/m/CAJ7c6TOy7fUW9MuNeOWor3cSFnQg9tgz=mjXHDb94GORtM_Eyg@mail.gmail.com

(!!!) bump catversion and change the corresponding TODO FIXME line in pg_upgrade.h
---
 src/backend/access/transam/clog.c             |  2 +-
 src/backend/access/transam/commit_ts.c        |  3 +-
 src/backend/access/transam/multixact.c        |  6 +-
 src/backend/access/transam/slru.c             | 71 ++++--------------
 src/backend/access/transam/subtrans.c         |  2 +-
 src/backend/commands/async.c                  |  2 +-
 src/backend/storage/lmgr/predicate.c          |  2 +-
 src/bin/pg_upgrade/pg_upgrade.c               | 74 +++++++++++++++++++
 src/bin/pg_upgrade/pg_upgrade.h               |  6 ++
 src/bin/pg_verifybackup/t/003_corruption.pl   |  2 +-
 src/include/access/slru.h                     | 10 +--
 .../test_slru/t/002_multixact_wraparound.pl   |  6 +-
 src/test/modules/test_slru/test_slru.c        |  8 +-
 13 files changed, 106 insertions(+), 88 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index b5c38bbb162..e46dc13fb5a 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -809,7 +809,7 @@ CLOGShmemInit(void)
 	XactCtl->PagePrecedes = CLOGPagePrecedes;
 	SimpleLruInit(XactCtl, "transaction", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
 				  "pg_xact", LWTRANCHE_XACT_BUFFER,
-				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG, false);
+				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG);
 	SlruPagePrecedesUnitTests(XactCtl, CLOG_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 082b564da8f..25774fbc1b9 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -554,8 +554,7 @@ CommitTsShmemInit(void)
 	SimpleLruInit(CommitTsCtl, "commit_timestamp", CommitTsShmemBuffers(), 0,
 				  "pg_commit_ts", LWTRANCHE_COMMITTS_BUFFER,
 				  LWTRANCHE_COMMITTS_SLRU,
-				  SYNC_HANDLER_COMMIT_TS,
-				  false);
+				  SYNC_HANDLER_COMMIT_TS);
 	SlruPagePrecedesUnitTests(CommitTsCtl, COMMIT_TS_XACTS_PER_PAGE);
 
 	commitTsShared = ShmemInitStruct("CommitTs shared",
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 3f423636b48..3482b1c16ad 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1736,15 +1736,13 @@ MultiXactShmemInit(void)
 				  "multixact_offset", multixact_offset_buffers, 0,
 				  "pg_multixact/offsets", LWTRANCHE_MULTIXACTOFFSET_BUFFER,
 				  LWTRANCHE_MULTIXACTOFFSET_SLRU,
-				  SYNC_HANDLER_MULTIXACT_OFFSET,
-				  false);
+				  SYNC_HANDLER_MULTIXACT_OFFSET);
 	SlruPagePrecedesUnitTests(MultiXactOffsetCtl, MULTIXACT_OFFSETS_PER_PAGE);
 	SimpleLruInit(MultiXactMemberCtl,
 				  "multixact_member", multixact_member_buffers, 0,
 				  "pg_multixact/members", LWTRANCHE_MULTIXACTMEMBER_BUFFER,
 				  LWTRANCHE_MULTIXACTMEMBER_SLRU,
-				  SYNC_HANDLER_MULTIXACT_MEMBER,
-				  true);
+				  SYNC_HANDLER_MULTIXACT_MEMBER);
 	/* doesn't call SimpleLruTruncate() or meet criteria for unit tests */
 
 	/* Initialize our shared state struct */
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 549c7e3e64b..18baedd6de6 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -77,41 +77,21 @@
  *
  * "path" should point to a buffer at least MAXPGPATH characters long.
  *
- * If ctl->long_segment_names is true, segno can be in the range [0, 2^60-1].
- * The resulting file name is made of 15 characters, e.g. dir/123456789ABCDEF.
- *
- * If ctl->long_segment_names is false, segno can be in the range [0, 2^24-1].
- * The resulting file name is made of 4 to 6 characters, as of:
- *
- *  dir/1234   for [0, 2^16-1]
- *  dir/12345  for [2^16, 2^20-1]
- *  dir/123456 for [2^20, 2^24-1]
+ * segno can be in the range [0, 2^60-1]. The resulting file name is made
+ * of 15 characters, e.g. dir/123456789ABCDEF.
  */
 static inline int
 SlruFileName(SlruCtl ctl, char *path, int64 segno)
 {
-	if (ctl->long_segment_names)
-	{
-		/*
-		 * We could use 16 characters here but the disadvantage would be that
-		 * the SLRU segments will be hard to distinguish from WAL segments.
-		 *
-		 * For this reason we use 15 characters. It is enough but also means
-		 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%015" PRIX64, ctl->Dir, segno);
-	}
-	else
-	{
-		/*
-		 * Despite the fact that %04X format string is used up to 24 bit
-		 * integers are allowed. See SlruCorrectSegmentFilenameLength()
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir,
-						(unsigned int) segno);
-	}
+	/*
+	 * We could use 16 characters here but the disadvantage would be that
+	 * the SLRU segments will be hard to distinguish from WAL segments.
+	 *
+	 * For this reason we use 15 characters. It is enough but also means
+	 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
+	 */
+	Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
+	return snprintf(path, MAXPGPATH, "%s/%015" PRIX64, ctl->Dir, segno);
 }
 
 /*
@@ -251,7 +231,7 @@ SimpleLruAutotuneBuffers(int divisor, int max)
 void
 SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 			  const char *subdir, int buffer_tranche_id, int bank_tranche_id,
-			  SyncRequestHandler sync_handler, bool long_segment_names)
+			  SyncRequestHandler sync_handler)
 {
 	SlruShared	shared;
 	bool		found;
@@ -342,7 +322,6 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 	 */
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
-	ctl->long_segment_names = long_segment_names;
 	ctl->nbanks = nbanks;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
@@ -1773,30 +1752,6 @@ SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int64 segpage, void *data)
 	return false;				/* keep going */
 }
 
-/*
- * An internal function used by SlruScanDirectory().
- *
- * Returns true if a file with a name of a given length may be a correct
- * SLRU segment.
- */
-static inline bool
-SlruCorrectSegmentFilenameLength(SlruCtl ctl, size_t len)
-{
-	if (ctl->long_segment_names)
-		return (len == 15);		/* see SlruFileName() */
-	else
-
-		/*
-		 * Commit 638cf09e76d allowed 5-character lengths. Later commit
-		 * 73c986adde5 allowed 6-character length.
-		 *
-		 * Note: There is an ongoing plan to migrate all SLRUs to 64-bit page
-		 * numbers, and the corresponding 15-character file names, which may
-		 * eventually deprecate the support for 4, 5, and 6-character names.
-		 */
-		return (len == 4 || len == 5 || len == 6);
-}
-
 /*
  * Scan the SimpleLru directory and apply a callback to each file found in it.
  *
@@ -1828,7 +1783,7 @@ SlruScanDirectory(SlruCtl ctl, SlruScanCallback callback, void *data)
 
 		len = strlen(clde->d_name);
 
-		if (SlruCorrectSegmentFilenameLength(ctl, len) &&
+		if ((len == 15) &&
 			strspn(clde->d_name, "0123456789ABCDEF") == len)
 		{
 			segno = strtoi64(clde->d_name, NULL, 16);
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index c0987f43f11..3536971ca7d 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -242,7 +242,7 @@ SUBTRANSShmemInit(void)
 	SubTransCtl->PagePrecedes = SubTransPagePrecedes;
 	SimpleLruInit(SubTransCtl, "subtransaction", SUBTRANSShmemBuffers(), 0,
 				  "pg_subtrans", LWTRANCHE_SUBTRANS_BUFFER,
-				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE, false);
+				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE);
 	SlruPagePrecedesUnitTests(SubTransCtl, SUBTRANS_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 40c42f572ed..0dd1a923fc9 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -536,7 +536,7 @@ AsyncShmemInit(void)
 	NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
 	SimpleLruInit(NotifyCtl, "notify", notify_buffers, 0,
 				  "pg_notify", LWTRANCHE_NOTIFY_BUFFER, LWTRANCHE_NOTIFY_SLRU,
-				  SYNC_HANDLER_NONE, true);
+				  SYNC_HANDLER_NONE);
 
 	if (!found)
 	{
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index fe75ead3501..8e97bb35225 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -814,7 +814,7 @@ SerialInit(void)
 	SimpleLruInit(SerialSlruCtl, "serializable",
 				  serializable_buffers, 0, "pg_serial",
 				  LWTRANCHE_SERIAL_BUFFER, LWTRANCHE_SERIAL_SLRU,
-				  SYNC_HANDLER_NONE, false);
+				  SYNC_HANDLER_NONE);
 #ifdef USE_ASSERT_CHECKING
 	SerialPagePrecedesLogicallyUnitTests();
 #endif
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 2127d297bfe..434843e4afe 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -41,6 +41,7 @@
 
 #include "postgres_fe.h"
 
+#include <dirent.h>
 #include <time.h>
 
 #include "access/multixact.h"
@@ -64,6 +65,8 @@ static void prepare_new_cluster(void);
 static void prepare_new_globals(void);
 static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
+static void check_slru_segment_filenames(void);
+static void rename_slru_segments(const char *dirname);
 static void set_frozenxids(bool minmxid_only);
 static void make_outputdirs(char *pgdata);
 static void setup(char *argv0);
@@ -162,6 +165,7 @@ main(int argc, char **argv)
 
 	copy_xact_xlog_xid();
 	set_new_cluster_char_signedness();
+	check_slru_segment_filenames();
 
 	/* New now using xids of the old system */
 
@@ -902,6 +906,76 @@ copy_xact_xlog_xid(void)
 	check_ok();
 }
 
+static void
+rename_slru_segments(const char* dirname)
+{
+	DIR		   *dir;
+	struct dirent *de;
+	int 		len;
+	int64 		segno;
+	char		dir_path[MAXPGPATH];
+	char		old_path[MAXPGPATH];
+	char		new_path[MAXPGPATH];
+
+	prep_status("Renaming SLRU segments in %s", dirname);
+	snprintf(dir_path, sizeof(dir_path), "%s/%s", new_cluster.pgdata, dirname);
+
+	dir = opendir(dir_path);
+	if (dir == NULL)
+		pg_fatal("could not open directory \"%s\": %m", dir_path);
+
+	while (errno = 0, (de = readdir(dir)) != NULL)
+	{
+		/*
+		 * ignore '.', '..' and everything else that doesn't look
+		 * like an SLRU segment with a short file name
+		 */
+
+		len = strlen(de->d_name);
+		if(len != 4 && len != 5 && len != 6)
+			continue;
+
+		if(strspn(de->d_name, "0123456789ABCDEF") != len)
+			continue;
+
+		segno = strtoi64(de->d_name, NULL, 16);
+		snprintf(new_path, MAXPGPATH, "%s/%015llX", dir_path,
+					(long long) segno);
+		snprintf(old_path, MAXPGPATH, "%s/%s", dir_path, de->d_name);
+
+		if (pg_mv_file(old_path, new_path) != 0)
+			pg_fatal("could not rename file \"%s\" to \"%s\": %m",
+					 old_path, new_path);
+	}
+
+	if (errno)
+		pg_fatal("could not read directory \"%s\": %m", dir_path);
+
+	if (closedir(dir))
+		pg_fatal("could not close directory \"%s\": %m", dir_path);
+
+	check_ok();
+}
+
+static void
+check_slru_segment_filenames(void)
+{
+	int i;
+	static const char* dirs[] = {
+		"pg_xact",
+		"pg_commit_ts",
+		"pg_multixact/offsets",
+		"pg_multixact/members",
+		"pg_subtrans",
+		"pg_serial",
+	};
+
+	if(old_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER)
+		return;
+
+	for (i = 0; i < sizeof(dirs)/sizeof(dirs[0]); i++)
+		rename_slru_segments(dirs[i]);
+}
 
 /*
  *	set_frozenxids()
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index ec018e4f292..58102e6a483 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -138,6 +138,12 @@ extern char *output_files[];
  */
 #define DEFAULT_CHAR_SIGNEDNESS_CAT_VER 202502212
 
+/*
+ * change of SLRU segment filenames length in PG19
+ * TODO FIXME CHANGE TO THE ACTUAL VALUE BEFORE COMMITTING
+ */
+#define SLRU_SEG_FILENAMES_CHANGE_CAT_VER 202601061
+
 /*
  * Each relation is represented by a relinfo structure.
  */
diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl
index b1d65b8aa0f..4677e9584ca 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -246,7 +246,7 @@ sub mutilate_extra_tablespace_file
 sub mutilate_missing_file
 {
 	my ($backup_path) = @_;
-	my $pathname = "$backup_path/pg_xact/0000";
+	my $pathname = "$backup_path/pg_xact/000000000000000";
 	unlink($pathname) || die "$pathname: $!";
 	return;
 }
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 4cb8f478fce..541febf86bc 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -116,13 +116,6 @@ typedef struct SlruCtlData
 	/* Number of banks in this SLRU. */
 	uint16		nbanks;
 
-	/*
-	 * If true, use long segment file names.  Otherwise, use short file names.
-	 *
-	 * For details about the file name format, see SlruFileName().
-	 */
-	bool		long_segment_names;
-
 	/*
 	 * Which sync handler function to use when handing sync requests over to
 	 * the checkpointer.  SYNC_HANDLER_NONE to disable fsync (eg pg_notify).
@@ -169,8 +162,7 @@ extern Size SimpleLruShmemSize(int nslots, int nlsns);
 extern int	SimpleLruAutotuneBuffers(int divisor, int max);
 extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 						  const char *subdir, int buffer_tranche_id,
-						  int bank_tranche_id, SyncRequestHandler sync_handler,
-						  bool long_segment_names);
+						  int bank_tranche_id, SyncRequestHandler sync_handler);
 extern int	SimpleLruZeroPage(SlruCtl ctl, int64 pageno);
 extern void SimpleLruZeroAndWritePage(SlruCtl ctl, int64 pageno);
 extern int	SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
diff --git a/src/test/modules/test_slru/t/002_multixact_wraparound.pl b/src/test/modules/test_slru/t/002_multixact_wraparound.pl
index 3793ac1c450..8b4601ab8e1 100644
--- a/src/test/modules/test_slru/t/002_multixact_wraparound.pl
+++ b/src/test/modules/test_slru/t/002_multixact_wraparound.pl
@@ -40,7 +40,7 @@ my $slru_pages_per_segment = $1;
 my $multixact_offsets_per_page = $blcksz / 8;   # sizeof(MultiXactOffset) == 8
 my $segno =
   int(0xFFFFFFF8 / $multixact_offsets_per_page / $slru_pages_per_segment);
-my $slru_file = sprintf('%s/pg_multixact/offsets/%04X', $node_pgdata, $segno);
+my $slru_file = sprintf('%s/pg_multixact/offsets/%015X', $node_pgdata, $segno);
 open my $fh, ">", $slru_file
   or die "could not open \"$slru_file\": $!";
 binmode $fh;
@@ -50,8 +50,8 @@ syswrite($fh, "\0" x $bytes_per_seg) == $bytes_per_seg
 close $fh;
 
 # remove old file
-unlink("$node_pgdata/pg_multixact/offsets/0000")
-  or die "could not unlink \"$node_pgdata/pg_multixact/offsets/0000\": $!";
+unlink("$node_pgdata/pg_multixact/offsets/000000000000000")
+  or die "could not unlink \"$node_pgdata/pg_multixact/offsets/000000000000000\": $!";
 
 # Consume multixids to wrap around.  We start at 0xFFFFFFF8, so after
 # creating 16 multixacts we should definitely have wrapped around.
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 4dc74e19620..642f1aeb8ca 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -213,11 +213,6 @@ test_slru_page_precedes_logically(int64 page1, int64 page2)
 static void
 test_slru_shmem_startup(void)
 {
-	/*
-	 * Short segments names are well tested elsewhere so in this test we are
-	 * focusing on long names.
-	 */
-	const bool	long_segment_names = true;
 	const char	slru_dir_name[] = "pg_test_slru";
 	int			test_tranche_id = -1;
 	int			test_buffer_tranche_id = -1;
@@ -247,8 +242,7 @@ test_slru_shmem_startup(void)
 	TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
 	SimpleLruInit(TestSlruCtl, "TestSLRU",
 				  NUM_TEST_BUFFERS, 0, slru_dir_name,
-				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE,
-				  long_segment_names);
+				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE);
 }
 
 void
-- 
2.43.0



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: [PATCH] Refactor SLRU to always use long file names
  2025-01-15 12:42 Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-02-25 14:18 ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-03-28 20:27   ` Re: [PATCH] Refactor SLRU to always use long file names Rustam ALLAKOV <[email protected]>
  2025-04-03 14:25     ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-08-13 11:54       ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2026-01-06 15:18         ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
@ 2026-01-06 15:28           ` Heikki Linnakangas <[email protected]>
  2026-01-07 14:42             ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Heikki Linnakangas @ 2026-01-06 15:28 UTC (permalink / raw)
  To: Aleksander Alekseev <[email protected]>; [email protected]; +Cc: Rustam ALLAKOV <[email protected]>; Michael Paquier <[email protected]>

On 06/01/2026 17:18, Aleksander Alekseev wrote:
> +static void
> +check_slru_segment_filenames(void)
> +{
> +	int i;
> +	static const char* dirs[] = {
> +		"pg_xact",
> +		"pg_commit_ts",
> +		"pg_multixact/offsets",
> +		"pg_multixact/members",
> +		"pg_subtrans",
> +		"pg_serial",
> +	};
> +
> +	if(old_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER)
> +		return;
> +
> +	for (i = 0; i < sizeof(dirs)/sizeof(dirs[0]); i++)
> +		rename_slru_segments(dirs[i]);
> +}

Since commit bd8d9c9bdf "Widen MultiXactOffset to 64 bits", 
"pg_multixact/members" should not be in that list anymore.

Also, it seems misleading that a function called "check_*" doesn't 
merely check for things, but renames files. Also, it seems silly to 
first copy/link the files with the old short names, and rename them 
later. Could we copy/link them with the new long names to begin with?

(No comment on whether this is a good idea in general or the rest of the 
patch)

- Heikki







^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: [PATCH] Refactor SLRU to always use long file names
  2025-01-15 12:42 Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-02-25 14:18 ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-03-28 20:27   ` Re: [PATCH] Refactor SLRU to always use long file names Rustam ALLAKOV <[email protected]>
  2025-04-03 14:25     ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-08-13 11:54       ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2026-01-06 15:18         ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2026-01-06 15:28           ` Re: [PATCH] Refactor SLRU to always use long file names Heikki Linnakangas <[email protected]>
@ 2026-01-07 14:42             ` Aleksander Alekseev <[email protected]>
  2026-04-08 10:09               ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 9+ messages in thread

From: Aleksander Alekseev @ 2026-01-07 14:42 UTC (permalink / raw)
  To: [email protected]; +Cc: Heikki Linnakangas <[email protected]>; Rustam ALLAKOV <[email protected]>; Michael Paquier <[email protected]>

Hi Heikki,

> Since commit bd8d9c9bdf "Widen MultiXactOffset to 64 bits",
> "pg_multixact/members" should not be in that list anymore.

I missed this one. Fixed, thanks.

> Also, it seems misleading that a function called "check_*" doesn't
> merely check for things, but renames files.

Fair point. I renamed it to `ensure_long_slru_segment_filenames`.

> Could we copy/link them with the new long names to begin with?

That's an interesting idea.

What I personally don't like about it is the fact that a single
migration will affect the logic of every run of pg_upgrade, even in
the far future, for instances that don't need this migration.
Previously I showed [1] that the entire migration takes little time
(note that we had to migrate pg_multixact/members back then). So I
don't think this optimization is a good idea in the long run, unless
we reach a consensus on the opposite.

[1]: https://www.postgresql.org/message-id/CAJ7c6TPdDSnVR2ZGn-oirfvvFXZghS5PAXYwmtb7_nNps53eEg%40mail.gma...

-- 
Best regards,
Aleksander Alekseev


Attachments:

  [text/x-patch] v8-0001-Always-use-long-SLRU-segment-file-names.patch (16.4K, 2-v8-0001-Always-use-long-SLRU-segment-file-names.patch)
  download | inline diff:
From 8223661f9194a76d1d3e09a2fad4ef3cec8f909f Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v8] Always use long SLRU segment file names

PG17 introduced long SLRU segment file names (commit 4ed8f0913bfd). We used
short or long file names depending on SlruCtl->long_segment_names. This commit
refactors SLRU to always use long file names in order to simplify the code.

Author: Aleksander Alekseev <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Reviewed-by: Rustam ALLAKOV <[email protected]>
Reviewed-by: Heikki Linnakangas <[email protected]>
Discussion: https://postgr.es/m/CAJ7c6TOy7fUW9MuNeOWor3cSFnQg9tgz=mjXHDb94GORtM_Eyg@mail.gmail.com

(!!!) bump catversion and change the corresponding TODO FIXME line in pg_upgrade.h
---
 src/backend/access/transam/clog.c             |  2 +-
 src/backend/access/transam/commit_ts.c        |  3 +-
 src/backend/access/transam/multixact.c        |  6 +-
 src/backend/access/transam/slru.c             | 71 ++++--------------
 src/backend/access/transam/subtrans.c         |  2 +-
 src/backend/commands/async.c                  |  2 +-
 src/backend/storage/lmgr/predicate.c          |  2 +-
 src/bin/pg_upgrade/pg_upgrade.c               | 73 +++++++++++++++++++
 src/bin/pg_upgrade/pg_upgrade.h               |  6 ++
 src/bin/pg_verifybackup/t/003_corruption.pl   |  2 +-
 src/include/access/slru.h                     | 10 +--
 .../test_slru/t/002_multixact_wraparound.pl   |  6 +-
 src/test/modules/test_slru/test_slru.c        |  8 +-
 13 files changed, 105 insertions(+), 88 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index b5c38bbb162..e46dc13fb5a 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -809,7 +809,7 @@ CLOGShmemInit(void)
 	XactCtl->PagePrecedes = CLOGPagePrecedes;
 	SimpleLruInit(XactCtl, "transaction", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
 				  "pg_xact", LWTRANCHE_XACT_BUFFER,
-				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG, false);
+				  LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG);
 	SlruPagePrecedesUnitTests(XactCtl, CLOG_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 082b564da8f..25774fbc1b9 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -554,8 +554,7 @@ CommitTsShmemInit(void)
 	SimpleLruInit(CommitTsCtl, "commit_timestamp", CommitTsShmemBuffers(), 0,
 				  "pg_commit_ts", LWTRANCHE_COMMITTS_BUFFER,
 				  LWTRANCHE_COMMITTS_SLRU,
-				  SYNC_HANDLER_COMMIT_TS,
-				  false);
+				  SYNC_HANDLER_COMMIT_TS);
 	SlruPagePrecedesUnitTests(CommitTsCtl, COMMIT_TS_XACTS_PER_PAGE);
 
 	commitTsShared = ShmemInitStruct("CommitTs shared",
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 3f423636b48..3482b1c16ad 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1736,15 +1736,13 @@ MultiXactShmemInit(void)
 				  "multixact_offset", multixact_offset_buffers, 0,
 				  "pg_multixact/offsets", LWTRANCHE_MULTIXACTOFFSET_BUFFER,
 				  LWTRANCHE_MULTIXACTOFFSET_SLRU,
-				  SYNC_HANDLER_MULTIXACT_OFFSET,
-				  false);
+				  SYNC_HANDLER_MULTIXACT_OFFSET);
 	SlruPagePrecedesUnitTests(MultiXactOffsetCtl, MULTIXACT_OFFSETS_PER_PAGE);
 	SimpleLruInit(MultiXactMemberCtl,
 				  "multixact_member", multixact_member_buffers, 0,
 				  "pg_multixact/members", LWTRANCHE_MULTIXACTMEMBER_BUFFER,
 				  LWTRANCHE_MULTIXACTMEMBER_SLRU,
-				  SYNC_HANDLER_MULTIXACT_MEMBER,
-				  true);
+				  SYNC_HANDLER_MULTIXACT_MEMBER);
 	/* doesn't call SimpleLruTruncate() or meet criteria for unit tests */
 
 	/* Initialize our shared state struct */
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 549c7e3e64b..18baedd6de6 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -77,41 +77,21 @@
  *
  * "path" should point to a buffer at least MAXPGPATH characters long.
  *
- * If ctl->long_segment_names is true, segno can be in the range [0, 2^60-1].
- * The resulting file name is made of 15 characters, e.g. dir/123456789ABCDEF.
- *
- * If ctl->long_segment_names is false, segno can be in the range [0, 2^24-1].
- * The resulting file name is made of 4 to 6 characters, as of:
- *
- *  dir/1234   for [0, 2^16-1]
- *  dir/12345  for [2^16, 2^20-1]
- *  dir/123456 for [2^20, 2^24-1]
+ * segno can be in the range [0, 2^60-1]. The resulting file name is made
+ * of 15 characters, e.g. dir/123456789ABCDEF.
  */
 static inline int
 SlruFileName(SlruCtl ctl, char *path, int64 segno)
 {
-	if (ctl->long_segment_names)
-	{
-		/*
-		 * We could use 16 characters here but the disadvantage would be that
-		 * the SLRU segments will be hard to distinguish from WAL segments.
-		 *
-		 * For this reason we use 15 characters. It is enough but also means
-		 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%015" PRIX64, ctl->Dir, segno);
-	}
-	else
-	{
-		/*
-		 * Despite the fact that %04X format string is used up to 24 bit
-		 * integers are allowed. See SlruCorrectSegmentFilenameLength()
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir,
-						(unsigned int) segno);
-	}
+	/*
+	 * We could use 16 characters here but the disadvantage would be that
+	 * the SLRU segments will be hard to distinguish from WAL segments.
+	 *
+	 * For this reason we use 15 characters. It is enough but also means
+	 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
+	 */
+	Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
+	return snprintf(path, MAXPGPATH, "%s/%015" PRIX64, ctl->Dir, segno);
 }
 
 /*
@@ -251,7 +231,7 @@ SimpleLruAutotuneBuffers(int divisor, int max)
 void
 SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 			  const char *subdir, int buffer_tranche_id, int bank_tranche_id,
-			  SyncRequestHandler sync_handler, bool long_segment_names)
+			  SyncRequestHandler sync_handler)
 {
 	SlruShared	shared;
 	bool		found;
@@ -342,7 +322,6 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 	 */
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
-	ctl->long_segment_names = long_segment_names;
 	ctl->nbanks = nbanks;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
@@ -1773,30 +1752,6 @@ SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int64 segpage, void *data)
 	return false;				/* keep going */
 }
 
-/*
- * An internal function used by SlruScanDirectory().
- *
- * Returns true if a file with a name of a given length may be a correct
- * SLRU segment.
- */
-static inline bool
-SlruCorrectSegmentFilenameLength(SlruCtl ctl, size_t len)
-{
-	if (ctl->long_segment_names)
-		return (len == 15);		/* see SlruFileName() */
-	else
-
-		/*
-		 * Commit 638cf09e76d allowed 5-character lengths. Later commit
-		 * 73c986adde5 allowed 6-character length.
-		 *
-		 * Note: There is an ongoing plan to migrate all SLRUs to 64-bit page
-		 * numbers, and the corresponding 15-character file names, which may
-		 * eventually deprecate the support for 4, 5, and 6-character names.
-		 */
-		return (len == 4 || len == 5 || len == 6);
-}
-
 /*
  * Scan the SimpleLru directory and apply a callback to each file found in it.
  *
@@ -1828,7 +1783,7 @@ SlruScanDirectory(SlruCtl ctl, SlruScanCallback callback, void *data)
 
 		len = strlen(clde->d_name);
 
-		if (SlruCorrectSegmentFilenameLength(ctl, len) &&
+		if ((len == 15) &&
 			strspn(clde->d_name, "0123456789ABCDEF") == len)
 		{
 			segno = strtoi64(clde->d_name, NULL, 16);
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index c0987f43f11..3536971ca7d 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -242,7 +242,7 @@ SUBTRANSShmemInit(void)
 	SubTransCtl->PagePrecedes = SubTransPagePrecedes;
 	SimpleLruInit(SubTransCtl, "subtransaction", SUBTRANSShmemBuffers(), 0,
 				  "pg_subtrans", LWTRANCHE_SUBTRANS_BUFFER,
-				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE, false);
+				  LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE);
 	SlruPagePrecedesUnitTests(SubTransCtl, SUBTRANS_XACTS_PER_PAGE);
 }
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 40c42f572ed..0dd1a923fc9 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -536,7 +536,7 @@ AsyncShmemInit(void)
 	NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
 	SimpleLruInit(NotifyCtl, "notify", notify_buffers, 0,
 				  "pg_notify", LWTRANCHE_NOTIFY_BUFFER, LWTRANCHE_NOTIFY_SLRU,
-				  SYNC_HANDLER_NONE, true);
+				  SYNC_HANDLER_NONE);
 
 	if (!found)
 	{
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index fe75ead3501..8e97bb35225 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -814,7 +814,7 @@ SerialInit(void)
 	SimpleLruInit(SerialSlruCtl, "serializable",
 				  serializable_buffers, 0, "pg_serial",
 				  LWTRANCHE_SERIAL_BUFFER, LWTRANCHE_SERIAL_SLRU,
-				  SYNC_HANDLER_NONE, false);
+				  SYNC_HANDLER_NONE);
 #ifdef USE_ASSERT_CHECKING
 	SerialPagePrecedesLogicallyUnitTests();
 #endif
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 2127d297bfe..94964f32eae 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -41,6 +41,7 @@
 
 #include "postgres_fe.h"
 
+#include <dirent.h>
 #include <time.h>
 
 #include "access/multixact.h"
@@ -64,6 +65,8 @@ static void prepare_new_cluster(void);
 static void prepare_new_globals(void);
 static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
+static void ensure_long_slru_segment_filenames(void);
+static void rename_slru_segments(const char *dirname);
 static void set_frozenxids(bool minmxid_only);
 static void make_outputdirs(char *pgdata);
 static void setup(char *argv0);
@@ -162,6 +165,7 @@ main(int argc, char **argv)
 
 	copy_xact_xlog_xid();
 	set_new_cluster_char_signedness();
+	ensure_long_slru_segment_filenames();
 
 	/* New now using xids of the old system */
 
@@ -902,6 +906,75 @@ copy_xact_xlog_xid(void)
 	check_ok();
 }
 
+static void
+rename_slru_segments(const char* dirname)
+{
+	DIR		   *dir;
+	struct dirent *de;
+	int 		len;
+	int64 		segno;
+	char		dir_path[MAXPGPATH];
+	char		old_path[MAXPGPATH];
+	char		new_path[MAXPGPATH];
+
+	prep_status("Renaming SLRU segments in %s", dirname);
+	snprintf(dir_path, sizeof(dir_path), "%s/%s", new_cluster.pgdata, dirname);
+
+	dir = opendir(dir_path);
+	if (dir == NULL)
+		pg_fatal("could not open directory \"%s\": %m", dir_path);
+
+	while (errno = 0, (de = readdir(dir)) != NULL)
+	{
+		/*
+		 * ignore '.', '..' and everything else that doesn't look
+		 * like an SLRU segment with a short file name
+		 */
+
+		len = strlen(de->d_name);
+		if(len != 4 && len != 5 && len != 6)
+			continue;
+
+		if(strspn(de->d_name, "0123456789ABCDEF") != len)
+			continue;
+
+		segno = strtoi64(de->d_name, NULL, 16);
+		snprintf(new_path, MAXPGPATH, "%s/%015llX", dir_path,
+					(long long) segno);
+		snprintf(old_path, MAXPGPATH, "%s/%s", dir_path, de->d_name);
+
+		if (pg_mv_file(old_path, new_path) != 0)
+			pg_fatal("could not rename file \"%s\" to \"%s\": %m",
+					 old_path, new_path);
+	}
+
+	if (errno)
+		pg_fatal("could not read directory \"%s\": %m", dir_path);
+
+	if (closedir(dir))
+		pg_fatal("could not close directory \"%s\": %m", dir_path);
+
+	check_ok();
+}
+
+static void
+ensure_long_slru_segment_filenames(void)
+{
+	int i;
+	static const char* dirs[] = {
+		"pg_xact",
+		"pg_commit_ts",
+		"pg_multixact/offsets",
+		"pg_subtrans",
+		"pg_serial",
+	};
+
+	if(old_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER)
+		return;
+
+	for (i = 0; i < sizeof(dirs)/sizeof(dirs[0]); i++)
+		rename_slru_segments(dirs[i]);
+}
 
 /*
  *	set_frozenxids()
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index ec018e4f292..58102e6a483 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -138,6 +138,12 @@ extern char *output_files[];
  */
 #define DEFAULT_CHAR_SIGNEDNESS_CAT_VER 202502212
 
+/*
+ * change of SLRU segment filenames length in PG19
+ * TODO FIXME CHANGE TO THE ACTUAL VALUE BEFORE COMMITTING
+ */
+#define SLRU_SEG_FILENAMES_CHANGE_CAT_VER 202601061
+
 /*
  * Each relation is represented by a relinfo structure.
  */
diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl
index b1d65b8aa0f..4677e9584ca 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -246,7 +246,7 @@ sub mutilate_extra_tablespace_file
 sub mutilate_missing_file
 {
 	my ($backup_path) = @_;
-	my $pathname = "$backup_path/pg_xact/0000";
+	my $pathname = "$backup_path/pg_xact/000000000000000";
 	unlink($pathname) || die "$pathname: $!";
 	return;
 }
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 4cb8f478fce..541febf86bc 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -116,13 +116,6 @@ typedef struct SlruCtlData
 	/* Number of banks in this SLRU. */
 	uint16		nbanks;
 
-	/*
-	 * If true, use long segment file names.  Otherwise, use short file names.
-	 *
-	 * For details about the file name format, see SlruFileName().
-	 */
-	bool		long_segment_names;
-
 	/*
 	 * Which sync handler function to use when handing sync requests over to
 	 * the checkpointer.  SYNC_HANDLER_NONE to disable fsync (eg pg_notify).
@@ -169,8 +162,7 @@ extern Size SimpleLruShmemSize(int nslots, int nlsns);
 extern int	SimpleLruAutotuneBuffers(int divisor, int max);
 extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 						  const char *subdir, int buffer_tranche_id,
-						  int bank_tranche_id, SyncRequestHandler sync_handler,
-						  bool long_segment_names);
+						  int bank_tranche_id, SyncRequestHandler sync_handler);
 extern int	SimpleLruZeroPage(SlruCtl ctl, int64 pageno);
 extern void SimpleLruZeroAndWritePage(SlruCtl ctl, int64 pageno);
 extern int	SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
diff --git a/src/test/modules/test_slru/t/002_multixact_wraparound.pl b/src/test/modules/test_slru/t/002_multixact_wraparound.pl
index 3793ac1c450..8b4601ab8e1 100644
--- a/src/test/modules/test_slru/t/002_multixact_wraparound.pl
+++ b/src/test/modules/test_slru/t/002_multixact_wraparound.pl
@@ -40,7 +40,7 @@ my $slru_pages_per_segment = $1;
 my $multixact_offsets_per_page = $blcksz / 8;   # sizeof(MultiXactOffset) == 8
 my $segno =
   int(0xFFFFFFF8 / $multixact_offsets_per_page / $slru_pages_per_segment);
-my $slru_file = sprintf('%s/pg_multixact/offsets/%04X', $node_pgdata, $segno);
+my $slru_file = sprintf('%s/pg_multixact/offsets/%015X', $node_pgdata, $segno);
 open my $fh, ">", $slru_file
   or die "could not open \"$slru_file\": $!";
 binmode $fh;
@@ -50,8 +50,8 @@ syswrite($fh, "\0" x $bytes_per_seg) == $bytes_per_seg
 close $fh;
 
 # remove old file
-unlink("$node_pgdata/pg_multixact/offsets/0000")
-  or die "could not unlink \"$node_pgdata/pg_multixact/offsets/0000\": $!";
+unlink("$node_pgdata/pg_multixact/offsets/000000000000000")
+  or die "could not unlink \"$node_pgdata/pg_multixact/offsets/000000000000000\": $!";
 
 # Consume multixids to wrap around.  We start at 0xFFFFFFF8, so after
 # creating 16 multixacts we should definitely have wrapped around.
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 4dc74e19620..642f1aeb8ca 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -213,11 +213,6 @@ test_slru_page_precedes_logically(int64 page1, int64 page2)
 static void
 test_slru_shmem_startup(void)
 {
-	/*
-	 * Short segments names are well tested elsewhere so in this test we are
-	 * focusing on long names.
-	 */
-	const bool	long_segment_names = true;
 	const char	slru_dir_name[] = "pg_test_slru";
 	int			test_tranche_id = -1;
 	int			test_buffer_tranche_id = -1;
@@ -247,8 +242,7 @@ test_slru_shmem_startup(void)
 	TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
 	SimpleLruInit(TestSlruCtl, "TestSLRU",
 				  NUM_TEST_BUFFERS, 0, slru_dir_name,
-				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE,
-				  long_segment_names);
+				  test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE);
 }
 
 void
-- 
2.43.0



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

* Re: [PATCH] Refactor SLRU to always use long file names
  2025-01-15 12:42 Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-02-25 14:18 ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-03-28 20:27   ` Re: [PATCH] Refactor SLRU to always use long file names Rustam ALLAKOV <[email protected]>
  2025-04-03 14:25     ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2025-08-13 11:54       ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2026-01-06 15:18         ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
  2026-01-06 15:28           ` Re: [PATCH] Refactor SLRU to always use long file names Heikki Linnakangas <[email protected]>
  2026-01-07 14:42             ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
@ 2026-04-08 10:09               ` Aleksander Alekseev <[email protected]>
  0 siblings, 0 replies; 9+ messages in thread

From: Aleksander Alekseev @ 2026-04-08 10:09 UTC (permalink / raw)
  To: [email protected]; +Cc: Heikki Linnakangas <[email protected]>; Rustam ALLAKOV <[email protected]>; Michael Paquier <[email protected]>

Hi,

> > Since commit bd8d9c9bdf "Widen MultiXactOffset to 64 bits",
> > "pg_multixact/members" should not be in that list anymore.
>
> I missed this one. Fixed, thanks.
>
> > Also, it seems misleading that a function called "check_*" doesn't
> > merely check for things, but renames files.
>
> Fair point. I renamed it to `ensure_long_slru_segment_filenames`.
>
> > Could we copy/link them with the new long names to begin with?
>
> That's an interesting idea.
>
> What I personally don't like about it is the fact that a single
> migration will affect the logic of every run of pg_upgrade, even in
> the far future, for instances that don't need this migration.
> Previously I showed [1] that the entire migration takes little time
> (note that we had to migrate pg_multixact/members back then). So I
> don't think this optimization is a good idea in the long run, unless
> we reach a consensus on the opposite.

Rebased.

-- 
Best regards,
Aleksander Alekseev


Attachments:

  [text/x-patch] v9-0001-Always-use-long-SLRU-segment-file-names.patch (14.1K, 2-v9-0001-Always-use-long-SLRU-segment-file-names.patch)
  download | inline diff:
From 0275ac047cfe6fced648091aa14f8423fc2383a8 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Wed, 11 Sep 2024 13:17:33 +0300
Subject: [PATCH v9] Always use long SLRU segment file names

PG17 introduced long SLRU segment file names (commit 4ed8f0913bfd). We used
short or long file names depending on SlruCtl->long_segment_names. This commit
refactors SLRU to always use long file names in order to simplify the code.

Author: Aleksander Alekseev <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Reviewed-by: Rustam ALLAKOV <[email protected]>
Reviewed-by: Heikki Linnakangas <[email protected]>
Discussion: https://postgr.es/m/CAJ7c6TOy7fUW9MuNeOWor3cSFnQg9tgz=mjXHDb94GORtM_Eyg@mail.gmail.com

(!!!) bump catversion and change the corresponding TODO FIXME line in pg_upgrade.h
---
 src/backend/access/transam/clog.c             |  1 -
 src/backend/access/transam/commit_ts.c        |  2 -
 src/backend/access/transam/multixact.c        |  2 -
 src/backend/access/transam/slru.c             | 68 +++--------------
 src/backend/access/transam/subtrans.c         |  1 -
 src/backend/commands/async.c                  |  3 -
 src/backend/storage/lmgr/predicate.c          |  1 -
 src/bin/pg_upgrade/pg_upgrade.c               | 73 +++++++++++++++++++
 src/bin/pg_upgrade/pg_upgrade.h               |  6 ++
 src/bin/pg_verifybackup/t/003_corruption.pl   |  2 +-
 src/include/access/slru.h                     |  8 --
 .../test_slru/t/002_multixact_wraparound.pl   |  6 +-
 src/test/modules/test_slru/test_slru.c        |  6 --
 13 files changed, 95 insertions(+), 84 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 75012d4b8f0..eba4ff6edf9 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -812,7 +812,6 @@ CLOGShmemRequest(void *arg)
 	SimpleLruRequest(.desc = &XactSlruDesc,
 					 .name = "transaction",
 					 .Dir = "pg_xact",
-					 .long_segment_names = false,
 
 					 .nslots = CLOGShmemBuffers(),
 					 .nlsns = CLOG_LSNS_PER_PAGE,
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 2625cbf93bf..d9b5f112975 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -551,8 +551,6 @@ CommitTsShmemRequest(void *arg)
 	SimpleLruRequest(.desc = &CommitTsSlruDesc,
 					 .name = "commit_timestamp",
 					 .Dir = "pg_commit_ts",
-					 .long_segment_names = false,
-
 					 .nslots = CommitTsShmemBuffers(),
 
 					 .PagePrecedes = CommitTsPagePrecedes,
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index cb78ba0842d..8037a6ac451 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1785,7 +1785,6 @@ MultiXactShmemRequest(void *arg)
 	SimpleLruRequest(.desc = &MultiXactOffsetSlruDesc,
 					 .name = "multixact_offset",
 					 .Dir = "pg_multixact/offsets",
-					 .long_segment_names = false,
 
 					 .nslots = multixact_offset_buffers,
 
@@ -1800,7 +1799,6 @@ MultiXactShmemRequest(void *arg)
 	SimpleLruRequest(.desc = &MultiXactMemberSlruDesc,
 					 .name = "multixact_member",
 					 .Dir = "pg_multixact/members",
-					 .long_segment_names = true,
 
 					 .nslots = multixact_member_buffers,
 
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 47dd52d6749..4a4dff20183 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -80,41 +80,21 @@
  *
  * "path" should point to a buffer at least MAXPGPATH characters long.
  *
- * If ctl->long_segment_names is true, segno can be in the range [0, 2^60-1].
- * The resulting file name is made of 15 characters, e.g. dir/123456789ABCDEF.
- *
- * If ctl->long_segment_names is false, segno can be in the range [0, 2^24-1].
- * The resulting file name is made of 4 to 6 characters, as of:
- *
- *  dir/1234   for [0, 2^16-1]
- *  dir/12345  for [2^16, 2^20-1]
- *  dir/123456 for [2^20, 2^24-1]
+ * segno can be in the range [0, 2^60-1]. The resulting file name is made
+ * of 15 characters, e.g. dir/123456789ABCDEF.
  */
 static inline int
 SlruFileName(SlruDesc *ctl, char *path, int64 segno)
 {
-	if (ctl->options.long_segment_names)
-	{
-		/*
-		 * We could use 16 characters here but the disadvantage would be that
-		 * the SLRU segments will be hard to distinguish from WAL segments.
-		 *
-		 * For this reason we use 15 characters. It is enough but also means
-		 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%015" PRIX64, ctl->options.Dir, segno);
-	}
-	else
-	{
-		/*
-		 * Despite the fact that %04X format string is used up to 24 bit
-		 * integers are allowed. See SlruCorrectSegmentFilenameLength()
-		 */
-		Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFF));
-		return snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->options.Dir,
-						(unsigned int) segno);
-	}
+	/*
+	 * We could use 16 characters here but the disadvantage would be that
+	 * the SLRU segments will be hard to distinguish from WAL segments.
+	 *
+	 * For this reason we use 15 characters. It is enough but also means
+	 * that in the future we can't decrease SLRU_PAGES_PER_SEGMENT easily.
+	 */
+	Assert(segno >= 0 && segno <= INT64CONST(0xFFFFFFFFFFFFFFF));
+	return snprintf(path, MAXPGPATH, "%s/%015" PRIX64, ctl->options.Dir, segno);
 }
 
 /*
@@ -1801,30 +1781,6 @@ SlruScanDirCbDeleteAll(SlruDesc *ctl, char *filename, int64 segpage, void *data)
 	return false;				/* keep going */
 }
 
-/*
- * An internal function used by SlruScanDirectory().
- *
- * Returns true if a file with a name of a given length may be a correct
- * SLRU segment.
- */
-static inline bool
-SlruCorrectSegmentFilenameLength(SlruDesc *ctl, size_t len)
-{
-	if (ctl->options.long_segment_names)
-		return (len == 15);		/* see SlruFileName() */
-	else
-
-		/*
-		 * Commit 638cf09e76d allowed 5-character lengths. Later commit
-		 * 73c986adde5 allowed 6-character length.
-		 *
-		 * Note: There is an ongoing plan to migrate all SLRUs to 64-bit page
-		 * numbers, and the corresponding 15-character file names, which may
-		 * eventually deprecate the support for 4, 5, and 6-character names.
-		 */
-		return (len == 4 || len == 5 || len == 6);
-}
-
 /*
  * Scan the SimpleLru directory and apply a callback to each file found in it.
  *
@@ -1856,7 +1812,7 @@ SlruScanDirectory(SlruDesc *ctl, SlruScanCallback callback, void *data)
 
 		len = strlen(clde->d_name);
 
-		if (SlruCorrectSegmentFilenameLength(ctl, len) &&
+		if ((len == 15) &&
 			strspn(clde->d_name, "0123456789ABCDEF") == len)
 		{
 			segno = strtoi64(clde->d_name, NULL, 16);
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index b79e648b899..916a4fb6c0a 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -246,7 +246,6 @@ SUBTRANSShmemRequest(void *arg)
 	SimpleLruRequest(.desc = &SubTransSlruDesc,
 					 .name = "subtransaction",
 					 .Dir = "pg_subtrans",
-					 .long_segment_names = false,
 
 					 .nslots = SUBTRANSShmemBuffers(),
 
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index db6a9a6561b..ab4708e7866 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -811,9 +811,6 @@ AsyncShmemRequest(void *arg)
 					 .name = "notify",
 					 .Dir = "pg_notify",
 
-	/* long segment names are used in order to avoid wraparound */
-					 .long_segment_names = true,
-
 					 .nslots = notify_buffers,
 
 					 .sync_handler = SYNC_HANDLER_NONE,
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 899a4ef06e4..1ffbe779237 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -1219,7 +1219,6 @@ PredicateLockShmemRequest(void *arg)
 	SimpleLruRequest(.desc = &SerialSlruDesc,
 					 .name = "serializable",
 					 .Dir = "pg_serial",
-					 .long_segment_names = false,
 
 					 .nslots = serializable_buffers,
 
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 2127d297bfe..a1a0d1766f5 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -41,6 +41,7 @@
 
 #include "postgres_fe.h"
 
+#include <dirent.h>
 #include <time.h>
 
 #include "access/multixact.h"
@@ -64,6 +65,8 @@ static void prepare_new_cluster(void);
 static void prepare_new_globals(void);
 static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
+static void ensure_long_slru_segment_filenames(void);
+static void rename_slru_segments(const char *dirname);
 static void set_frozenxids(bool minmxid_only);
 static void make_outputdirs(char *pgdata);
 static void setup(char *argv0);
@@ -162,6 +165,7 @@ main(int argc, char **argv)
 
 	copy_xact_xlog_xid();
 	set_new_cluster_char_signedness();
+	ensure_long_slru_segment_filenames();
 
 	/* New now using xids of the old system */
 
@@ -902,6 +906,75 @@ copy_xact_xlog_xid(void)
 	check_ok();
 }
 
+static void
+rename_slru_segments(const char *dirname)
+{
+	DIR		   *dir;
+	struct dirent *de;
+	int			len;
+	int64		segno;
+	char		dir_path[MAXPGPATH];
+	char		old_path[MAXPGPATH];
+	char		new_path[MAXPGPATH];
+
+	prep_status("Renaming SLRU segments in %s", dirname);
+	snprintf(dir_path, sizeof(dir_path), "%s/%s", new_cluster.pgdata, dirname);
+
+	dir = opendir(dir_path);
+	if (dir == NULL)
+		pg_fatal("could not open directory \"%s\": %m", dir_path);
+
+	while (errno = 0, (de = readdir(dir)) != NULL)
+	{
+		/*
+		 * ignore '.', '..' and everything else that doesn't look like an SLRU
+		 * segment with a short file name
+		 */
+
+		len = strlen(de->d_name);
+		if (len != 4 && len != 5 && len != 6)
+			continue;
+
+		if (strspn(de->d_name, "0123456789ABCDEF") != len)
+			continue;
+
+		segno = strtoi64(de->d_name, NULL, 16);
+		snprintf(new_path, MAXPGPATH, "%s/%015" PRIX64, dir_path,
+				 (uint64) segno);
+		snprintf(old_path, MAXPGPATH, "%s/%s", dir_path, de->d_name);
+
+		if (pg_mv_file(old_path, new_path) != 0)
+			pg_fatal("could not rename file \"%s\" to \"%s\": %m",
+					 old_path, new_path);
+	}
+
+	if (errno)
+		pg_fatal("could not read directory \"%s\": %m", dir_path);
+
+	if (closedir(dir))
+		pg_fatal("could not close directory \"%s\": %m", dir_path);
+
+	check_ok();
+}
+
+static void
+ensure_long_slru_segment_filenames(void)
+{
+	int			i;
+	static const char *dirs[] = {
+		"pg_xact",
+		"pg_commit_ts",
+		"pg_multixact/offsets",
+		"pg_subtrans",
+		"pg_serial",
+	};
+
+	if (old_cluster.controldata.cat_ver >= SLRU_SEG_FILENAMES_CHANGE_CAT_VER)
+		return;
+
+	for (i = 0; i < sizeof(dirs) / sizeof(dirs[0]); i++)
+		rename_slru_segments(dirs[i]);
+}
 
 /*
  *	set_frozenxids()
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 1d767bbda2d..07278dbc914 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -138,6 +138,12 @@ extern char *output_files[];
  */
 #define DEFAULT_CHAR_SIGNEDNESS_CAT_VER 202502212
 
+/*
+ * change of SLRU segment filenames length in PG19
+ * TODO FIXME CHANGE TO THE ACTUAL VALUE BEFORE COMMITTING
+ */
+#define SLRU_SEG_FILENAMES_CHANGE_CAT_VER 202601061
+
 /*
  * Each relation is represented by a relinfo structure.
  */
diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl
index 405ea793b68..37300634bfa 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -259,7 +259,7 @@ sub mutilate_extra_tablespace_file
 sub mutilate_missing_file
 {
 	my ($backup_path) = @_;
-	my $pathname = "$backup_path/pg_xact/0000";
+	my $pathname = "$backup_path/pg_xact/000000000000000";
 	unlink($pathname) || die "$pathname: $!";
 	return;
 }
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index b4adb1789c7..82a802c20d9 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -145,14 +145,6 @@ typedef struct SlruOpts
 	 */
 	const char *Dir;
 
-	/*
-	 * If true, use long segment file names.  Otherwise, use short file names.
-	 *
-	 * For details about the file name format, see SlruFileName().
-	 */
-	bool		long_segment_names;
-
-
 	/*
 	 * Decide whether a page is "older" for truncation and as a hint for
 	 * evicting pages in LRU order.  Return true if every entry of the first
diff --git a/src/test/modules/test_slru/t/002_multixact_wraparound.pl b/src/test/modules/test_slru/t/002_multixact_wraparound.pl
index 3793ac1c450..8b4601ab8e1 100644
--- a/src/test/modules/test_slru/t/002_multixact_wraparound.pl
+++ b/src/test/modules/test_slru/t/002_multixact_wraparound.pl
@@ -40,7 +40,7 @@ my $slru_pages_per_segment = $1;
 my $multixact_offsets_per_page = $blcksz / 8;   # sizeof(MultiXactOffset) == 8
 my $segno =
   int(0xFFFFFFF8 / $multixact_offsets_per_page / $slru_pages_per_segment);
-my $slru_file = sprintf('%s/pg_multixact/offsets/%04X', $node_pgdata, $segno);
+my $slru_file = sprintf('%s/pg_multixact/offsets/%015X', $node_pgdata, $segno);
 open my $fh, ">", $slru_file
   or die "could not open \"$slru_file\": $!";
 binmode $fh;
@@ -50,8 +50,8 @@ syswrite($fh, "\0" x $bytes_per_seg) == $bytes_per_seg
 close $fh;
 
 # remove old file
-unlink("$node_pgdata/pg_multixact/offsets/0000")
-  or die "could not unlink \"$node_pgdata/pg_multixact/offsets/0000\": $!";
+unlink("$node_pgdata/pg_multixact/offsets/000000000000000")
+  or die "could not unlink \"$node_pgdata/pg_multixact/offsets/000000000000000\": $!";
 
 # Consume multixids to wrap around.  We start at 0xFFFFFFF8, so after
 # creating 16 multixacts we should definitely have wrapped around.
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 40efffdbf62..d4b696d10ff 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -237,12 +237,6 @@ test_slru_shmem_request(void *arg)
 					 .name = "TestSLRU",
 					 .Dir = TestSlruDir,
 
-	/*
-	 * Short segments names are well tested elsewhere so in this test we are
-	 * focusing on long names.
-	 */
-					 .long_segment_names = true,
-
 					 .nslots = NUM_TEST_BUFFERS,
 					 .nlsns = 0,
 
-- 
2.43.0



^ permalink  raw  reply  [nested|flat] 9+ messages in thread

end of thread, other threads:[~2026-04-08 10:09 UTC | newest]

Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-01-15 12:42 Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
2025-02-25 14:18 ` Aleksander Alekseev <[email protected]>
2025-03-28 20:27   ` Rustam ALLAKOV <[email protected]>
2025-04-03 14:25     ` Aleksander Alekseev <[email protected]>
2025-08-13 11:54       ` Aleksander Alekseev <[email protected]>
2026-01-06 15:18         ` Aleksander Alekseev <[email protected]>
2026-01-06 15:28           ` Heikki Linnakangas <[email protected]>
2026-01-07 14:42             ` Aleksander Alekseev <[email protected]>
2026-04-08 10:09               ` Aleksander Alekseev <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox