my-ship-it opened a new issue, #1973:
URL: https://github.com/apache/cloudberry/issues/1973
### Apache Cloudberry version
main (`89baa48a4f4`, 2026-09-07) and local
`3.0.0-devel+dev.8551.gc781604c5ba` (PostgreSQL 16.9 base). The mismatch has
existed since the GPDB-era commits `b0208894eb5` / `68c0ff2d4ec` /
`b74f174b6cd` (2019) that extended `TwoPhaseFileHeader`.
### What happened
`ParsePrepareRecord()` in `src/backend/access/rmgrdesc/xactdesc.c` walks a
`XLOG_XACT_PREPARE` record using the layout of `xl_xact_prepare`
(`src/include/access/xact.h`). But the record is actually written by
`StartPrepare()`/`EndPrepare()` (`twophase.c`) as a `TwoPhaseFileHeader`
(`src/include/access/twophase_xlog.h`), and Cloudberry extended that struct
with fields that `xl_xact_prepare` never got:
```
TwoPhaseFileHeader (what is written, 96 bytes) xl_xact_prepare (what is
parsed, 72 bytes)
... ...
int32 nabortrels; int32 nabortrels;
int32 ncommitdbs; <- extra int32 ncommitstats;
int32 nabortdbs; <- extra int32 nabortstats;
int32 ncommitstats; int32 ninvalmsgs;
int32 nabortstats; bool initfileinval;
int32 ninvalmsgs; uint16 gidlen;
bool initfileinval; XLogRecPtr origin_lsn;
Oid tablespace_oid_to_delete_on_abort; <- extra TimestampTz
origin_timestamp;
Oid tablespace_oid_to_delete_on_commit; <- extra
uint16 gidlen;
XLogRecPtr origin_lsn;
TimestampTz origin_timestamp;
```
Consequences of reading through the wrong struct:
- `xlrec->gidlen` is read from offset 54, which is the upper half of the
real `nabortstats` (normally 0), so `parsed->twophase_gid` is an **empty
string**.
- `xlrec->ncommitstats` / `nabortstats` / `ninvalmsgs` actually read
`ncommitdbs` / `nabortdbs` / `ncommitstats`; `initfileinval` and `origin_lsn`
read from unrelated bytes.
- `bufptr` starts at offset 72 instead of 96, so `parsed->subxacts`,
`xlocators`, `abortlocators`, `stats`, `abortstats`, `msgs` all point at the
wrong bytes. The parser also never skips the `commitdbs` / `abortdbs`
(`DbDirNode`) arrays that `EndPrepare()` writes between `abortrels` and the
stats arrays.
Two user-visible symptoms:
1. **`pg_waldump`** prints an empty gid for every PREPARE record (every
distributed transaction on a segment):
```
rmgr: Transaction ... desc: PREPARE gid : 2026-09-09 05:06:49.215203 CST
rmgr: Transaction ... desc: COMMIT_PREPARED 5256: 2026-09-09
05:06:49.216803 CST gxid = 34413
```
(`COMMIT_PREPARED` is parsed by `ParseCommitRecord()` and is fine, which
shows the gid really is `34413` in the WAL.)
2. **Logical decoding with `two_phase`** emits `PREPARE TRANSACTION ''` with
an empty gid, while the matching `COMMIT PREPARED` carries the real gid. A
pgoutput subscriber would try to `PREPARE TRANSACTION ''`. Worse,
`DecodePrepare()` passes the shifted `parsed->subxacts` / `nsubxacts` into
`SnapBuildCommitTxn()` and `ReorderBufferPrepare()`, so decoding can consume
garbage subxact ids.
Crash recovery is not affected because `xact_redo()` for PREPARE goes
through `twophase.c`'s own `TwoPhaseFileHeader`-based parsing, not
`ParsePrepareRecord()`.
### What you think should happen instead
`xl_xact_prepare` must have the same layout as `TwoPhaseFileHeader` (the
upstream PostgreSQL comment in `twophase.c` says the two must stay in sync),
and `ParsePrepareRecord()` must skip the `commitdbs` / `abortdbs` arrays. Then
`pg_waldump` shows `PREPARE gid 34413: ...` and two-phase logical decoding
emits `PREPARE TRANSACTION '34413'`.
A static assert comparing `sizeof(xl_xact_prepare)` and
`sizeof(TwoPhaseFileHeader)` (or simply typedef-ing one to the other) would
keep this from regressing.
### How to reproduce
On a demo cluster with `wal_level = logical` (needed only for step 2; step 1
works with `replica`):
```sql
-- via coordinator
create table t2pc(id int primary key, v text) distributed by (id);
insert into t2pc select g, 'x' from generate_series(1,6) g; -- touches all
segments -> 2PC
```
1. `pg_waldump` on any primary segment:
```
pg_waldump -r Transaction <segdatadir>/pg_wal/<current wal file> | grep
-E 'PREPARE|COMMIT_PREPARED' | tail -2
```
shows `PREPARE gid : <timestamp>` (empty gid) followed by
`COMMIT_PREPARED ... gxid = <n>`.
2. two-phase logical decoding on a segment (utility mode):
```sql
-- PGOPTIONS='-c gp_role=utility' psql -p <segport>
select * from pg_create_logical_replication_slot('s2pc', 'test_decoding',
false, true);
-- run the INSERT above on the coordinator
select data from pg_logical_slot_peek_changes('s2pc', NULL, NULL,
'include-xids', '1');
```
output:
```
BEGIN 5256
table public.t2pc: INSERT: id[integer]:2 v[text]:'x'
...
PREPARE TRANSACTION '', txid 5256
COMMIT PREPARED '34413', txid 5256
```
### Operating System
CentOS 7 (kernel 3.10.0-1160), gcc 12.2.1
### Anything else
Found while prototyping an MPP logical-replication receiver, where the
segment-side gid (which is the distributed xid) is the key for aligning
per-segment streams. The fix is small and self-contained; happy to send a PR.
### Are you willing to submit PR?
- [x] Yes I am willing to submit a PR!
### Code of Conduct
- [x] I agree to follow this project's [Code of
Conduct](https://www.apache.org/foundation/policies/conduct)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
