mmaprototype: remove health tracking, do some housekeeping #156933
Conversation
|
This change is |
tbg
force-pushed
the
storehealth-preliminaries-1
branch
2 times, most recently
from
ba68487 to
c86666b
Compare
loadSummary is also a type. Linter complained in CI all out of a sudden 🤷
We'll replace it with a store-level construct in follow-up commits.
We'll replace it with a store-level construct in follow-up commits.
tbg
force-pushed
the
storehealth-preliminaries-1
branch
from
c86666b to
c0ea6d4
Compare
There was a problem hiding this comment.
Thanks for taking care of the housekeeping tasks and the easy review as usual.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @sumeerbhola)
pkg/kv/kvserver/allocator/mmaprototype/allocator_state.go line 816 at r2 (raw file):
// RemoveNodeAndStores implements the Allocator interface. func (a *allocatorState) RemoveNodeAndStores(nodeID roachpb.NodeID) error {
Should we delete RemoveNodeAndStores as well? What is it for?
pkg/kv/kvserver/allocator/mmaprototype/store_load_summary.go line 13 at r7 (raw file):
// load across a set of permissible stores (often, all that satisfy the // constraints for a given range). Sources and targets are primarily picked // based on the store- and node level load summaries contained in this struct.
super nit: :s/-// (no need to push again for this - feel free to just squeeze this in future pr)
There was a problem hiding this comment.
@sumeerbhola reviewed 6 of 10 files at r2, 1 of 2 files at r3, 6 of 10 files at r4, 2 of 2 files at r5, 2 of 3 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status:
It is unused and unclear what it would be used for. Easy to re-add should a need emerge.
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/allocator/mmaprototype/allocator_state.go line 816 at r2 (raw file):
Previously, wenyihu6 (Wenyi Hu) wrote…
Should we delete RemoveNodeAndStores as well? What is it for?
No idea. It is unused and unimplemented. Removed.
|
TFTR! bors r+ |
156938: mmaprototype: introduce store status r=tbg a=tbg This introduces the `Status` struct and hooks it up to the `TestClusterState` test. It doesn't yet attach any actual semantics to the new member of `storeStatus` that is being added as part of doing so. This will happen in follow-up commits, loosely replacing the integration points removed in #156933. Touches #156776. Epic: CRDB-55052 156999: kvserver: deflake TestPromoteNonVoterInAddVoter r=tbg a=pav-kv Fixes #156701 Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com> Co-authored-by: Pavel Kalinnikov <pavel@cockroachlabs.com>
4 participants
Epic: CRDB-55052