The failover and conviction planners copied out every shard a node holds - #525
Open
bjmeetsfo wants to merge 2 commits into
Open
The failover and conviction planners copied out every shard a node holds#525bjmeetsfo wants to merge 2 commits into
bjmeetsfo wants to merge 2 commits into
Conversation
Both run on a timer when enabled, and both began by cloning every server. A
server record carries one shard load, one stat load and one shard serving state
per shard the node holds, and the serving states carry three strings each. The
failover planner reads an address, a node id and a state; the conviction
planner reads those, a location, a heartbeat time, a reboot flag, and which
shards the node is serving.
32 servers, shards each failover conviction
250 1029.1us 1223.3us
1000 6131.5us 9611.3us
after 250 3.5us 228.6us
1000 2.8us 1224.6us
Failover no longer depends on what the nodes are holding at all.
Conviction still does, and should: it needs the set of shards a node is
serving, and that set comes from the reported states. What changed is where the
reduction happens -- the serving states become a set of shard ids when the view
is taken, rather than being copied out whole and reduced afterwards. It is 7.4x
cheaper and still grows with the shards, which is honest for what it computes.
Each planner has its own view rather than a shared one. They read different
things, and a shared view would hand each of them something it does not use,
which is the thing being fixed.
The serving-set test still builds a full record with reported shard states on
it and goes through the view, so it still proves the planner's serving set
comes from what the datanode reported -- that step simply moved.
Measured with the arms interleaved and the changed arm run twice.
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
2 participants
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.
Both planners run on a timer when enabled, and both began by cloning every server.
A server record carries one shard load, one stat load and one shard serving state per shard the node holds, and the serving states carry three strings each. The failover planner reads an address, a node id and a state. The conviction planner reads those, plus a location, a heartbeat time, a reboot flag, and which shards the node is serving.
Failover no longer depends on what the nodes are holding at all.
Conviction still grows with the shards, and should
It needs the set of shards a node is serving, and that set comes from the reported states. What changed is where the reduction happens: the serving states become a set of shard ids when the view is taken, rather than being copied out whole and reduced afterwards.
7.4x cheaper, still proportional to the shards. That is honest for what it computes -- it is not flat and this does not claim it is.
Separate views on purpose
Each planner has its own view rather than a shared one. They read different things, and a shared view would hand each of them something it does not use, which is the thing being fixed.
Testing
plan_round_reads_the_serving_set_from_the_heartbeatstill builds a full server record with reported shard states on it and goes through the view, so it still proves the planner's serving set comes from what the datanode reported. That step simply moved to where the view is taken.The test helpers in both modules now hand back the view, so every existing call site and every existing assertion is unchanged.
Suites: 39 failure-detector tests, 6 failover tests, 326 metadata tests, 47 metaserver binary tests, and
cargo check --all-targetsclean.Measurements were taken with the arms interleaved and the changed arm run twice.
Note on when this runs
Both paths are opt-in --
TS_RAFT_AUTO_FAILOVERandTS_META_ADAPTIVE_FAILURE_DETECTORboth default to off. This costs nothing to a deployment that leaves them off, and removes a per-tick cost proportional to the whole cluster's holdings from one that turns them on.