roachpb: surface more accurate bytes scattered in AdminScatterResponse#80338
roachpb: surface more accurate bytes scattered in AdminScatterResponse#80338craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
|
Going to tag on a commit that changes the adder and batcher to use the new field, should we nuke the MVCCStats from the response unless we think we'll need it in the future @dt? |
aayushshah15
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @adityamaru, @andreimatei, and @dt)
pkg/kv/kvserver/replica_command.go, line 3435 at r1 (raw file):
RangeInfos: []roachpb.RangeInfo{ri}, MVCCStats: stats, // Note, we use this range's MVCCStats to estimate the size of the replicas
This replica's*
|
pkg/kv/kvserver/replica_command.go, line 3419 at r1 (raw file):
Sorry I didn't read this carefully enough before approving. Note that Essentially what we want is something like |
adityamaru
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/replica_command.go, line 3419 at r1 (raw file):
Previously, aayushshah15 (Aayush Shah) wrote…
Sorry I didn't read this carefully enough before approving. Note that
ReplicaIDs are monotonically increasing (as replicas are rebalanced/added) and not some function of the number of replicas (which is what this seems to be assuming).Essentially what we want is something like
if _, ok := oldStoreIDs[rd.ReplicaID]; !ok { numReplicasMoved++ }
ahh TIL, thanks
pkg/kv/kvserver/replica_command.go, line 3435 at r1 (raw file):
Previously, aayushshah15 (Aayush Shah) wrote…
This replica's*
Done.
|
@aayushshah15 going to wait on another stamp from you before borsing |
aayushshah15
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/replica_command.go line 3365 at r3 (raw file):
oldStoreIDs := make(map[roachpb.ReplicaID]struct{}) for _, rd := range r.Desc().Replicas().Descriptors() { oldStoreIDs[rd.ReplicaID] = struct{}{}
nit: let's rename this map to something more accurate.
This change approximates the byte size of all the replicas that were moved as part of an AdminScatter request by comparing their store IDs before and after the replicas are passed through the replicate queue. This change also fixes a bug where MVCCStats returned as part of the AdminScatterResponse were not being combined. This would cause misrepresentation of the stats if the scatter key span covered more than one range. Release note: None
|
TFTR! bors r=aayushshah15 |
|
Build failed (retrying...): |
|
Build succeeded: |
This change approximates the byte size of all the replicas that
were moved as part of an AdminScatter request by comparing their
store IDs before and after the replicas are passed through the
replicate queue.
This change also fixes a bug where MVCCStats returned as part of the
AdminScatterResponse were not being combined. This would cause
misrepresentation of the stats if the scatter key span covered
more than one range.
Release note: None