kvserver: remove detritus for old change replicas and preemptive snaps #72237
Conversation
|
This change is |
|
This is certainly going to break in CI in some way, but I think should still be ok for review. |
There was a problem hiding this comment.
Reviewed 6 of 6 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 10 of 10 files at r5, 8 of 8 files at r6, 3 of 3 files at r7, 1 of 1 files at r8, 2 of 2 files at r9, 2 of 2 files at r10, 1 of 1 files at r11, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @tbg)
pkg/kv/kvserver/raft.proto, line 174 at r5 (raw file):
// by raft because at that point it is better to queue up the stream // than to cancel it. bool can_decline = 4;
Same here. We should reserve this field number as well.
pkg/roachpb/data.proto, line 228 at r1 (raw file):
// TODO(tbg): when removing this, also rename internal_x_replicas to just // x_replicas and remove the getter. ReplicaChangeType deprecated_change_type = 1;
Should we mark these field numbers as reserved?
tbg
force-pushed
the
deprecated-change-replicas
branch
from
ad05310 to
a432d57
Compare
There was a problem hiding this comment.
TFTR! I had to massage the assertion I added a little bit to account for the testing knob that some tests use (and which can violate the assertions).
Addressed the missing reserved fields too, so will merge on green.
Reviewable status:
|
bors r=nvanbenschoten |
|
bors r- messed something up (I had picked up two unrelated commits in the last push, not sure how) |
|
Canceled. |
These became obsolete long before we introduced long-running migration (we switched to the new method of doing things in 19.2). In the meantime, all ranges have migrated into the applied state and in the process, have conveniently flushed out any triggers still using the old format from their logs. So we get to "just do this"; thanks, long-running migrations. Release note: None
Release note: None
Release note: None
It's used in 7+ nontrivial tests that have nothing to do with preemptive snapshots. Release note: None
Only preemptive snapshots specified CanDecline, and we haven't been using those since 19.2. This also removes the corresponding rejection enum reason `SnapshotResponse_DECLINED`, and retires the `server.declined_reservation_timeout` cluster setting. Release note: None
Release note: None
Release note: None
This was set before both the default range size and snapshot rate limit changed. Re-do the math and adjust the constant. Release note: None
I'll get to this, but the mission at hand is removing residual mentions of preemptive snapshots. Release note: None
This is an invariant that was established in [cockroachdb#40892]. We now check it more aggressively. There is an awkward exception thanks to uninitialized replicas. [cockroachdb#40892]: cockroachdb#40892 Release note: None fixupassert
The cluster settings should be used instead. Neither env var was referenced from our codebase, i.e. they are not used in roachtests. Also, update a TODO. Release note: None
tbg
force-pushed
the
deprecated-change-replicas
branch
from
a432d57 to
80a9eba
Compare
|
bors r=nvanbenschoten |
|
Build succeeded: |
See individual commits.
Release note: None