hardening(1.22): v1.0.4 — directory naming + allocator idempotency + L16 drop + WAL ABI lock

User 复审 v1.0.3 ship 后(commit dba746a)发现 4 项 finding 全 valid:
3 个 P1 + 1 个 P2。本 commit 一次 ship 4 类修订。

P1-1 (directory naming separation):
  v1.0.3 cluster_undo_path_resolve / ensure_undo_instance_subdir
  / build_undo_segment_path 直接用 owner_instance 当目录名,但
  initdb 用 instance_0/(目录名 = cluster_node_id 0)。两套约定:
    - initdb: 目录名 cluster_node_id (0..127); header 值
      owner_instance (1..128 = node_id + 1)
    - allocator (v1.0.3): 目录名 owner_instance (1..128) — 错
  单节点默认 (cluster_node_id=0, owner=1) → allocator 写到
  pg_undo/instance_1/ 与 seed 段 instance_0/ 完全脱节。

  v1.0.4 修法: 目录名 = cluster_node_id = owner_instance - 1;
  header / WAL payload 保留 owner_instance (sentinel 0 = unallocated)。
  Allocator 现在写 instance_0/seg_N.dat 与 seed 同子目录。
  代码改动: 3 处 path snprintf 加 (owner_instance - 1) 转目录编号;
  Assert 锁 owner_instance ∈ [1, MAX]。

P1-2 (allocator unconditional ftruncate):
  v1.0.3 只在 created=true 时 ftruncate。但 allocator crash 在
  O_CREAT 与 ftruncate 之间留下 orphan zero-size 文件 → 下次
  allocate 看到 existing file → created=false → 跳过 ftruncate
  → 文件永远不到 64MB → segment 中段 EOF。

  v1.0.4 修法: ftruncate 无条件触发(idempotent);加 fsync 父目录
  (与 redo handler 6 步幂等模式对称)。

P1-3 (drop TAP L16 + remove broken skip_all):
  L16 v1.0.3 假设删除 seed seg_0.dat 后重启 WAL replay 能重建。
  错: initdb 写 seed 直接 pg_pwrite 不 emit WAL (类 pg_control,
  initial cluster image),只有 cluster_undo_segment_allocate 才
  emit XLOG_UNDO_SEGMENT_INIT。L16 不可能 pass。

  另发现: 070_undo_tablespace.pl 顶部 plan skip_all 检查
  pg_config --configure 不工作,导致 CI run 25416097363 整个
  test 文件 "skipped: cluster build not enabled",L16 没真跑过 —
  CI 绿是假阳性。其他 TAP (060-069) 没此检查,直接用 Makefile-
  level 配置过滤。

  v1.0.4 修法:
    - 删除 broken plan skip_all 检查(让 070 在 CI 真跑)
    - 删 L16 4 个 assertions + 加 docstring 说明 seed 是 base
      image 不靠 WAL (类 pg_control corruption 必须 base-backup
      restore);redo 幂等性 defer 到 cluster_unit harness 通过
      direct call cluster_undo_segment_allocate (Stage 1.22 不
      暴露 SQL UDF,等 feature-117)

P2-1 (WAL ABI 文档错 + StaticAssertDecl):
  cluster_undo_xlog.h 注释 "payload = 6-byte fixed header" 错;
  实际 struct 因 segment_id uint32 alignment 落 offset 8 →
  size 12 byte。代码用 sizeof(hdr) emit/redo 一致,但文档 +
  cross-version replay invariant 缺。

  v1.0.4 修法:
    - 注释改 "12-byte fixed header"
    - 加 StaticAssertDecl(sizeof == 12) + offsetof(segment_id) == 8
    - Lessons SSOT L45 扩展(WAL ABI struct 也是 on-disk struct,
      需 per-field C alignment + StaticAssertDecl 锁)

Verification:
- PG 219/219 disable mode
- cluster_unit 31/31 binaries (含 21 spec-1.22 tests)
- cluster_regress 2/2
- L38 hygiene: clang-format 0 violations / comment-headers 0 errors /
  cppcheck baseline 47 不变
- initdb spot-check: pg_undo/instance_0/seg_0.dat (单节点默认正确
  落 instance_0/ 与 seed 一致)
- spec-1.22 ## Hardening v1.0.4 amend (pgrac amend pending separate
  commit)

Author: Yingjie Wang

