On Tue, Aug 12, 2025 at 05:57:45PM +0900, Fujii Masao wrote:
> On Tue, Aug 12, 2025 at 4:38 PM Japin Li <japi...@hotmail.com> wrote:
> >
> > On Tue, Aug 12, 2025 at 12:24:10PM +0530, shveta malik wrote:
> > > It looks like commit 2633dae (mistakenly) introduced a change ([1]) in
> > > SnapBuildSnapshotExists(), altering the format used for snapshot file
> > > names during the search. However, SnapBuildSerialize still uses the
> > > old format ("%s/%X-%X.snap"), which led to the slot-sync worker being
> > > unable to locate existing snapshot files.
>
> Thanks for the investigation!
>
>
> > Sorry, it's my fault. I forgot to update the format in
> > SnapBuildSerialize().
> >
> > I'd prefer to update the format in SnapBuildSerialize() instead of reverting
> > this change.
>
> +1.
> If we make this change, we should also update other places using "%X-%X"?
>
Agreed.
> $ git grep -E "%X-%X.(snap|tmp)"
> contrib/pg_logicalinspect/pg_logicalinspect.c: if (sscanf(filename,
> "%X-%X.snap", &hi, &lo) != 2)
> contrib/pg_logicalinspect/pg_logicalinspect.c: sprintf(tmpfname,
> "%X-%X.snap", hi, lo);
> src/backend/replication/logical/snapbuild.c: sprintf(path, "%s/%X-%X.snap",
> src/backend/replication/logical/snapbuild.c: sprintf(tmppath,
> "%s/%X-%X.snap.%d.tmp",
> src/backend/replication/logical/snapbuild.c: sprintf(path, "%s/%X-%X.snap",
> src/backend/replication/logical/snapbuild.c: if
> (sscanf(snap_de->d_name, "%X-%X.snap", &hi, &lo) != 2)
>
I believe that the format %X-%X also works with sscanf(). However, to maintain
consistency, the format for sscanf() has been updated as well.
--
Best regards,
Japin Li
ChengDu WenWu Information Technology Co., LTD.
>From 63b7214b8f66266daff4907fab5b5ff4ff0bb13a Mon Sep 17 00:00:00 2001
From: Japin Li <japi...@hotmail.com>
Date: Tue, 12 Aug 2025 17:14:46 +0800
Subject: [PATCH v2] Standardize snapshot filename formatting
---
contrib/pg_logicalinspect/pg_logicalinspect.c | 4 ++--
src/backend/replication/logical/snapbuild.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/contrib/pg_logicalinspect/pg_logicalinspect.c b/contrib/pg_logicalinspect/pg_logicalinspect.c
index 50e805d3195..fe88857fda1 100644
--- a/contrib/pg_logicalinspect/pg_logicalinspect.c
+++ b/contrib/pg_logicalinspect/pg_logicalinspect.c
@@ -69,7 +69,7 @@ parse_snapshot_filename(const char *filename)
* check including the suffix. The subsequent check validates if the given
* filename has the expected suffix.
*/
- if (sscanf(filename, "%X-%X.snap", &hi, &lo) != 2)
+ if (sscanf(filename, "%08X-%08X.snap", &hi, &lo) != 2)
goto parse_error;
/*
@@ -77,7 +77,7 @@ parse_snapshot_filename(const char *filename)
* to the given filename. This check strictly checks if the given filename
* follows the format of the snapshot filename.
*/
- sprintf(tmpfname, "%X-%X.snap", hi, lo);
+ sprintf(tmpfname, "%08X-%08X.snap", hi, lo);
if (strcmp(tmpfname, filename) != 0)
goto parse_error;
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 8532bfd27e5..b13ceae694a 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1529,7 +1529,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
* unless the user used pg_resetwal or similar. If a user did so, there's
* no hope continuing to decode anyway.
*/
- sprintf(path, "%s/%X-%X.snap",
+ sprintf(path, "%s/%08X-%08X.snap",
PG_LOGICAL_SNAPSHOTS_DIR,
LSN_FORMAT_ARGS(lsn));
@@ -1573,7 +1573,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
elog(DEBUG1, "serializing snapshot to %s", path);
/* to make sure only we will write to this tempfile, include pid */
- sprintf(tmppath, "%s/%X-%X.snap.%d.tmp",
+ sprintf(tmppath, "%s/%08X-%08X.snap.%d.tmp",
PG_LOGICAL_SNAPSHOTS_DIR,
LSN_FORMAT_ARGS(lsn), MyProcPid);
@@ -1747,7 +1747,7 @@ SnapBuildRestoreSnapshot(SnapBuildOnDisk *ondisk, XLogRecPtr lsn,
Size sz;
char path[MAXPGPATH];
- sprintf(path, "%s/%X-%X.snap",
+ sprintf(path, "%s/%08X-%08X.snap",
PG_LOGICAL_SNAPSHOTS_DIR,
LSN_FORMAT_ARGS(lsn));
@@ -2019,7 +2019,7 @@ CheckPointSnapBuild(void)
* We just log a message if a file doesn't fit the pattern, it's
* probably some editors lock/state file or similar...
*/
- if (sscanf(snap_de->d_name, "%X-%X.snap", &hi, &lo) != 2)
+ if (sscanf(snap_de->d_name, "%08X-%08X.snap", &hi, &lo) != 2)
{
ereport(LOG,
(errmsg("could not parse file name \"%s\"", path)));
--
2.43.0
