Skip to content

fix(pruning): clear in-flight snapshot state when creation fails - #26787

Merged
swift1337 merged 14 commits into
cosmos:mainfrom
songgaoye:song/fix-prune-failure
Sep 14, 2026
Merged

swift1337 merged 14 commits into
cosmos:mainfrom
songgaoye:song/fix-prune-failure

Conversation

@songgaoye

@songgaoye songgaoye commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix pruning remaining blocked after snapshot creation fails.

The pruning manager previously inferred snapshot progress from announced and completed heights. When a snapshot failed after being announced, there was no failure transition to remove its height. The stale in-flight height could then cap pruning at failedHeight - 1 until the node restarted.

This change replaces that implicit bookkeeping with an explicit snapshot lifecycle:

  • StartSnapshot records an in-flight snapshot.
  • CompleteSnapshot removes it and advances the durable completed checkpoint.
  • FailSnapshot removes it without advancing the checkpoint.

Pruning is now bounded by the latest completed snapshot and the earliest snapshot still in flight. This correctly handles failures and out-of-order completion. For example, if snapshot 10 fails and snapshot 20 succeeds, pruning can advance using snapshot 20. If snapshot 10 is still running, pruning remains capped below 10 until it reaches a terminal state.

Only the completed checkpoint is persisted. In-flight state remains memory-only, and checkpoint persistence is performed atomically with the in-memory transition. Duplicate or stale completion calls are harmless and do not trigger unnecessary database writes.

This is a follow-up to #23638 and #24583. While #24583 improved pruning height calculation and restart handling, a failed snapshot could still remain tracked as in flight and block subsequent pruning.

Tests

Added coverage for snapshot lifecycle transitions, out-of-order completion, restart behavior, and persistence failures.

Closes: #XXXX

@songgaoye
songgaoye marked this pull request as ready for review August 29, 2026 13:24
@songgaoye
songgaoye requested a review from a team as a code owner August 29, 2026 13:24
@swift1337

Copy link
Copy Markdown
Member

@greptile review

@greptile-apps

greptile-apps Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces implicit snapshot-height bookkeeping with explicit start, completion, and failure transitions so failed snapshots no longer block pruning.

  • Tracks active snapshots in memory and persists only the highest completed checkpoint.
  • Connects snapshot creation success and failure paths to the new lifecycle.
  • Adds coverage for failures, out-of-order completion, restart behavior, persistence errors, and compatibility fallback behavior.

Confidence Score: 4/5

The PR should preserve compatibility with SnapshotAnnouncer-only snapshotters before merging, because they currently lose the required start notification.

The built-in root multi-store follows the new lifecycle correctly, but existing custom snapshotters implementing the retained announcement interface now receive only a post-success prune callback, breaking their established start-to-terminal callback sequence.

Files Needing Attention: store/snapshots/manager.go, store/snapshots/types/snapshotter.go

Important Files Changed

Filename Overview
store/pruning/manager.go Introduces explicit in-flight state and a durable completed checkpoint with ordered, idempotent lifecycle transitions.
store/snapshots/manager.go Routes snapshot outcomes through the lifecycle, but drops notifications for existing SnapshotAnnouncer-only implementations.
store/snapshots/types/snapshotter.go Adds SnapshotLifecycle while retaining SnapshotAnnouncer, creating a compatibility expectation the manager no longer honors.
store/rootmulti/store.go Implements the new lifecycle by forwarding transitions to the pruning manager.
store/pruning/manager_test.go Adds thorough lifecycle, restart, ordering, duplicate-call, and persistence-failure coverage.
store/snapshots/manager_test.go Covers lifecycle failure and bare-Snapshotter fallback paths but omits the existing SnapshotAnnouncer-only compatibility case.

Sequence Diagram

sequenceDiagram
    participant SM as Snapshot Manager
    participant LC as Snapshot Lifecycle
    participant SS as Snapshot Store
    participant PM as Pruning Manager
    SM->>LC: StartSnapshot(height)
    LC->>PM: Track in-flight height
    SM->>SS: Save snapshot
    alt Save succeeds
        SM->>LC: CompleteSnapshot(height)
        LC->>PM: Persist checkpoint and clear in-flight
    else Save fails
        SM->>LC: FailSnapshot(height)
        LC->>PM: Clear in-flight only
    end
Loading

Reviews (1): Last reviewed commit: "Merge branch 'main' into song/fix-prune-..." | Re-trigger Greptile

Comment thread store/snapshots/manager.go
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.30%. Comparing base (8f265eb) to head (52d30f8).

Files with missing lines Patch % Lines
store/rootmulti/store.go 0.00% 6 Missing ⚠️
store/pruning/manager.go 94.59% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #26787      +/-   ##
==========================================
+ Coverage   64.97%   65.30%   +0.32%     
==========================================
  Files         785      830      +45     
  Lines       55413    58622    +3209     
==========================================
+ Hits        36007    38282    +2275     
- Misses      19406    20340     +934     
Files with missing lines Coverage Δ
store/snapshots/manager.go 71.23% <100.00%> (ø)
store/pruning/manager.go 94.11% <94.59%> (ø)
store/rootmulti/store.go 79.84% <0.00%> (ø)

... and 52 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@swift1337

Copy link
Copy Markdown
Member

@songgaoye approved. please resolve conflicts

@songgaoye

Copy link
Copy Markdown
Contributor Author

@swift1337 please check again. Thanks!

@swift1337
swift1337 enabled auto-merge September 14, 2026 18:41
@swift1337
swift1337 added this pull request to the merge queue Sep 14, 2026
Merged via the queue into cosmos:main with commit c382bb4 Sep 14, 2026
44 checks passed
warpbuild-benchmark-bot Bot added a commit to WarpBuilds/cosmos-sdk that referenced this pull request Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants