Skip to content

fix(scheduler): failed client-keepalive write no longer advances local timestamp - #42

Open
b7r6 wants to merge 2 commits into
mainfrom
fix/store-keepalive-retry
Open

b7r6 wants to merge 2 commits into
mainfrom
fix/store-keepalive-retry

Conversation

@b7r6

@b7r6 b7r6 commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

What and why

Lane A stability fix from the findings catalog (see nativelink-findings-2026-10-05.csv
on the meta/findings-catalog-2026-10-05 branch), rebased onto OSS main
@bf4b3aa855. This fork PR exists to validate the change on CI that is
representative of upstream PR lanes (public read-only cache, same endpoints)
before any upstream submission.

How was this verified?

The branch carries its own regression tests (fails-without where the fix
class allows it — see the branch's test commits). This PR's purpose is the
full representative CI run; green here rehearses the exact checks an
upstream PR would face.

Risk

Fork-internal validation PR; nothing lands anywhere by merging this. The
underlying fix's risk assessment happens at upstream-submission time.

AI assistance

Agent-assisted pipeline (Claude) for rebase and PR mechanics under my
direction; the fix itself carries verification in its tests.

@b7r6
b7r6 force-pushed the fix/store-keepalive-retry branch 2 times, most recently from 7ac5548 to 9c6bbf0 Compare October 7, 2026 20:31
b7r6 and others added 2 commits October 7, 2026 23:07
…keepalive_ts, retiring a live client's action as abandoned

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…l timestamp

In OperationSubscriber::changed (USE_SEPARATE_CLIENT_KEEPALIVE_KEY
branch), a failed ck_* store write still advanced the local
last_known_keepalive_ts and then parked for a full
CLIENT_KEEPALIVE_DURATION. Peer replicas judge client liveness solely
from the store, so a couple of consecutive transient store failures
(Redis blip) left the store's keepalive stamp aging past
client_action_timeout and a peer's sweep retired the live client's
action as abandoned.

Now the local timestamp only advances on a successful write; on failure
the sleep becomes a bounded CLIENT_KEEPALIVE_RETRY_DELAY (1s) so the
write is retried promptly without spinning when the store is down hard.

Repro test fails before the fix (attempts = 1 across 3x keepalive
periods) and passes after.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@b7r6
b7r6 force-pushed the fix/store-keepalive-retry branch from 9c6bbf0 to 67505a6 Compare October 8, 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.

1 participant