Hi, I tested v3 against two questions: whether 0003 needs a gap in between, and whether 0005 really leaves no garbage in the WAL. Everything below can be rerun with the attached files.
0003: who uses PGAlignedXLogBlock out of tree
---------------------------------------------
A GitHub code search (default branches only, so a lower bound), leaving
out forks that carry their own c.h and copies of the in-core files,
finds it in percona/pg_tde (pg_tde_archive_decrypt.c,
pg_tde_restore_encrypt.c, fetools/pg16..pg19/pg_rewind/tde_ops.c),
commandprompt/open_pg_tde, bdrouvot/pg_wal_fp_extract,
wublabdubdub/pg_flashback, 3Davydov/WAL-DIFF,
ApsaraDB/PolarDB-BackupAgent and OpenTeleDB. All of them use it as a
page buffer for plain read/write on WAL segments; none of those
repositories uses O_DIRECT in its own code. For example, pg_tde:
PGAlignedXLogBlock buf;
...
r = read(tmpfd, buf.data, XLOG_BLCKSZ);
and WAL-DIFF opens its file with O_RDONLY | PG_BINARY.
To see what each option does to that code, I compiled
xlogblock_user.c (attached, the same pattern in 30 lines, with an
optional O_DIRECT read of the same buffer) against 19beta2, against
master plus 0001 only, and against master plus 0001-0005:
gcc -D_GNU_SOURCE -I$(pg_config --includedir-server) xlogblock_user.c \
-L$(pg_config --libdir) -lpgcommon -lpgport
19beta2:
alignof(PGAlignedXLogBlock) = 4096, buf % 4096 = 0
pread: 8192 bytes
pread with O_DIRECT: 8192 bytes
0001 only:
error: unknown type name 'PGAlignedXLogBlock'; did you mean
'PGIOAlignedXLogBlock'?
0001-0005, file on ext4:
alignof(PGAlignedXLogBlock) = 8, buf % 4096 = 2720
pread: 8192 bytes
alignof(PGAlignedXLogBlock) = 8, buf % 4096 = 336
pread with O_DIRECT: Invalid argument
0001-0005, same binary, file on btrfs:
pread with O_DIRECT: 8192 bytes
So both sides of your hesitation are real. 0003 keeps the code I found
compiling unchanged, and that code only does plain I/O, which works.
But code doing direct I/O through the type would also compile silently
and then fail with EINVAL, and only on some file systems: btrfs accepts
the misaligned buffer, ext4 rejects it. I found no such code, but a
failure that depends on the file system would be hard to trace back to
this change. With 0001 alone, every user above gets a compile error that
names the replacement.
0005: padding bytes in the WAL
------------------------------
XLogRecord has a two-byte hole at offset 18, between xl_rmid and xl_crc
(gdb "ptype /o struct XLogRecord" on the build). wal_padding.py
(attached) takes every record that pg_waldump lists, reads those two
bytes straight from the segment file, and counts the non-zero ones.
Workload: pgbench -i -s 5, pgbench -c 4 -t 2000, CREATE INDEX,
VACUUM ANALYZE, CHECKPOINT (wal_run.sh, attached).
19beta2: 0 of 67595 records non-zero
0001-0005: 0 of 67716 records non-zero
0001-0005 without the new
memset in 0005: 16033 of 67535 records non-zero
(e.g. lsn 0/0502C978 -> 8fc3)
The third run is there to show that the check does catch garbage: the
memset that 0005 adds is what keeps the WAL identical to before. The
new workspace is 952 bytes on the stack (sizeof(struct
XLogRecordHeaderScratch)).
The rest: v3 0001-0005 applies to master at dfb474ca6d8, builds with
--enable-cassert without warnings, and all 239 regression tests pass.
Regards,
Manu
El mié, 16 sept 2026 a las 6:16, Peter Eisentraut
(<[email protected]>) escribió:
>
> On 08.09.26 12:08, Peter Eisentraut wrote:
> > There are a number of places where palloc()/malloc()/etc. was used
> > solely to obtain an aligned buffer. We can do these much simpler by
> > using alignas with a local variable instead. See attached patch.
>
> Here is a new patch set that aims to address all the comments.
>
> First of all, while changing this to make use of the existing
> "AlignedBlock" types, I noticed that PGAlignedXLogBlock is misnamed: It
> should be PGIOAlignedXLogBlock, to maintain the similarity with
> PGAlignedBlock and PGIOAlignedBlock, respectively. So I'm proposing to
> rename it in patch 0001.
>
> We could then re-introduce the "correct" PGAlignedXLogBlock and make use
> of it, which is patch 0003. But I'm hesitant to change the meaning of
> PGAlignedXLogBlock without some gap in between, so I'm not sure about
> this patch.
>
> Patch 0002 is as before, but with the "AlignedBlock" types used, and the
> copy_file() change backed out and a comment added.
>
> Patch 0004 adds some comments and an assertion for HEADER_SCRATCH_SIZE,
> and patch 0005 refactors things to convert the workspace from static
> variable to a normal (non-static) local variable. (This could be
> squashed into 0002, but it seems cleaner to review this way at least.)
>
> (The pgindent changes were already committed separately.)
wal_run.sh
Description: application/shellscript
"""Read the two padding bytes of every XLogRecord header in a WAL directory.
XLogRecord has a 2-byte hole at offset 18 (between xl_rmid and xl_crc).
For each record listed by pg_waldump, read those bytes straight from the
segment file and count how many records have them non-zero.
"""
import re
import subprocess
import sys
from pathlib import Path
WAL_SEG = 16 * 1024 * 1024
PAGE = 8192
HOLE_OFF, HOLE_LEN, HDR = 18, 2, 24
waldump, waldir = sys.argv[1], Path(sys.argv[2])
segments = sorted(p for p in waldir.iterdir() if re.fullmatch(r"[0-9A-F]{24}", p.name))
total = skipped = nonzero = 0
examples = []
for seg in segments:
out = subprocess.run([waldump, "-p", str(waldir), seg.name],
capture_output=True, text=True).stdout
data = seg.read_bytes()
for m in re.finditer(r"lsn: ([0-9A-F]+)/([0-9A-F]+)", out):
lsn = (int(m.group(1), 16) << 32) | int(m.group(2), 16)
off = lsn % WAL_SEG
if off % PAGE + HDR > PAGE:
skipped += 1
continue
total += 1
pad = data[off + HOLE_OFF: off + HOLE_OFF + HOLE_LEN]
if pad != b"\0\0":
nonzero += 1
if len(examples) < 3:
examples.append(f"{m.group(1)}/{m.group(2)} -> {pad.hex()}")
print(f"records checked: {total} (skipped, header across a page boundary: {skipped})")
print(f"records with non-zero padding: {nonzero}")
for e in examples:
print(f" e.g. lsn {e}")
/*
* The pattern used out of tree (e.g. percona/pg_tde
* src/bin/pg_tde_archive_decrypt.c): a PGAlignedXLogBlock as a page buffer
* for plain I/O on a WAL segment. With an argument, the same buffer is used
* with O_DIRECT, the case that needs PG_IO_ALIGN_SIZE.
*/
#include "postgres_fe.h"
#include <fcntl.h>
#include <unistd.h>
int
main(int argc, char **argv)
{
PGAlignedXLogBlock buf;
bool direct = argc > 2;
int fd;
ssize_t r;
printf("alignof(PGAlignedXLogBlock) = %zu, buf %% 4096 = %zu\n",
alignof(PGAlignedXLogBlock), (size_t) ((uintptr_t) buf.data % 4096));
fd = open(argv[1], O_RDONLY | (direct ? O_DIRECT : 0));
if (fd < 0)
{
perror("open");
return 1;
}
r = pread(fd, buf.data, sizeof(buf.data), 0);
if (r < 0)
printf("pread%s: %m\n", direct ? " with O_DIRECT" : "");
else
printf("pread%s: %zd bytes\n", direct ? " with O_DIRECT" : "", r);
close(fd);
return 0;
}
