Use GetLatestSnapshot in CleanupRecordExists (#8594) - #8709
Merged
ibrahim halatci (ihalatci) merged 1 commit intoAug 4, 2026
Conversation
DESCRIPTION: Prevent a possible race condition in shard cleanup systable_beginscan without a snapshot calls GetCatalogSnapshot, which might be cached from a previous call. That means that if a shard move succeeded and the deletion of the cleanup record completed just before the cleaner tries to acquire a lock, `CleanupRecordExists` might still see the record in its snapshot and shard cleanup will incorrectly proceed. --------- Co-authored-by: Colm McHugh <colm.mchugh@gmail.com> (cherry picked from commit c41586c)
This was referenced Aug 4, 2026
Closed
ibrahim halatci (ihalatci)
merged commit Aug 4, 2026
f289638
into
release-13.2
172 of 210 checks passed
ibrahim halatci (ihalatci)
added a commit
that referenced
this pull request
Aug 4, 2026
…8714) Backport of #8594 to `release-14.0`. Tracking issue: #8713. **Verbatim cherry-pick** — `git cherry-pick -x c41586c`, zero conflicts, **zero adaptation**. `git patch-id --stable` is identical to the upstream commit. ### The bug `CleanupRecordExists` ran its catalog lookup under whatever snapshot the caller happened to hold. In the shard-cleanup path that snapshot can predate the deletion of a cleanup record, so an already-deleted record still looks present — and cleanup then proceeds to drop a shard that is actually live. ### The fix Take a fresh snapshot for the lookup: ```c Snapshot snapshot = GetLatestSnapshot(); ``` ### Why this cannot change behavior on 14.0 `CleanupRecordExists` is `static` with exactly **one** call site, and that site is reached only after a successful `TryLockOperationId()`. A fresher snapshot can only ever flip the result `true → false` — i.e. it can only *prevent* an erroneous drop. It cannot cause a drop that the old code would have skipped. ### Applicability to release-14.0 Vulnerable code confirmed present before the pick: `shard_cleaner.c:1183 CleanupRecordExists(uint64 recordId)` — the old signature with no snapshot argument. `git diff c41586c^ release-14.0` over the touched file was empty, i.e. the branch was in the exact pre-fix state. ### Validation Branch is 1 ahead / 0 behind `8cdb17e25`. Full A/B on PG16.14 (WSL), unmodified `release-14.0` vs a stack of all five backports, **with `make install` before each leg and an assertion that the installed `citus.so` md5 matches the worktree build**: | leg | `check-multi` | `check-multi-1` | |---|---|---| | baseline `8cdb17e25` | 190 ok, 0 failed | 207 ok, 0 failed | | all five stacked | 190 ok, 0 failed | 207 ok, 0 failed | Normalized status sets are **byte-identical** between the two legs (`check-multi` SHA256 `d94f42d7…`, `check-multi-1` SHA256 `f47f18bf…`), with zero `not ok` lines on either side. Also swept: `release-14.0` has **no** `_N` numbered expected-output variants anywhere, so there is no PG-version-variant exposure of the kind that required a five-line adaptation when #8556 was backported to `release-13.2`. Same fix shipped in **12.1.14** via #8703 and proposed for `release-13.2` in #8709. Co-authored-by: Marco Slot <marco.slot@snowflake.com> Co-authored-by: Colm McHugh <colm.mchugh@gmail.com>
This was referenced Aug 4, 2026
Merged
ibrahim halatci (ihalatci)
pushed a commit
that referenced
this pull request
Aug 5, 2026
Adds the CHANGELOG entry for v13.4.0. This release includes the following changes merged into release-13.2 since v13.3.0: - #8621 - citus_internal.distribute_object() repair UDF - #8625 - citus.allow_unsafe_insert_select_pushdown GUC - #8566 (via #8695) - skip 2PC for single-statement single-shard procedures - #8587 - object ownership checks for more citus-internal UDFs - #8638 (via #8654) - re-range sequences when promoting a clone node - #8651 (via #8657) - drop orphaned Citus local table shard copies - #8497 - wrong results when recursive planning projects columns as NULL - #8692 - UPDATE/DELETE with shard key equality and always-false predicate - #8594 (via #8709) - race condition in shard cleanup - #8498 (via #8710) - type mismatch with COLLATE and type cast - #8556 (via #8711) - segfault in EXPLAIN with LEFT JOIN and subqueries - #8561 (via #8712) - crash on writable standby coordinator Version bump to 13.4.0 was already done in 0aa073f. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 61fba89e-b958-4d9f-902e-eb76004b5580
ibrahim halatci (ihalatci)
pushed a commit
that referenced
this pull request
Aug 6, 2026
Adds the CHANGELOG entry for v13.4.0. This release includes the following changes merged into release-13.2 since v13.3.0: - #8621 - citus_internal.distribute_object() repair UDF - #8625 - citus.allow_unsafe_insert_select_pushdown GUC - #8566 (via #8695) - skip 2PC for single-statement single-shard procedures - #8587 - object ownership checks for more citus-internal UDFs - #8638 (via #8654) - re-range sequences when promoting a clone node - #8651 (via #8657) - drop orphaned Citus local table shard copies - #8497 - wrong results when recursive planning projects columns as NULL - #8692 - UPDATE/DELETE with shard key equality and always-false predicate - #8594 (via #8709) - race condition in shard cleanup - #8498 (via #8710) - type mismatch with COLLATE and type cast - #8556 (via #8711) - segfault in EXPLAIN with LEFT JOIN and subqueries - #8561 (via #8712) - crash on writable standby coordinator Version bump to 13.4.0 was already done in 0aa073f. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 61fba89e-b958-4d9f-902e-eb76004b5580
ibrahim halatci (ihalatci)
added a commit
that referenced
this pull request
Aug 6, 2026
Adds the CHANGELOG entry for **v13.4.0**.
## This is a CHANGELOG-only release
The version bump was already landed by `0aa073fe4` ("Set 13.4.0 as
upcoming version; run N-1 tests against 13.3.0", #8632). Re-verified on
this branch, all already at 13.4.0 / 13.4-1:
| artifact | state |
|---|---|
| `configure.ac` | `AC_INIT([Citus], [13.4.0])` |
| `configure` | 9 version strings at `13.4.0` |
| `src/backend/distributed/citus.control` | `default_version = '13.4-1'`
|
| `src/backend/columnar/citus_columnar.control` | `default_version =
'13.4-1'` |
| `sql/citus--13.3-1--13.4-1.sql` + downgrade | present |
| `sql/citus_columnar--13.3-1--13.4-1.sql` + downgrade | present |
| `expected/multi_extension.out` | contains the 13.4-1 tests |
So this PR touches **one file**: `CHANGELOG.md`, 44 insertions, 0
deletions.
This mirrors the shape of the v13.3.0 release commit `e5b5a3c45`
(#8588), which was likewise CHANGELOG-only because its bump had landed
in `e8a841d46`.
I also checked the one-off cleanup that `e5b5a3c45` carried: it removed
4 accidentally duplicated `DROP FUNCTION` lines from
`citus--13.3-1--13.2-1.sql`. The 13.4 counterpart
`citus--13.4-1--13.3-1.sql` has no duplicates, so no SQL cleanup is
needed here.
## Contents
`v13.3.0..release-13.2` contains 14 non-merge commits; 12 are
changelogged. The two excluded:
- `0aa073fe4` - version housekeeping (referenced in the commit message
instead)
- `f5b93d328` - PG-version/N-1 expected-output fixup that folds into
#8625
Citations use **origin PR numbers**, with `(via #backportPR)` noted in
the commit message only where they differ - matching the v13.3.0
convention (its bullet for #8466 reads just `(#8466)` even though the
commit body says "via #8579").
| origin PR | via | entry |
|---|---|---|
| #8621 | - | `citus_internal.distribute_object()` repair UDF |
| #8625 | - | `citus.allow_unsafe_insert_select_pushdown` GUC |
| #8566 | #8695 | skip 2PC for single-statement single-shard procedures
|
| #8587 | - | object ownership for more citus-internal UDFs |
| #8638 | #8654 | re-range sequences when promoting a clone node |
| #8651 | #8657 | drop orphaned Citus local table shard copies |
| #8497 | - | wrong results when recursive planning projects columns as
NULL |
| #8692 | - | `UPDATE`/`DELETE` with shard key equality + always-false
predicate |
| #8594 | #8709 | race condition in shard cleanup |
| #8498 | #8710 | type mismatch with `COLLATE` + type cast |
| #8556 | #8711 | segfault in `EXPLAIN` with `LEFT JOIN` + correlated
subqueries |
| #8561 | #8712 | crash on writable standby coordinator |
The last four are the 12.1/13.2/14.0 P1 backport set tracked in #8707.
## Formatting
Verified against the existing sections: header `### citus v13.4.0
(August 4, 2026) ###`, all lines <= 76 columns, 2-space continuation
indent, blank line between every bullet, `Adds` entries before `Fixes`.
Note this repo has `core.autocrlf=true`, so the worktree is CRLF while
the blob is LF. The section was written with CRLF and the committed blob
verified LF-only (`git ls-files --eol` -> `i/lf w/crlf`), which is why
the diff is a clean 44-line insertion rather than a whole-file rewrite.
Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 61fba89e-b958-4d9f-902e-eb76004b5580
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DESCRIPTION: Prevent a possible race condition in shard cleanup
Backport of #8594 to
release-13.2.Tracks #8707.
systable_beginscanwithout an explicit snapshot may use a cached catalogsnapshot. If an operation deletes its cleanup record just before the cleaner
acquires the operation-ID lock,
CleanupRecordExistscould still see the stalerow and incorrectly proceed to drop the underlying resource.
Cherry-picked from
c41586cc8fa6e96a677b921c01be6f585f27d819.Adaptation
Clean apply, no adaptation. The stable patch ID is identical to upstream.
Behavior-change analysis
CleanupRecordExistsisstaticand has exactly one call site, inDropOrphanedResourcesForCleanup, immediately after a successfulTryLockOperationId().TryLockOperationId()acquires the operation-ID exclusive lock withdontWait = true; an active operation owner therefore causes the cleaner toskip the record.
after the original list was read. It can only prevent the stale-row drop
race; no other caller inherits the changed snapshot semantics.
Verification (PostgreSQL 17.10)
./configure --without-libcurl.check-operations: all 17 tests passed.check-multiA/B against pristinerelease-13.2: all 190 test statuseswere identical. Both runs had the same pre-existing local-environment failure,
custom_aggregate_support, because TopN was installed while the selectedexpected variant assumes it is absent. The normalized status files have the
same SHA-256, and the failing result files are byte-identical.