Use the raw xmin when looking up a local row's commit timestamp by mason-sharp · Pull Request #674 · pgEdge/spock

mason-sharp · 2026-10-22T00:47:44Z

Fixes #672.

Problem

get_tuple_origin() read the local row's xmin with HeapTupleHeaderGetXmin(),
which returns FrozenTransactionId once the row is frozen. The non-normal xid
branch then reported a commit timestamp of 0, so under last_update_wins any
incoming change won against a frozen row, even one older than the local
change. Since PostgreSQL 16, VACUUM freezes whole pages early when it writes a
full-page image anyway, so recently written rows can be frozen and nodes can
end up with different values.

Fix

Freezing only sets infomask bits and keeps the original xid, so read it with
HeapTupleHeaderGetRawXmin(). This is what PostgreSQL's own conflict
detection gets through heap_getsysattr(). The commit timestamp stays
available until commit_ts is truncated; after that the lookup fails and the
existing fallback applies. The guard for a non-normal xid stays, for rows
frozen before 9.4 and carried over by pg_upgrade.

Testing

New TAP test 116_lww_frozen_local_row: an older UPDATE from n1 arrives on
n2 after n2 has updated the same row and frozen it (confirmed with
pageinspect); n2 must keep its value. Fails without the fix, passes with
it.
Builds clean on PG 15, 16, 17 and 18; make regresscheck passes.

coderabbitai · 2026-10-22T00:48:04Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: aea5dbd3-47fc-4f79-abee-31453872b069

📥 Commits

Reviewing files that changed from the base of the PR and between 6b72e69 and d2d9c4d.

📒 Files selected for processing: 2

tests/tap/schedule
tests/tap/t/116_lww_frozen_local_row.pl

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

Conflict resolution now reads a tuple’s raw xmin for commit-timestamp lookup. A scheduled TAP regression test checks last-update-wins behavior after a local row is frozen and replication resumes.

Changes: Frozen-row conflict resolution

Layer / File(s)	Summary
Raw xmin lookup `src/spock_conflict.c`	`get_tuple_origin` reads the raw xmin for commit-timestamp lookup. The non-normal-xid comment identifies pre-9.4 rows carried through `pg_upgrade`.
Frozen-row regression `tests/tap/t/116_lww_frozen_local_row.pl`, `tests/tap/schedule`	The TAP test pauses replication, updates the row on both nodes, and retries `VACUUM (FREEZE)` until it detects the frozen bits. It checks the raw xmin, resumes replication, and verifies that the local value remains `from_n2`. The schedule includes the test.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to d2d9c

No identified issue remains that should delay merging this frozen-row fix.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main code change: using the raw xmin for local row commit-timestamp lookup.
Description check		The description directly explains the frozen-row conflict, the raw xmin fix, and the regression test coverage. It is related to the changeset.
Linked Issues check		Issue `#672` requires raw xmin lookup, the non-normal-xid guard, the existing fallback after commit-timestamp truncation, and TAP coverage for a frozen local row. `src/spock_conflict.c` uses `HeapTupleH…
Out of Scope Changes check		The supplied whole-PR summary identifies changes in `src/spock_conflict.c`, `tests/tap/schedule`, and the focused TAP test. The source change and test directly implement issue `#672`. No unrelated chang…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 …

Full details: Linked Issues check

Explanation

Issue #672 requires raw xmin lookup, the non-normal-xid guard, the existing fallback after commit-timestamp truncation, and TAP coverage for a frozen local row. src/spock_conflict.c uses HeapTupleHeaderGetRawXmin() and retains the non-normal-xid guard and failed commit-timestamp lookup path. tests/tap/t/116_lww_frozen_local_row.pl freezes the local row, verifies its raw xmin, applies an older remote update, and verifies that the newer local value remains. tests/tap/schedule schedules the test.

Full details: Out of Scope Changes check

Explanation

The supplied whole-PR summary identifies changes in src/spock_conflict.c, tests/tap/schedule, and the focused TAP test. The source change and test directly implement issue #672. No unrelated change is established.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{A rabbit checks the xmin trail,
While frozen rows keep timestamps pale.
Two nodes pause, then sync once more,
The local value holds its score.
The test hops through each careful state,
And checks the row before it’s late.}

_{Comment @coderabbitai help to get the list of available commands.}

codacy-production · 2026-10-22T00:48:46Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric Results

Duplication 0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

ibrarahmad

The fix is right and matches core: heap_getsysattr() returns HeapTupleHeaderGetRawXmin() for xmin (heaptuple.c:741), and PG 18's conflict.c:84 feeds exactly that to TransactionIdGetCommitTsData(). The non-normal guard still covers rows frozen before 9.4, and the commit_ts lookup returns false with ts = 0 outside the retained range rather than erroring, so the fallback is as described. This is the only place in src/ that read xmin this way. I ran the test on PG 18 with pageinspect installed: 14/14 with the fix, and with the one line reverted test 13 fails with got from_n1, expected from_n2. Two notes on the test inline.

ibrarahmad · 2026-10-22T07:36:53Z

+my $db_password = $config->{db_password};
+
+SKIP: {
+    skip 'pageinspect extension not available', 1


Without pageinspect this block is skipped and the file still passes, 7 tests instead of 14, so a run on a bare install proves nothing and nothing says so. CI builds contrib, so it ran there (14 tests), but a plan skip_all with the reason, as the injection-point tests do, would make a skipped run visible.

ibrarahmad · 2026-10-22T08:19:52Z

 test: 108_apply_unknown_column_exception
 test: 109_node_info_propagation
 test: 110_resync_node_info_warning
+test: 111_lww_frozen_local_row


#649 and #639, both open against main, also add a test numbered 111 on this line, so whichever merges second conflicts here and leaves duplicate numbers. 114 is spoken for as well (#675 and #546); 115 is the lowest number no open PR uses.

get_tuple_origin() read the local row's xmin with HeapTupleHeaderGetXmin(), which returns FrozenTransactionId once the row is frozen. The non-normal xid branch then set local_ts to 0, so under last_update_wins any incoming UPDATE won against a frozen row, even one older than the local change. Since PostgreSQL 16, VACUUM freezes whole pages early when it is writing a full-page image anyway, so recently written rows can be frozen and nodes can end up with different values. Freezing only sets infomask bits and keeps the original xid, so read it with HeapTupleHeaderGetRawXmin(). This is what core's own conflict detection gets through heap_getsysattr(). Its commit timestamp stays available until commit_ts is truncated; after that the lookup fails and the existing fallback applies. The guard for a non-normal xid stays, for rows frozen before 9.4 and carried over by pg_upgrade. Add TAP test 116_lww_frozen_local_row: an older UPDATE from n1 arrives on n2 after n2 has updated the same row and frozen it; n2 must keep its value.

mason-sharp mentioned this pull request Oct 22, 2026

[v5] Use the raw xmin when looking up a local row's commit timestamp #676

Merged

mason-sharp requested a review from rasifr October 22, 2026 01:16

ibrarahmad approved these changes Oct 22, 2026

View reviewed changes

ibrarahmad reviewed Oct 22, 2026

View reviewed changes

mason-sharp force-pushed the fix-frozen-commit-ts branch from 6b72e69 to d2d9c4d Compare October 22, 2026 19:58

mason-sharp merged commit b49b11f into main Oct 23, 2026
43 of 44 checks passed

mason-sharp deleted the fix-frozen-commit-ts branch October 23, 2026 01:59

kloba mentioned this pull request Oct 27, 2026

Don't leave SpockCtx->lock held when no exception log slot is free #691

Open

Navigation Menu

Repository navigation

Use the raw xmin when looking up a local row's commit timestamp - #674

Use the raw xmin when looking up a local row's commit timestamp#674
mason-sharp merged 1 commit into
mainfrom
fix-frozen-commit-ts

mason-sharp commented Oct 22, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 22, 2026 •

edited

Loading

Uh oh!

codacy-production Bot commented Oct 22, 2026

Uh oh!

ibrarahmad left a comment

Uh oh!

ibrarahmad Oct 22, 2026

Uh oh!

ibrarahmad Oct 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mason-sharp commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Testing

Uh oh!

coderabbitai Bot commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codacy-production Bot commented Oct 22, 2026

Up to standards ✅

Uh oh!

ibrarahmad left a comment

Choose a reason for hiding this comment

Uh oh!

ibrarahmad Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

ibrarahmad Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mason-sharp commented Oct 22, 2026 •

edited

Loading

coderabbitai Bot commented Oct 22, 2026 •

edited

Loading