src/backend/cluster/storage/cluster_undo_alloc.c

@@ -51,17 +51,33 @@
  * cluster_undo_path_resolve
  *
  *   Pure path builder (no I/O, no errors apart from buffer overflow).
+ *
+ *   Hardening v1.0.4 P1-1 (directory naming separation):
+ *     - The owner_instance VALUE in headers / WAL payloads is
+ *       (cluster_node_id + 1), so 0 stays the "unallocated" sentinel.
+ *     - The DIRECTORY NAME on disk uses cluster_node_id directly
+ *       (= owner_instance - 1) so that single-node default
+ *       (cluster_node_id = 0) lays out at pg_undo/instance_0/...
+ *       matching the initdb seed segment (also at instance_0/).
+ *       Otherwise allocator segments would land at instance_1/ and
+ *       split from the seed, breaking the per-instance subdir
+ *       invariant.
+ *
+ *   Caller MUST pass owner_instance in [1, UNDO_OWNER_INSTANCE_MAX];
+ *   the assert catches sentinel-0 misuse.
  */
 int
-cluster_undo_path_resolve(uint8 instance, uint32 segment_id, char *buf, size_t buf_size)
+cluster_undo_path_resolve(uint8 owner_instance, uint32 segment_id, char *buf, size_t buf_size)
 {
 	int ret;
 
 	if (buf == NULL || buf_size == 0)
 		return -1;
+	Assert(owner_instance >= 1 && owner_instance <= UNDO_OWNER_INSTANCE_MAX);
 
-	ret = snprintf(buf, buf_size, "%s/pg_undo/instance_%u/seg_%u.dat", DataDir, (unsigned)instance,
-				   (unsigned)segment_id);
+	/* directory uses cluster_node_id (= owner_instance - 1) */
+	ret = snprintf(buf, buf_size, "%s/pg_undo/instance_%u/seg_%u.dat", DataDir,
+				   (unsigned)(owner_instance - 1), (unsigned)segment_id);
 	if (ret < 0 || (size_t)ret >= buf_size)
 		return -1;
 	return 0;
@@ -111,16 +127,21 @@ open_or_create_segment(const char *path, bool *out_created)
  * rejects it, but if a future caller does, mkdir() handles it.
  */
 static void
-ensure_instance_subdir(uint8 instance)
+ensure_instance_subdir(uint8 owner_instance)
 {
 	char path[MAXPGPATH];
 	int ret;
 
-	ret = snprintf(path, sizeof(path), "%s/pg_undo/instance_%u", DataDir, (unsigned)instance);
+	Assert(owner_instance >= 1 && owner_instance <= UNDO_OWNER_INSTANCE_MAX);
+
+	/* directory uses cluster_node_id (= owner_instance - 1); see
+	 * cluster_undo_path_resolve docstring. */
+	ret = snprintf(path, sizeof(path), "%s/pg_undo/instance_%u", DataDir,
+				   (unsigned)(owner_instance - 1));
 	if (ret < 0 || (size_t)ret >= sizeof(path))
-		ereport(ERROR,
-				(errcode(ERRCODE_NAME_TOO_LONG),
-				 errmsg("undo instance subdir path too long: instance=%u", (unsigned)instance)));
+		ereport(ERROR, (errcode(ERRCODE_NAME_TOO_LONG),
+						errmsg("undo instance subdir path too long: owner_instance=%u",
+							   (unsigned)owner_instance)));
 
 	if (mkdir(path, S_IRWXU) < 0 && errno != EEXIST)
 		ereport(ERROR, (errcode_for_file_access(),
@@ -171,24 +192,30 @@ cluster_undo_segment_allocate(uint32 segment_id, uint8 owner_instance)
 	ensure_instance_subdir(owner_instance);
 
 	fd = open_or_create_segment(path, &created);
+	(void)created; /* tracked for logging/observability; ftruncate is unconditional */
 
 	cluster_undo_segment_make_header_bytes(segment_id, owner_instance, page.data);
 
 	/*
-	 * Extend file to UNDO_SEGMENT_SIZE_BYTES on first creation.  ftruncate
-	 * gives a sparse file (tail bytes read as zero); the allocator path
-	 * writes actual undo records lazily later.
+	 * Hardening v1.0.4 P1-2: unconditional ftruncate.
+	 *
+	 * v1.0.3 only ftruncate'd when created=true, but a crash between
+	 * O_CREAT and ftruncate leaves a partial / zero-size orphan file.
+	 * The next allocate call sees the existing file, so created=false,
+	 * and the file would never reach UNDO_SEGMENT_SIZE_BYTES -- segment
+	 * tail accesses would EOF.  ftruncate is idempotent (shrink-to-same
+	 * and extend-to-target are both no-ops when size already matches),
+	 * so unconditional is safe and self-healing.  Mirrors the redo
+	 * handler's 6-step idempotent pattern (cluster_undo_xlog.c).
 	 */
-	if (created) {
-		if (ftruncate(fd, (off_t)UNDO_SEGMENT_SIZE_BYTES) != 0) {
-			int save_errno = errno;
-
-			close(fd);
-			errno = save_errno;
-			ereport(ERROR, (errcode_for_file_access(),
-							errmsg("could not extend undo segment file \"%s\" to %d bytes: %m",
-								   path, UNDO_SEGMENT_SIZE_BYTES)));
-		}
+	if (ftruncate(fd, (off_t)UNDO_SEGMENT_SIZE_BYTES) != 0) {
+		int save_errno = errno;
+
+		close(fd);
+		errno = save_errno;
+		ereport(ERROR, (errcode_for_file_access(),
+						errmsg("could not extend undo segment file \"%s\" to %d bytes: %m", path,
+							   UNDO_SEGMENT_SIZE_BYTES)));
 	}
 
 	written = pg_pwrite(fd, page.data, BLCKSZ, 0);
@@ -216,6 +243,21 @@ cluster_undo_segment_allocate(uint32 segment_id, uint8 owner_instance)
 		ereport(ERROR, (errcode_for_file_access(),
 						errmsg("could not close undo segment file \"%s\": %m", path)));
 
+	/*
+	 * Hardening v1.0.4 P1-2: fsync parent directory after file creation /
+	 * truncate.  Required for the create case so the dirent is durable;
+	 * harmless for the already-exists case.  Mirrors redo handler.
+	 */
+	{
+		char dir[MAXPGPATH];
+		int dret;
+
+		dret = snprintf(dir, sizeof(dir), "%s/pg_undo/instance_%u", DataDir,
+						(unsigned)(owner_instance - 1));
+		if (dret >= 0 && (size_t)dret < sizeof(dir))
+			fsync_fname(dir, true);
+	}
+
 	/*
 	 * Emit WAL record so crash recovery can recreate the header (the
 	 * segment file itself is created earlier in this function; a crash

src/backend/cluster/storage/cluster_undo_xlog.c

@@ -60,16 +60,23 @@
  *   only instance_0 from initdb; redo path may need other instance
  *   subdirs on standbys / cross-instance crash recovery).
  *
+ *   Hardening v1.0.4 P1-1: directory naming uses cluster_node_id
+ *   (= owner_instance - 1) so that single-node default
+ *   (cluster_node_id = 0, owner_instance = 1) lands at instance_0/
+ *   matching the initdb seed.  See cluster_undo_alloc.c
+ *   cluster_undo_path_resolve docstring for full rationale.
+ *
  *   Returns 0 on success, -1 on path-too-long.  Caller supplies
  *   buf with capacity >= MAXPGPATH.
  */
 static int
-build_undo_segment_path(uint8 instance, uint32 segment_id, char *buf, size_t buf_size)
+build_undo_segment_path(uint8 owner_instance, uint32 segment_id, char *buf, size_t buf_size)
 {
 	int ret;
 
-	ret = snprintf(buf, buf_size, "%s/pg_undo/instance_%u/seg_%u.dat", DataDir, (unsigned)instance,
-				   (unsigned)segment_id);
+	Assert(owner_instance >= 1 && owner_instance <= UNDO_OWNER_INSTANCE_MAX);
+	ret = snprintf(buf, buf_size, "%s/pg_undo/instance_%u/seg_%u.dat", DataDir,
+				   (unsigned)(owner_instance - 1), (unsigned)segment_id);
 	if (ret < 0 || (size_t)ret >= buf_size)
 		return -1;
 	return 0;
@@ -88,15 +95,19 @@ build_undo_segment_path(uint8 instance, uint32 segment_id, char *buf, size_t buf
  *   established by initdb (D4) and assumed to exist.
  */
 static void
-ensure_undo_instance_subdir(uint8 instance)
+ensure_undo_instance_subdir(uint8 owner_instance)
 {
 	char path[MAXPGPATH];
 	int ret;
 
-	ret = snprintf(path, sizeof(path), "%s/pg_undo/instance_%u", DataDir, (unsigned)instance);
+	Assert(owner_instance >= 1 && owner_instance <= UNDO_OWNER_INSTANCE_MAX);
+
+	/* directory uses cluster_node_id (= owner_instance - 1) per Hardening v1.0.4 P1-1 */
+	ret = snprintf(path, sizeof(path), "%s/pg_undo/instance_%u", DataDir,
+				   (unsigned)(owner_instance - 1));
 	if (ret < 0 || (size_t)ret >= sizeof(path))
-		ereport(PANIC,
-				(errmsg("undo instance subdir path too long: instance=%u", (unsigned)instance)));
+		ereport(PANIC, (errmsg("undo instance subdir path too long: owner_instance=%u",
+							   (unsigned)owner_instance)));
 
 	if (mkdir(path, S_IRWXU) != 0 && errno != EEXIST)
 		ereport(PANIC, (errcode_for_file_access(),
@@ -245,8 +256,9 @@ cluster_undo_redo_segment_init(XLogReaderState *record)
 		char dir[MAXPGPATH];
 		int dret;
 
+		/* directory uses cluster_node_id (= owner_instance - 1) per Hardening v1.0.4 P1-1 */
 		dret = snprintf(dir, sizeof(dir), "%s/pg_undo/instance_%u", DataDir,
-						(unsigned)hdr->instance);
+						(unsigned)(hdr->instance - 1));
 		if (dret >= 0 && (size_t)dret < sizeof(dir))
 			fsync_fname(dir, true);
 	}

src/include/cluster/storage/cluster_undo_xlog.h

@@ -55,23 +55,45 @@
 /*
  * On-disk WAL payload layout for XLOG_UNDO_SEGMENT_INIT.
  *
- *   payload = 6-byte fixed header + BLCKSZ-byte page image.
+ *   payload = 12-byte fixed header + BLCKSZ-byte page image.
  *   Stored sequentially in a single XLogRegisterData block.
  *
  *   v0.2 P1-A: payload uses a custom path-based identification
  *   (instance + segment_id) rather than RelFileLocator/BlockNumber
  *   because pg_undo/ files live outside PG's relfilenode namespace.
  *   redo handler resolves the path via cluster_undo_path_resolve()
  *   and pwrites directly.
+ *
+ *   Hardening v1.0.4 P2-1: header is 12 bytes (not the "6-byte"
+ *   v1.0.3 doc claimed); _pad[1] + _pad2[2] add up to 5 padding bytes
+ *   that round segment_id to its uint32 8-byte alignment slot
+ *   (1 + 1 + 4 + 4 = 10 bytes data; layout pads to offset 8 for
+ *   segment_id, total struct size 12).  StaticAssertDecl below locks
+ *   the on-disk WAL ABI; future maintainers see the assert fire if
+ *   a field add / reorder breaks cross-version replay.
  */
 typedef struct xl_cluster_undo_segment_init {
-	uint8 instance;	   /* owner instance (1..128); spec-1.21 Q5 */
-	uint8 _pad[1];	   /* alignment pad */
-	uint16 _pad2[2];   /* alignment pad to 8 bytes */
+	uint8 instance;	   /* offset 0; 1 byte; owner instance (1..128); spec-1.21 Q5 */
+	uint8 _pad[1];	   /* offset 1; 1 byte; alignment pad */
+	uint16 _pad2[2];   /* offset 2; 4 bytes; pad up to uint32 alignment */
 	uint32 segment_id; /* offset 8; 4 bytes */
 					   /* Followed by char page_image[BLCKSZ] (segment header block 0). */
 } xl_cluster_undo_segment_init;
 
+/*
+ * WAL ABI invariants (Hardening v1.0.4 P2-1; lessons SSOT L45 ext).
+ *
+ *   StaticAssertDecl locks struct size + segment_id offset so cross-
+ *   version replay (mixed 1.22 / 1.23+ binaries) catches incompatible
+ *   field add / reorder at compile time.
+ */
+StaticAssertDecl(sizeof(xl_cluster_undo_segment_init) == 12,
+				 "xl_cluster_undo_segment_init must be exactly 12 bytes "
+				 "(WAL ABI lock; Hardening v1.0.4 P2-1)");
+StaticAssertDecl(offsetof(xl_cluster_undo_segment_init, segment_id) == 8,
+				 "xl_cluster_undo_segment_init.segment_id must be at offset 8 "
+				 "(uint32 8-byte alignment slot; Hardening v1.0.4 P2-1)");
+
 
 /*
  * Public API.

src/test/cluster_tap/t/070_undo_tablespace.pl

@@ -52,15 +52,12 @@
 use File::Path qw(make_path);
 use File::Spec;
 
-# Skip if cluster build is disabled.
-if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\bcluster\b/)
-{
-	if (`@{[$ENV{PG_CONFIG} || 'pg_config']} --configure 2>/dev/null` !~ /--enable-pgrac-cluster/)
-	{
-		plan skip_all => 'cluster build not enabled';
-	}
-}
-
+# Hardening v1.0.4 P1-3: removed plan skip_all check.
+# v1.0.3 had a "skip if --enable-pgrac-cluster not configured" guard
+# that was triggering "skipped: cluster build not enabled" on every
+# CI run, masking real test failures.  Other cluster_tap tests
+# (060-069) don't gate on this -- the cluster_tap Makefile-level
+# check is sufficient.
 
 my $node = PostgreSQL::Test::Cluster->new('spec122_undo_tbs');
 $node->init;
@@ -243,43 +240,25 @@
 
 
 # ----------
-# L16: redo handler idempotent file/dir/size restore (Hardening v1.0.3 P1-B).
-# Stop server -> delete seg_0.dat (simulate operator error / standby-with-no-
-# allocator-history) -> start server.  Server should NOT FATAL; WAL replay
-# should rebuild the segment file (mkdir + O_CREAT + ftruncate + pwrite +
-# fsync), restoring the byte-perfect header.
+# L16: REMOVED in Hardening v1.0.4 P1-3.
+#
+# v1.0.3 attempted "rm seg_0.dat -> restart -> WAL replay rebuilds it"
+# but this premise is wrong: the initdb seed segment is part of the
+# initial cluster image (similar to pg_control / template1), not
+# WAL-protected.  initdb writes the seed via libpgport pg_pwrite
+# directly with no WAL emit; the only WAL records that could rebuild
+# a segment are emitted by cluster_undo_segment_allocate (backend-only
+# path), which never touched the seed.  Deleting the seed file is
+# corruption equivalent to deleting pg_control -- nothing in the
+# recovery path can heal it; users must base-backup-restore.  Redo
+# idempotency (the actual Hardening v1.0.3 P1-B fix) is exercised
+# instead by allocator-created segments.  Stage 1.22 doesn't expose
+# a SQL UDF for cluster_undo_segment_allocate, so the redo idempotency
+# is verified by a future cluster_unit harness once feature-117 lands
+# a public allocator API.
+#
+# Spec: spec-1.22-undo-tablespace-bootstrap.md ## Hardening v1.0.4.
 # ----------
-$node->stop;
-
-# Read pre-deletion bytes for byte-equality check after replay.
-open(my $pre_del_fh, '<:raw', $seed_path)
-	or die "could not open seed pre-deletion: $!";
-my $pre_del_bytes;
-read $pre_del_fh, $pre_del_bytes, 128;
-close $pre_del_fh;
-
-# Delete the seed segment file.
-unlink($seed_path) or die "could not unlink $seed_path: $!";
-ok(! -f $seed_path, 'L16a seg_0.dat deleted');
-
-# Restart -- WAL replay should rebuild the segment file.
-$node->start;
-
-ok(-f $seed_path, 'L16b redo handler recreated seg_0.dat after deletion');
-
-my $post_del_size = -s $seed_path;
-is($post_del_size, 64 * 1024 * 1024,
-	'L16c redo handler restored seg_0.dat size to 64 MB (ftruncate idempotent)');
-
-open(my $post_del_fh, '<:raw', $seed_path)
-	or die "could not open seed post-replay: $!";
-my $post_del_bytes;
-read $post_del_fh, $post_del_bytes, 128;
-close $post_del_fh;
-
-is($post_del_bytes, $pre_del_bytes,
-	'L16d redo handler restored byte-perfect block 0 (segment_id / state / owner_instance unchanged)');
-
 
 
 # ----------