public inbox for [email protected]
help / color / mirror / Atom feedFrom: Aleksander Alekseev <[email protected]>
To: [email protected]
Cc: Heikki Linnakangas <[email protected]>
Cc: Rustam ALLAKOV <[email protected]>
Cc: Michael Paquier <[email protected]>
Subject: Re: [PATCH] Refactor SLRU to always use long file names
Date: Wed, 8 Apr 2026 13:09:38 +0300
Message-ID: <CAJ7c6TN9Lo_EKSPpgrSJRDKW=NTh9K5k-ve1v7FMiz-0bQSxGg@mail.gmail.com> (raw)
In-Reply-To: <CAJ7c6TP_k+s6K2fdi4No-a7t+TZqkV7GUfbwQeDggfaWRj3zGA@mail.gmail.com>
References: <CAJ7c6TOy7fUW9MuNeOWor3cSFnQg9tgz=mjXHDb94GORtM_Eyg@mail.gmail.com>
<CAJ7c6TNu+5m2gBgRoSQ6jEJHzuxDf7JsZy9Y6K67ZynyZ5sfKg@mail.gmail.com>
<174319365391.60294.17413945943923890743.pgcf@coridan.postgresql.org>
<CAJ7c6TP6aNSL3Q33wuqOPf1LxgxCmm1q-u4OVH5RksB49W_kFQ@mail.gmail.com>
<CAJ7c6TMQUp5p0njUtJR37J7Pg4Gry0FDAVzOL50tT1HX=-BKrQ@mail.gmail.com>
<CAJ7c6TNqaK3R52KiSaR78p7OkC=kccO4FHZaff7F9ijz36ihsw@mail.gmail.com>
<[email protected]>
<CAJ7c6TP_k+s6K2fdi4No-a7t+TZqkV7GUfbwQeDggfaWRj3zGA@mail.gmail.com>
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
view thread (9+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Refactor SLRU to always use long file names
In-Reply-To: <CAJ7c6TN9Lo_EKSPpgrSJRDKW=NTh9K5k-ve1v7FMiz-0bQSxGg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox