From 3e60b8362eaa5fc50093c3fc59bc16773dda550a Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Tue, 9 Aug 2022 14:42:10 +1200 Subject: [PATCH v13 2/2] Harden errors on some unexpected filesystem conditions. CheckPointSnapBuild() shouldn't fail to unlink() files. Raise an ERROR, there's something wrong with the filesystem. There is also no reason to tolerate random stray files under pg_logical/snapshots. ReorderBufferCleanupSerializedTXNs() shouldn't be called for a non-directory, so raise an ERROR if opendir() fails, there's something wrong with the filesystem. Author: Nathan Bossart Reviewed-by: Thomas Munro Reviewed-by: Bharath Rupireddy Reviewed-by: Andres Freund Discussion: https://postgr.es/m/20220215231123.GA2584239%40nathanxps13 --- .../replication/logical/reorderbuffer.c | 2 +- src/backend/replication/logical/snapbuild.c | 18 ++---------------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index f2b6cdf473..a80cbacd9b 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -4413,7 +4413,7 @@ ReorderBufferCleanupSerializedTXNs(const char *slotname) sprintf(path, "pg_replslot/%s", slotname); spill_dir = AllocateDir(path); - while ((spill_de = ReadDirExtended(spill_dir, path, INFO)) != NULL) + while ((spill_de = ReadDir(spill_dir, path)) != NULL) { /* only look at names that can be ours */ if (strncmp(spill_de->d_name, "xid", 3) == 0) diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index 8587e2de43..bb788e1adc 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -1964,16 +1964,10 @@ CheckPointSnapBuild(void) * everything but are postfixed by .$pid.tmp. We can just remove them * the same as other files because there can be none that are * currently being written that are older than cutoff. - * - * 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) - { - ereport(LOG, + ereport(ERROR, (errmsg("could not parse file name \"%s\"", path))); - continue; - } lsn = ((uint64) hi) << 32 | lo; @@ -1982,19 +1976,11 @@ CheckPointSnapBuild(void) { elog(DEBUG1, "removing snapbuild snapshot %s", path); - /* - * It's not particularly harmful, though strange, if we can't - * remove the file here. Don't prevent the checkpoint from - * completing, that'd be a cure worse than the disease. - */ if (unlink(path) < 0) - { - ereport(LOG, + ereport(ERROR, (errcode_for_file_access(), errmsg("could not remove file \"%s\": %m", path))); - continue; - } } } FreeDir(snap_dir); -- 2.35.1