Skip to content

feat(consensus): add ClientTable with WAL-backed commit path and view-change safety#3023

Merged
hubcio merged 12 commits intoapache:masterfrom
krishvishal:client-table
Apr 7, 2026
Merged

feat(consensus): add ClientTable with WAL-backed commit path and view-change safety#3023
hubcio merged 12 commits intoapache:masterfrom
krishvishal:client-table

Conversation

@krishvishal
Copy link
Copy Markdown
Contributor

@krishvishal krishvishal commented Mar 24, 2026

Summary

Implements the VSR client-table (VR Revisited, Section 4, Figure 2) with a WAL-backed commit path. Both metadata and partitions planes now execute committed ops through the same path on all replicas, ensuring the client table is always populated for view-change correctness.

  • ClientTable (client_table.rs): fixed-size slot array with HashMap secondary index for O(1) lookups. Tracks pending requests via Notify primitive for async commit notification. Deterministic eviction by lowest commit number.

  • PrepareJournal (renamed from MetadataJournal): reusable append-only WAL for consensus prepare messages, now shared by both planes.

  • WAL-backed commit path: operations are applied at commit time (not replicate time), eliminating duplicate data after view-change re-execution (IGGY-66). Both commit_journal (backup) and on_ack (primary) read from the WAL, apply the state machine, flush, advance commit_min, and update the client table - per-op, in a single loop.

  • commit_min / commit_max split: commit_max tracks what the cluster says is committed (advances immediately). commit_min tracks what this replica has actually executed (advances sequentially, +1 per op). DVC
    messages use commit_min. Fence and chain-replication use commit_min. Enables future message repair without data loss.

Additional fixes

  • build_reply_message uses prepare_header.op (not commit_max) for deterministic eviction ordering across replicas

  • fence_old_prepare_by_commit uses commit_min so retransmitted prepares needed for gap repair are not fenced out

  • handle_start_view returns range-based SendPrepareOk - caller must verify WAL presence before sending (prevents false acks)

  • drain_committable_prefix replaces drain_committable_all for strict global op ordering (prevents advance_commit_min invariant violation)

  • Sequencer/checksum updated after WAL append (not before) - failed append no longer leaves consensus state inconsistent

  • commit_reply asserts: client_id != 0, client_id == header.client, commit monotonicity, request monotonicity (all hard asserts)

  • advance_commit_min sequential invariant is a hard assert (not debug-only)

  • checkpoint_if_needed uses commit_min (not commit_max) - prevents draining unexecuted WAL entries

  • commit_messages called per-op (no dedup) - each apply's data flushed before advancing commit_min

  • ns_commits reset in clear() - prevents stale view-change state

  • IggyPartitions<C, J> - partitions plane now generic over journal type

    Closes Clients table implementation from VSR paper, for sending the response #3022

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 24, 2026

Codecov Report

❌ Patch coverage is 39.72603% with 440 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.48%. Comparing base (2afeaa3) to head (b6d8ea7).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
core/partitions/src/iggy_partitions.rs 0.00% 124 Missing ⚠️
core/shard/src/lib.rs 0.00% 85 Missing ⚠️
core/consensus/src/impls.rs 23.37% 59 Missing ⚠️
core/metadata/src/impls/metadata.rs 0.00% 54 Missing ⚠️
core/consensus/src/plane_helpers.rs 9.75% 37 Missing ⚠️
core/consensus/src/namespaced_pipeline.rs 14.28% 30 Missing ⚠️
core/shard/src/router.rs 0.00% 18 Missing ⚠️
core/consensus/src/client_table.rs 94.11% 10 Missing and 5 partials ⚠️
core/consensus/src/observability.rs 8.33% 11 Missing ⚠️
core/simulator/src/lib.rs 0.00% 4 Missing ⚠️
... and 3 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3023      +/-   ##
============================================
- Coverage     70.67%   70.48%   -0.20%     
  Complexity      943      943              
============================================
  Files          1114     1115       +1     
  Lines         94780    95388     +608     
  Branches      71980    72606     +626     
============================================
+ Hits          66987    67234     +247     
- Misses        25320    25658     +338     
- Partials       2473     2496      +23     
Components Coverage Δ
Rust Core 70.51% <39.72%> (-0.24%) ⬇️
Java SDK 62.30% <ø> (ø)
C# SDK 69.10% <ø> (-0.31%) ⬇️
Python SDK 81.43% <ø> (ø)
Node SDK 91.53% <ø> (ø)
Go SDK 38.97% <ø> (ø)
Files with missing lines Coverage Δ
core/binary_protocol/src/consensus/header.rs 46.79% <100.00%> (+7.56%) ⬆️
core/binary_protocol/src/consensus/message.rs 42.12% <100.00%> (+3.81%) ⬆️
core/iobuf/src/lib.rs 5.29% <ø> (ø)
core/journal/src/lib.rs 0.00% <ø> (ø)
core/journal/src/prepare_journal.rs 82.77% <92.30%> (ø)
core/metadata/src/impls/recovery.rs 70.63% <66.66%> (ø)
core/simulator/src/replica.rs 0.00% <0.00%> (ø)
core/simulator/src/lib.rs 0.00% <0.00%> (ø)
core/consensus/src/observability.rs 26.69% <8.33%> (-0.89%) ⬇️
core/consensus/src/client_table.rs 94.11% <94.11%> (ø)
... and 7 more

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

@krishvishal
Copy link
Copy Markdown
Contributor Author

Seems like a flaky test:

Summary [ 222.721s] 986 tests run: 985 passed (4 slow), 1 failed, 1125 skipped
FAIL [   4.814s] (730/986) integration::mod connectors::postgres::postgres_source::processed_column_source_marks_rows_after_producing

@krishvishal
Copy link
Copy Markdown
Contributor Author

Flaky test is fixed here: #3052

@krishvishal krishvishal changed the title feat(consensus): add ClientsTable for duplicate detection and reply caching feat(consensus): add ClientTable with WAL-backed commit path and view-change safety Apr 5, 2026
@krishvishal
Copy link
Copy Markdown
Contributor Author

More flaky connector tests:

FAIL [  47.082s] ( 770/1054) integration::mod connectors::iceberg::iceberg_sink::iceberg_sink_consumes_json_messages
FAIL [  31.201s] ( 773/1054) integration::mod connectors::iceberg::iceberg_sink::iceberg_sink_handles_bulk_messages
FAIL [  31.226s] ( 775/1054) integration::mod connectors::iceberg::iceberg_sink::iceberg_sink_initializes_and_runs

Working on fixing them.

@krishvishal
Copy link
Copy Markdown
Contributor Author

More flaky connector tests:

Fixed here: #3077

@hubcio hubcio merged commit 52b1986 into apache:master Apr 7, 2026
80 checks passed
@krishvishal krishvishal deleted the client-table branch April 7, 2026 11:55
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.

Clients table implementation from VSR paper, for sending the response

4 participants