Pin `Keeper` snapshots during transfer by antonio2368 · Pull Request #104941 · ClickHouse/ClickHouse · GitHub
Skip to content

Pin Keeper snapshots during transfer#104941

Merged
antonio2368 merged 2 commits into
masterfrom
keeper-snapshot-pin-fix
May 19, 2026
Merged

Pin Keeper snapshots during transfer#104941
antonio2368 merged 2 commits into
masterfrom
keeper-snapshot-pin-fix

Conversation

@antonio2368

@antonio2368 antonio2368 commented May 14, 2026

Copy link
Copy Markdown
Member

Pin local Keeper snapshot files while they are being transferred to recovering followers. This prevents snapshot rotation, cleanup, or cross-disk movement from removing the file behind an in-flight read.

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix a race in Keeper snapshot transfer where a snapshot could be removed or moved while it was being sent to a recovering follower.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

Version info

  • Merged into: 26.5.1.804

Manage `SnapshotFileInfo` entries with a deleter that unlinks retired snapshots only after active references are released. Use `getSnapshotPin` in `read_logical_snp_obj` so snapshot removal and cross-disk moves wait for in-flight transfers.

Track the cached remote `RemoteSnapshotLoader` by snapshot log index and reset it before snapshot retirement. Keep latest snapshot info as a local value so `moveSnapshotsIfNeeded` can move files once transfer and upload refs are gone.

Add `test_keeper_snapshot_rotation_race` for snapshot rotation during follower recovery and deferred deletion ordering. Remove unused helpers and redundant `sync` calls; `Keeper` reads go through Raft.
@clickhouse-gh

clickhouse-gh Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label May 14, 2026
@alexey-milovidov

Copy link
Copy Markdown
Member

This was fixed by #105146. Let's update the branch.

@clickhouse-gh

clickhouse-gh Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.20% 84.10% -0.10%
Functions 91.40% 91.40% +0.00%
Branches 76.60% 76.60% +0.00%

Changed lines: 72.73% (160/220) | lost baseline coverage: 14 line(s) · Uncovered code

Full report · Diff report

@antonio2368 antonio2368 enabled auto-merge May 19, 2026 08:12
@antonio2368 antonio2368 added this pull request to the merge queue May 19, 2026
Merged via the queue into master with commit a94282d May 19, 2026
165 of 167 checks passed
@antonio2368 antonio2368 deleted the keeper-snapshot-pin-fix branch May 19, 2026 08:27
@clickgapai

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants