Skip to content

fix(partitions): preserve purge cleanup across retries and recovery - #4130

Draft
diegomrsantos wants to merge 7 commits into
apache:masterfrom
diegomrsantos:test/purge-offset-directory-sync
Draft

diegomrsantos wants to merge 7 commits into
apache:masterfrom
diegomrsantos:test/purge-offset-directory-sync

Conversation

@diegomrsantos

@diegomrsantos diegomrsantos commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Purge now returns an error without advancing its applied generation when consumer bookmark cleanup or directory synchronization fails. Fixes #4128.

A checksummed purge.reset record separates durable message reset from cleanup completion. Retries for the same generation finish cleanup and publish purge.gen while preserving fresh committed messages and the original journal boundary. Recovery loads that boundary and finishes pending cleanup before loading bookmarks. Fresh bookmark mutations wait until cleanup completes.

The reset record follows the partition incarnation and discarded history. State transfer rebases both the record and WAL purge floor when replacing a longer operation sequence, with an install backup protecting rollback even when the partition has no WAL.

The simulator regressions cover both consumer kinds and both offset durability policies, including repeated cleanup failure during recovery and real Next polls after power loss. Separate partition tests exercise full purge retries with fresh committed messages, completion marker failures, waiting offset commits, singleton recovery, and NoAck requests. The simulator fixture rebuilds a partition from storage; it does not run full server boot or prove client receipt of replies.

Local validation on macOS:

  • All 11 simulator purge tests and 12 full retry tests passed. The eight original regressions are now green, and both controls remain.
  • cargo test -p journal -p partitions -p simulator -p server --lib: 1,053 passed, 7 ignored, no failures.
  • Workspace formatting, dependency ordering with --no-format, Clippy with all features and targets, and TOML formatting passed.
  • Private partition documentation built successfully. The combined server documentation build remains blocked by existing broken links in untouched HTTP, snapshot, and segment recovery files.

@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.17719% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.65%. Comparing base (2894368) to head (691ad0f).

Files with missing lines Patch % Lines
core/partitions/src/iggy_partition.rs 87.13% 17 Missing and 5 partials ⚠️
core/simulator/src/storage/purge.rs 93.31% 18 Missing and 3 partials ⚠️
core/partitions/src/state_transfer.rs 94.83% 11 Missing and 5 partials ⚠️
core/partitions/src/offset_storage.rs 95.27% 0 Missing and 7 partials ⚠️
core/shard/src/router.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master    #4130       +/-   ##
=============================================
- Coverage     87.62%   70.65%   -16.97%     
  Complexity     1575     1575               
=============================================
  Files          1284     1282        -2     
  Lines        225457   200038    -25419     
  Branches     188820   163400    -25420     
=============================================
- Hits         197549   141340    -56209     
- Misses        23177    54038    +30861     
+ Partials       4731     4660       -71     
Components Coverage Δ
Rust Core 67.79% <93.17%> (-20.94%) ⬇️
Java SDK 68.68% <ø> (ø)
C# SDK 77.42% <ø> (+0.07%) ⬆️
Python SDK 90.97% <ø> (ø)
PHP SDK 85.67% <ø> (ø)
Node SDK 96.45% <ø> (+0.08%) ⬆️
Go SDK 70.08% <ø> (+0.02%) ⬆️
Files with missing lines Coverage Δ
core/journal/src/partition_journal.rs 94.69% <100.00%> (+0.06%) ⬆️
core/server/src/partition_helpers.rs 83.45% <100.00%> (+0.07%) ⬆️
core/shard/src/router.rs 78.26% <0.00%> (ø)
core/partitions/src/offset_storage.rs 94.70% <95.27%> (+0.65%) ⬆️
core/partitions/src/state_transfer.rs 75.53% <94.83%> (+3.16%) ⬆️
core/simulator/src/storage/purge.rs 95.87% <93.31%> (-3.76%) ⬇️
core/partitions/src/iggy_partition.rs 93.13% <87.13%> (+0.24%) ⬆️

... and 352 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

A purge can record completion even though an offset directory sync failed, allowing stale bookmarks to survive power loss and skip fresh messages.

Exercise shared purge completion and recovery with SimStorage under both consumer offset durability policies. Keep direct completion failures separate from crash recovery checks so every policy and directory case runs.

The eight regression cases intentionally fail until the production bug is fixed.
@diegomrsantos
diegomrsantos force-pushed the test/purge-offset-directory-sync branch from 33c07b3 to 5b487f2 Compare September 21, 2026 17:10
@diegomrsantos diegomrsantos changed the title test(partitions): reproduce purge offset directory sync failure test(simulator): reproduce purge offset sync failure after power loss Sep 21, 2026
The purge regressions required following several helpers to understand one
scenario. Put both complete scenarios before their fixtures and declare the
four policy and directory cases together.

