Skip to content

fix(client): refresh the cached leader after a successful lease RPC - #717

Merged
SebastianThiebaud merged 1 commit into
mainfrom
sebastian/control-plane-refreshes-leader
Sep 22, 2026
Merged

SebastianThiebaud merged 1 commit into
mainfrom
sebastian/control-plane-refreshes-leader

Conversation

@SebastianThiebaud

Copy link
Copy Markdown
Contributor

Bug

The client's leader cache is fresh for leader_ttl (default 30 s) after its last refresh. Only the GetTs and GetSeq success paths refreshed it. control_plane_rpc seats a NOT_LEADER hint, but its success path never touched the pool.

So a caller that only uses the lease control-plane RPCs kept a seated hint for exactly leader_ttl, however many renewals succeeded against it. The next call then fell back to the first configured endpoint. If that endpoint is a follower, or a load-balanced address whose HTTP/2 connection is pinned to one, the call is refused with NOT_LEADER again. The caller sees one failed control-plane call every leader_ttl, forever.

Fix

Each control-plane RPC's map closure now returns a private ControlPlaneReply: the decoded output plus the epoch at which the reply proves the endpoint led, if it proves that at all. On success, control_plane_rpc hands that epoch to ChannelPool::record_success, so the refresh goes through the same monotone-forward rule GetTs uses. Redirect handling, transport eviction and deadlines are unchanged.

RPC Refreshes the cache Why
AcquireLease yes, at the response epoch leader-only, carries epoch
RenewLease yes, at the response epoch leader-only, carries epoch
ReleaseLease no leader-only, but the reply has no epoch
GetSafeFrontier no followers answer it with a zero frontier and Epoch::ZERO
GetCurrentMaxSafe no followers answer it with zero rather than NOT_LEADER

Epoch-less responses: ReleaseLease is skipped rather than given an invented epoch. An epoch-less confirmed success cannot rank itself against a cached entry, and on an empty or expired cache it would seat an entry with no monotone floor that a later success from the real leader could not displace. A lease holder's acquire and renewals already keep the leader fresh.

GetSafeFrontier and GetCurrentMaxSafe do carry an epoch, but a follower returns Epoch::ZERO, and a single-node file-driver leader also runs at Epoch::ZERO. A success from either RPC says nothing about which endpoint leads, so recording it could seat a follower. They leave the cache alone.

The client usage doc now lists which successes refresh the cache.

Tests

  • lease_successes_keep_the_redirected_leader_cached_past_leader_ttl (tsoracle-tests/tests/client_lease.rs): boots a follower and a leader, points a client with a 300 ms leader_ttl at the follower, follows the redirect, then renews every 75 ms for three TTLs. It asserts every renewal succeeds, cached_leader() keeps naming the leader, and only the leader holds the lease. On the unfixed code it fails at renewal 3, about 310 ms after seating, with NOT_LEADER from the follower. The follower-plus-leader setup the existing redirect test used moved into a shared helper.
  • renew_lease_success_seats_the_serving_endpoint_at_its_epoch: a renewal on an empty cache seats the serving endpoint at the reply's epoch. Fails on the unfixed code.
  • stale_renewal_success_does_not_unseat_a_fresher_leader: a renewal parked on a peer at epoch 4 completes after a leader at epoch 9 was seated elsewhere. The cache keeps the epoch-9 leader.
  • unproven_control_plane_successes_leave_the_leader_cache_alone: release, safe-frontier and max-safe successes leave an empty cache empty. Recording the safe-frontier epoch makes it fail.

FakeTso gained on_renew_lease and on_release_lease handlers for these tests.

Local checks: cargo fmt --all -- --check, cargo clippy --workspace --all-targets --all-features --locked -- -D warnings, cargo test -p tsoracle-client -p tsoracle-tests --all-features --locked, the critical-path guard and the header check.

The leader cache stays fresh for leader_ttl after its last refresh, and only the GetTs and GetSeq success paths refreshed it. control_plane_rpc seated a NOT_LEADER hint but ignored its own successes. A caller that only issues lease RPCs therefore kept a seated hint for exactly leader_ttl, however many renewals succeeded against it. The next call fell back to the first configured endpoint. When that endpoint is a follower, or a load-balanced address whose connection is pinned to one, the call was refused again: one failed control-plane call every leader_ttl, forever.

The map closure of each control-plane RPC now returns a ControlPlaneReply: the decoded output plus the epoch at which the reply proves the endpoint led, if it proves that at all. On success control_plane_rpc passes that epoch to ChannelPool::record_success, so the refresh follows the same monotone-forward rule as GetTs. AcquireLease and RenewLease are leader-only and carry an epoch, so they refresh. GetSafeFrontier and GetCurrentMaxSafe do not: followers answer them with a zero value and Epoch::ZERO, which a single-node leader also uses, so a success says nothing about leadership. ReleaseLease does not either: it is leader-only but its reply has no epoch, and an epoch-less confirmed success on an empty cache would seat an entry that a later success from the real leader could not displace.

Tests: lease_successes_keep_the_redirected_leader_cached_past_leader_ttl boots a follower and a leader, points a client with a 300 ms leader_ttl at the follower, follows the redirect, and renews every 75 ms for three TTLs. Without the fix renewal 3 is refused by the follower at about 310 ms. Unit tests cover a renewal seating the serving endpoint at its epoch, a late renewal at a lower epoch not unseating a fresher leader, and release, safe-frontier and max-safe successes leaving the cache untouched. The client usage doc now lists which successes refresh the cache.
Signed-off-by: Sebastian Thiebaud <sebastian@prismarisk.com>
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 35762151161

Coverage increased (+0.09%) to 96.628%

Details

  • Coverage increased (+0.09%) from the base build.
  • Patch coverage: 6 uncovered changes across 1 file (145 of 151 lines covered, 96.03%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
crates/tsoracle-client/src/test_support.rs 22 16 72.73%
Total (2 files) 151 145 96.03%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 19838
Covered Lines: 19169
Line Coverage: 96.63%
Coverage Strength: 265648.51 hits per line

💛 - Coveralls

@SebastianThiebaud
SebastianThiebaud merged commit a14e890 into main Sep 22, 2026
51 checks passed
@SebastianThiebaud
SebastianThiebaud deleted the sebastian/control-plane-refreshes-leader branch September 22, 2026 17:51
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