Repository navigation
slow member backpressure via 4lw - #126
Merged
Merged
Conversation
When a member falls behind, the leader keeps committing on the majority and the member drifts further away until it needs a snapshot. There was no way for an operator to make the leader slow down and let it catch up. This adds `slow_member_backpressure_enabled_`, off by default and switched while the server runs with `request_slow_member_backpressure`. While it is on, the leader does not move the commit index past any member it can reach, so writes go at the speed of the slowest member. It is deliberately all or nothing: no lag threshold, no trend detection, no time limit. An operator turns it on, watches the members catch up, and turns it off. Nothing in the leader decides on its own to start or stop throttling the cluster, and nothing has to be tuned for a cluster before it can be used. A member is left out only when the leader cannot reach it at all: its last request failed, it did not answer within `heart_beat_interval_ * full_consensus_leader_limit_`, or it has not answered at all yet on a new connection. Waiting for such a member would freeze the commit index with nothing to gain. A member receiving a snapshot is waited for: the transfer is no faster for waiting, but the log stops growing, so the member does not have to catch up on everything written during the transfer and then need another snapshot. A member the leader waits for can be behind what has already been committed, and one receiving a snapshot always is, so the clamp can ask for less than the current commit index. `commit` ignores anything that does not move the commit index forward, so the most this can do is freeze the commit index where it is. This is how the existing full consensus mode behaves too. Holding the commit index does not by itself stop the leader from taking new writes, so `slow_member_backpressure_max_uncommitted_` gives `max_uncommitted_log_entries_` a tighter value while the switch is on. Without it the log keeps growing and the member falls even further behind. The switch lasts for one leadership: `become_leader` turns it off. Only the leader acts on it, and a server that has just been elected may have missed the message that switched it, so acting on its own stale copy could throttle the cluster again after an operator had switched it off, with nothing to show why. This is close to `use_full_consensus_among_healthy_members_`, and it reuses that feature's definition of a member the leader can reach. The difference is what happens to a member that is behind: full consensus drops a member that is more than one batch behind the commit index, which is exactly the member this switch exists to wait for. The transport is the custom notification that carried the full consensus mode toggle, so `custom_notification_msg::set_full_consensus_mode` becomes `set_slow_member_backpressure`. The wire format is unchanged: a one-byte version and a one-byte flag under the same notification type. Along the way this fixes a pre-existing bug in `handle_custom_notification_resp`, which moved a peer's next log index backwards to the `next_slot` carried in the response. A custom notification does not replicate log entries, and while a peer is catching up or receiving a snapshot that value is below what the leader has already sent, so the leader would re-send entries it had already sent, or decide that the peer needs a snapshot again and restart the transfer from the beginning. The unit tests cover the switch itself, the members that are and are not waited for, the choice of the furthest member, and the tighter uncommitted limit.
The leader broadcast the setting to its peers so that every server would report the same value. It never achieved that. The broadcast is best-effort, so a peer that was busy or down missed it permanently, and `become_leader` switched the setting off locally without telling anyone - so right after an election two servers out of three reported it as on while the leader, the only one whose copy has any effect, reported it as off. A monitoring value that lies is worse than a narrow one, and it lies exactly when an operator is reading it. Only the leader ever holds the setting now. A follower asked for it already just forwards the request without setting it locally, so the one remaining way to hold a stale `true` was to be the leader and then stop being one. `become_follower` now switches it off, mirroring `become_leader`. Together they make the invariant real: the setting is true only on a leader an operator asked, so every server reports a value it can actually act on, and `get_current_params` on the leader is the answer. This is why the broadcast existed at all: it was inherited from the full consensus mode toggle it replaced. That mode is not per-leadership - it appears nowhere in `become_leader` - so every server genuinely needs its value, because any of them may be elected and must keep the mode. This setting is deliberately per-leadership, which is what makes propagating it pointless. With nothing broadcasting to peers, the branch in the request handler that applied a setting sent by the leader is unreachable: the only sender left is a follower forwarding to the leader, so the receiver is always the leader. It and the `leader_ == -1` reasoning that went with it are removed. The cost is that "is the cluster throttled?" has to be asked of the leader rather than of any server. `mntr` reports `server_state`, so finding it is one command.
The leader stopped waiting for a member once its response timer passed `heart_beat_interval_ * full_consensus_leader_limit_`. That constant belongs to full consensus mode, which is a different feature, and with ClickHouse Keeper's defaults it works out at two seconds - short enough to drop a member that is applying a large snapshot, which is exactly the member the backpressure exists for. It was also the same mistake as gating this on `stale_log_gap_`: a setting chosen for one purpose silently deciding another. `slow_member_backpressure_no_progress_timeout_` makes it explicit, with a default of thirty seconds. The rule is unchanged, only named: the response timer is reset solely by an accepted response, and an accepted response means the member took log entries or saved a snapshot object. So it measures progress rather than reachability - a member creeping forward keeps resetting it, and only a stuck one runs out. A member the leader cannot reach at all is still dropped straight away by its failed requests, without waiting for the timeout. `0` means the leader never stops waiting for a member it can reach. `silent_member_is_released_test` now sets a short timeout instead of shrinking the process global `raft_server::limits`, so it no longer has to restore them on every exit path. `no_progress_timeout_zero_waits_ indefinitely_test` pins the `0` case, without which the leader could quietly abandon a member an operator asked it to wait for.
Four comments still described the broadcast that was removed and the timeout that was replaced: - `slow_member_backpressure_enabled_` said a member is left out only when the leader cannot reach it. Making no progress within `slow_member_backpressure_no_progress_timeout_` is now a second reason. It also said a new leader switches the setting off, without mentioning that a departing one does too. - The stale term check justified itself by two servers that both think they are the leader sending the setting back and forth, which needed a leader to send it to peers. None does now. - The step-down before applying said a deposed leader must not send the setting on with its old term. There is nothing to send on. - `become_leader` said this server may have missed the message that switched the setting. With nothing broadcasting it, a non-leader never receives it at all, so the reason for the reset is simply that the setting does not carry over to a new leadership. Also `rpc_error_alone_releases_lagging_member_test`, which still spoke of "the expiry".
Thirty seconds was still a timeout in seconds, and the member worth waiting for is the one applying a large snapshot, which can be quiet for minutes. Giving up on it then is the worst possible moment: the leader resumes committing, the log runs ahead, and the snapshot lands stale, so the member needs another one - the loop the backpressure exists to break. The unit stays milliseconds, as every other duration parameter here is, only the default changes. A member the leader cannot reach at all is still dropped straight away by its failed requests, so this timeout only governs a member that answers but never progresses.
`apply_slow_member_backpressure` left out any member whose `matched_idx` is zero, on the grounds that the leader does not know where such a member stands and waiting for it could freeze the commit index for nothing. That reads zero as "no idea", but it is also what a reconnection leaves behind: `request_append_entries` resets `matched_idx` when it remakes the connection to a member, and the index is written again only once an install completes. A member that restarts, reconnects and then needs a snapshot therefore reports nothing for the whole transfer and was left out of the clamp, so the leader kept committing on the majority and could outrun it again - the loop the backpressure exists to break, and the opposite of what `slow_member_backpressure_enabled_` promises. Wait for such a member as well. A snapshot on its way to it is what tells the leader that zero is not ignorance: the leader knows exactly what the member holds, which is nothing of the log yet. There is no index to clamp to, so hold where the leader already is, which is as far as waiting for it can go. A member still probing after a reconnection has no snapshot on the way and is left out as before. The other two reasons to leave a member out still apply to a snapshot receiver, so a transfer that stops making progress releases the commit index after `slow_member_backpressure_no_progress_timeout_` as any other member would.
When the leader can send a member neither the log entries it needs nor a snapshot to replace them with, it warns the member that it is out of range and sends nothing else. Replication cannot recover such a member at all. The slow member backpressure kept waiting for it anyway, and none of the reasons to leave a member out caught the case. The warning is a request like any other and the member answers it; an accepted answer is what resets the response timer, so the member kept looking like it was making progress. Its matched index is whatever it reached before falling out of range, so it looked known, and its requests did not fail, so it looked reachable. That froze the commit index at the stale index, and the freeze sustained itself: snapshots are created from the committed index, so a leader that cannot commit never reaches the next one, and without a new snapshot the member never leaves the range it is stuck outside of. Nothing in the ordinary run of the cluster breaks the loop - it takes `bpof`, a forced snapshot, or a custom commit quorum, all of which need someone to notice first. Mark the member when the leader warns it, and leave a marked member out of the clamp. The mark is set inside the function that builds the warning, so the two cannot come apart, and it is cleared where the request is built rather than on each way out, so it cannot outlive the decision that set it. Building the warning moved into a function of its own for that reason; the message it sends is unchanged. The alternative, measuring progress by the matched index rather than by answers, would break the case the last change added: a member receiving a snapshot makes progress for a long time without its matched index moving at all.
`apply_slow_member_backpressure` iterates only regular members, so a learner is never held - `learner_is_never_held_test` pins that. The description said a member is waited for however far behind it is, which reads as a promise that covers learners too.
antonio2368
approved these changes
Sep 10, 2026
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
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.
Added slow member backpressure: while it is on, the leader does not commit past any member it is still waiting for, so that a member which has fallen behind can catch up. A member is waited for however far behind it is, including while it receives a snapshot, and is left out only when waiting cannot get it anywhere: the leader cannot reach it, or it made no progress within
slow_member_backpressure_no_progress_timeout_, which defaults to 30 s and counts an accepted response as progress. The behaviour is switched withrequest_slow_member_backpressurewhile the server runs and lasts for one leadership until disabled, only the leader holds it, andslow_member_backpressure_max_uncommitted_makes the leader refuse new writes while it is on.