Inject directory sync errors by path instead of locating an operation through
a second purge. Record the directory state at the failure and spell out the
expected recovery state for failed and successful completion separately.
Keep purge failure scenarios readable without modifying shared simulator behavior.

Use explicit test functions and a local storage adapter for directory sync failures. Allow cleanup to stop at the first error while preserving generation and recovery assertions.

Validation: workspace formatting and Clippy pass. The focused suite has two passing controls and eight expected failures for the existing purge bug.
Failed bookmark directory synchronization must leave purge completion pending without allowing a retry to erase fresh committed messages. Record the durable message reset separately, resume cleanup before loading bookmarks, and protect the reset boundary when state transfer replaces history.

Cover power loss, full purge retries, pending offset commits, marker failures, singleton recovery, and state transfer rollback.
@diegomrsantos diegomrsantos changed the title test(simulator): reproduce purge offset sync failure after power loss fix(partitions): preserve purge cleanup across retries and recovery Sep 22, 2026
@diegomrsantos
diegomrsantos marked this pull request as ready for review September 22, 2026 12:25
@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Sep 22, 2026
@diegomrsantos
diegomrsantos marked this pull request as draft September 22, 2026 13:35
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label Sep 22, 2026
@diegomrsantos

Copy link
Copy Markdown
Contributor Author

@hubcio @spetz, working on #4130 raised a broader question about the guarantees we want from purge.

Today, purge clears messages, restarts their offsets at zero, and separately removes consumer bookmarks. If a bookmark deletion is not durable, an old bookmark at 2 can return after power loss and silently skip new messages 0–2. It still looks valid because those offsets now exist in a different history. Retrying the entire purge can also delete fresh messages accepted after the original reset. That is why this PR separates message reset from bookmark cleanup and records their progress independently.

The fundamental requirement is that messages, bookmarks, and recovery metadata agree about which history they describe. Physical file deletion can take several steps, provided recovery restores a coherent logical state. A persisted cutoff is only meaningful if the sequence it describes is also recovered consistently.

There is a related defect in the current PR. A local partition regression with three replicas and both durability policies set to Replicated restores the old operation cutoff, but reopens no durable operation journal. When operations restart at 1, fresh committed messages at or below the old cutoff disappear from polls. These are internal operation numbers, separate from public message offsets. Existing tests miss this combination because they either restore the previous operation counter or exercise restart with a singleton.

I looked at how other systems handle this.

  • Kafka advances the earliest available offset while preserving message numbering. After deleting messages through 100, new messages continue from 101. A bookmark at 80 is recognizably outside the available range, and the consumer can resume at the beginning, jump to the end, or report an error according to its configuration. Consumer offsets are managed separately, so message deletion does not require atomically rewriting every bookmark. Deletion implementation, consumer behavior
  • NATS JetStream preserves numbering even when a stream becomes empty and records the exact boundary of a clustered purge. Recovery replays “purge through 100,” so a later message 101 survives. It adjusts consumer state separately. File cleanup uses prepared replacement storage and recovery logic that can finish an interrupted switch, with old files removed afterward. Replay protection, storage recovery
  • Redpanda replicates a command advancing the partition's starting offset, requests a disk flush, and waits for the command to be applied. The committed boundary determines which messages remain available, while background cleanup reclaims obsolete storage. This gives recovery an authoritative deletion decision without requiring all files to disappear together. Implementation
  • Pulsar identifies positions by ledger and entry, like a volume number and page number. Entry zero in a new ledger cannot be mistaken for entry zero in the old one. Truncation advances subscription bookmarks before trimming unused ledgers. This is not one transaction across all bookmarks, but it preserves the identity of their positions. Position model, truncate implementation
  • RabbitMQ quorum queues order purge with message operations in their replicated history. Replaying “receive A, purge, receive B” leaves B intact because purge retains its place in that history. Physical compaction happens separately. Its semantics differ from Iggy's, since consumer state and messages already delivered but awaiting acknowledgment survive purge. Purge submission, queue state transition

These systems have different atomicity and durability guarantees. The common ideas worth considering are stable identities, precise replay boundaries, and separating the committed logical change from storage reclamation.

I'd appreciate your input on three points.

  1. Is restarting public message offsets at zero after purge a required compatibility guarantee, or could we preserve numbering and advance the available boundary?
  2. If we keep the reset semantics, would a committed generation shared by message and bookmark validity give us a clearer recovery contract?
  3. What should a successful purge guarantee after restart or power loss under each durability policy, and is that guarantee per partition?

My preference is to fix the concrete recovery defect in this PR and add the missing restart regression, then evaluate the broader design separately. The tests should also cover failed directory synchronization, retries after fresh writes, and another crash during recovery.

This branch has not been deployed

No deployments
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.

Purge can record completion after consumer offset directory sync fails

1 participant