Rework ID usage in multiraft #2675
Conversation
bdarnell
force-pushed
the
use-replica-id
branch
2 times, most recently
from
34638e7 to
9ef2c84
Compare
|
One general comment: is there a test that demonstrates what this change enables? if not, is it feasible to add one? |
|
This PR is supposed to be a functionally-equivalent refactoring; the testable change will be in the next PR (which will introduce tombstones as described in the RFC and we'll be able to test various scenarios around replica removal and resurrection). |
|
OK, LGTM |
|
LGTM. Some tests where |
|
FYI our existing tests in |
|
LGTM |
|
This change appears to have altered the timing enough that it has exposed flakiness in some tests. I'm running the tests repeatedly to try and flush these out before merging; my apologies to anyone else in the CI queue today. |
See docs/RFCS/replica_tombstone.md for motivation and background. Raft requires that node IDs never be reused; coalesced heartbeats require that node IDs are consistent across all groups. We must therefore use different IDs for the two concepts. The never-reused raft node ID is known as a ReplicaID in cockroach; our node ID and store ID are used in multiraft for message routing but never make it down to the raft level. The mapping from (GroupID, ReplicaID) to (NodeID, StoreID) never changes once set, so multiraft maintains a cache and there is no need to worry about invalidations. This cache is populated by three sources: the range descriptors, incoming snapshots (both loaded from the application via new methods in the multiraft.Storage interface), and the to/from addresses of all non-heartbeat raft messages.
The renaming of storage.Range to storage.Replica made the proto.Replica name confusing.
Also change a couple of printfs that `go vet` is suddenly unhappy with (it's not recognizing the `String()` implementations on these specific variables, even though it works for other vars of the same types).
bdarnell
force-pushed
the
use-replica-id
branch
from
8b46af9 to
570b508
Compare
See docs/RFCS/replica_tombstone.md for motivation and background.
Raft requires that node IDs never be reused; coalesced heartbeats
require that node IDs are consistent across all groups. We must
therefore use different IDs for the two concepts. The never-reused raft
node ID is known as a ReplicaID in cockroach; our node ID and store ID
are used in multiraft for message routing but never make it down to the
raft level.
The mapping from (GroupID, ReplicaID) to (NodeID, StoreID) never changes
once set, so multiraft maintains a cache and there is no need to worry
about invalidations. This cache is populated by three sources: the range
descriptors, incoming snapshots (both loaded from the application via
new methods in the multiraft.Storage interface), and the to/from
addresses of all non-heartbeat raft messages.
The important changes are in the first commit; the other two are just renamings.