Repository navigation
Conversation
The UMBP_* knobs that seed the heartbeat interval are read once into function-local statics, so they cannot be changed in a running process. Worse, the interval only reaches a peer in RegisterClientResponse, so even restarting the master leaves already-connected peers on their old value until they reconnect. Retuning reporting cadence therefore meant restarting a master, which is disproportionate: every peer must re-register and re-ship a full-sync snapshot before routing is whole again. Re-advertise the effective interval on every HeartbeatResponse and add a SetRuntimeConfig RPC (plus a umbp_admin CLI) that overrides it in place. A change converges within one old heartbeat period with nothing restarted and no peer re-registering. Deliberately narrow: heartbeat_ttl, max_missed_heartbeats and the reaper are untouched, so failure-detection semantics are unchanged. An override is clamped to the expiry window, since past that point a peer would be reaped between two of its own heartbeats -- the invariant the interval divisor exists to protect. An advertised 0 means "no opinion" (what a master predating the field sends), and is ignored rather than adopted, which would collapse the wait and spin the heartbeat thread. Verified by unit tests (including an inverted run confirming the new case fails when the client-side adoption is disabled), single-node end-to-end, and a two-node Slurm job: 5000ms baseline -> 1000ms live -> clamp at 30000ms -> revert to 5000ms, all measured from master-side metrics, with the peer alive throughout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The heartbeat interval a peer uses is derived on the master from
UMBP_HEARTBEAT_TTL_SEC / UMBP_HEARTBEAT_INTERVAL_DIVISOR, and both are read once into function-local statics — they cannot change in a running process. The derived value then reaches a peer only inRegisterClientResponse, so even restarting the master leaves already-connected peers on their old interval until they reconnect.Retuning reporting cadence therefore meant restarting a master. That is disproportionate for a knob that only affects how often peers report: every peer has to re-register and re-ship a full-sync snapshot before routing is whole again.
This matters because the interval is the backstop for how long a completed
BatchPutstays invisible to the master. Events ship either when the peer's outbox hitsUMBP_AUTO_FLUSH_EVENT_THRESHOLD(128) or when the heartbeat fires (5s by default). A batch smaller than the threshold waits up to the full interval.Change
Re-advertise the effective interval on every
HeartbeatResponse, and add aSetRuntimeConfigRPC that overrides it in place, driven by a newumbp_adminCLI: