Skip to content

fix(partitions): sync checkpoints through original writers - #4253

Merged
hubcio merged 2 commits into
apache:masterfrom
jiengup:fix/failed-writeback
Sep 22, 2026
Merged

hubcio merged 2 commits into
apache:masterfrom
jiengup:fix/failed-writeback

Conversation

@jiengup

@jiengup jiengup commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR address?

Closes #4141
Relates to #4252

Rationale

Linux reports delayed writeback errors through per-open-file error
cursors. A checkpoint that reopens a materialized path after the error
can miss it and reclaim the WAL after the original writer lost bytes.

What changed?

Checkpoint mutations now carry durability barriers tied to the original
writers. Active indexes keep their existing synchronous fence and pass
an internal already-synced barrier, while simulator barriers retain the
original SimFile until the checkpoint mutation runs.

Every supplied barrier completes before path and directory barriers and
before WAL checkpoint publication. The simulator now covers the original
#4141 failure and multiple writers for one file, and records the related
install-backup gap from #4252 as an ignored follow-up regression.

Local Execution

Passed:

  • cargo fmt --all
  • cargo sort --no-format --workspace
  • cargo check -p partitions --no-default-features
  • cargo clippy --all-features --all-targets -- -D warnings
  • cargo test -p journal --lib
    • 95 passed, 0 failed
  • cargo test -p partitions --lib
    • 304 passed, 0 failed, 3 ignored
  • cargo test -p simulator --lib
    • 150 passed, 0 failed, 4 ignored
  • ./scripts/ci/taplo.sh --check
  • git diff --check

The #4141 regression was confirmed red before the implementation and
passes after the fix:

cargo test -p simulator --lib \
  given_a_failed_writeback_when_checkpointing_then_wal_history_should_not_be_reclaimed

The ignored #4252 regression was also confirmed to fail for the expected
reason: install_backup::link_tree publishes the backup after syncing a
fresh hard-link handle that sampled past the original writer's error.
#4252 works as a follow-up issue which will not block this PR.

AI Usage

  1. Tool: OpenAI Codex through Delta.
  2. Scope: root-cause analysis, checkpoint barrier design, implementation,
    deterministic regression tests, and repository-wide storage-path audit.
  3. Verification: red-green regression runs, journal/partitions/simulator
    unit suites, full workspace Clippy, formatting, Cargo sorting, and TOML
    checks.
  4. I can explain every changed line if asked.

@github-actions

Copy link
Copy Markdown

Thanks for the PR. It is labeled S-waiting-on-review and queued for review.

Slash commands (own line, regular comment) move it around the queue:

  • /ready - back to S-waiting-on-review after addressing feedback
  • /author - flip to S-waiting-on-author while you finish changes
  • /request-review @user-or-team - request a reviewer
  • /pin - exempt the PR from the stale bot, /unpin to undo

See CONTRIBUTING.md for details.

@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Sep 21, 2026
Fresh descriptors sample past earlier writeback errors, so synchronizing reopened checkpoint files can reclaim WAL after materialized bytes were lost.

Carry original-writer barriers into checkpoint mutations while preserving the active-index fence. Cover single and multiple writer failures, and record the related install-backup gap for follow-up.
@jiengup
jiengup force-pushed the fix/failed-writeback branch from 30f4a6f to 6ba7ef8 Compare September 21, 2026 17:16
@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.77778% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 69.37%. Comparing base (fa80704) to head (5a62498).

Files with missing lines Patch % Lines
core/partitions/src/iggy_partition.rs 92.30% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master    #4253       +/-   ##
=============================================
- Coverage     87.59%   69.37%   -18.23%     
- Complexity     1575     1576        +1     
=============================================
  Files          1284     1282        -2     
  Lines        224493   191250    -33243     
  Branches     187856   154614    -33242     
=============================================
- Hits         196653   132676    -63977     
- Misses        23127    53964    +30837     
+ Partials       4713     4610      -103     
Components Coverage Δ
Rust Core 66.01% <97.77%> (-22.69%) ⬇️
Java SDK 68.70% <ø> (+0.01%) ⬆️
C# SDK 77.42% <ø> (+0.07%) ⬆️
Python SDK 90.97% <ø> (ø)
PHP SDK 85.67% <ø> (ø)
Node SDK 96.43% <ø> (+0.06%) ⬆️
Go SDK 70.11% <ø> (+0.05%) ⬆️
Files with missing lines Coverage Δ
core/partitions/src/iggy_index_writer.rs 93.81% <100.00%> (+0.19%) ⬆️
core/partitions/src/lib.rs 0.00% <ø> (ø)
core/partitions/src/persistence.rs 92.05% <100.00%> (+0.17%) ⬆️
core/partitions/src/iggy_partition.rs 92.90% <92.30%> (+<0.01%) ⬆️

... and 369 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.

@jiengup

jiengup commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

/request-review @numinnex

@github-actions
github-actions Bot requested a review from numinnex September 22, 2026 06:21
@hubcio
hubcio merged commit 2cacea5 into apache:master Sep 22, 2026
101 checks passed
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label Sep 22, 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.

Fix given_a_failed_writeback_when_a_fresh_handle_syncs_then_it_should_not_report_success

4 participants