Re: Use C11 alignas instead of palloc/malloc for alignment

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
(<peter(at)eisentraut(dot)org>) 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.)