Skip to content

Commit addb8cf

Browse files
committed
kvserver: add TestObsoleteCode
We often make changes that will allow follow-up cleanup to occur at some future point in time, once enough "water has passed under the bridge". One example of this is #144613, which partially migrated out of a boolean snapshot RPC parameter but had to retain the flag on the RPC for a few more releases. We aim to be more proactive about this cleanup once it becomes possible, but there isn't a great way to remind ourselves. Additionally, the moment the cleanup becomes possible is usually a bump of the MinSupportedVersion, which is carried out by a release team member who may be uncomfortable making various cleanups in teams' code they don't fully understand. This PR attempts a low-touch approach to this: TestObsoleteCode fails when a MinVersionBump makes additional cleanups possible. The test instructs the PR author to file issues/reach out to the team and to then disable the check. The hope is that this will encourage the team to perform the cleanup as soon as possible. Epic: none Release note: None
1 parent aae9466 commit addb8cf

File tree

2 files changed

+43
-0
lines changed

2 files changed

+43
-0
lines changed

pkg/kv/kvserver/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ go_test(
324324
"metrics_test.go",
325325
"mvcc_gc_queue_test.go",
326326
"node_liveness_test.go",
327+
"obsolete_code_test.go",
327328
"queue_concurrency_test.go",
328329
"queue_test.go",
329330
"raft_log_queue_test.go",
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package kvserver
7+
8+
import (
9+
"testing"
10+
11+
"github.com/cockroachdb/cockroach/pkg/clusterversion"
12+
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
13+
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
14+
)
15+
16+
// TestObsoleteCode contains nudges for cleanups that may be possible in the
17+
// future. When this test fails (which is necessarily a result of bumping the
18+
// MinSupportedVersion), please carry out the cleanups that are now possible or
19+
// file issues against the KV team asking them to do so (at which point you may
20+
// comment out the failing check).
21+
func TestObsoleteCode(t *testing.T) {
22+
defer leaktest.AfterTest(t)()
23+
24+
msv := clusterversion.RemoveDevOffset(clusterversion.MinSupported.Version())
25+
t.Logf("MinSupported: %v", msv)
26+
27+
v25dot2 := clusterversion.RemoveDevOffset(clusterversion.V25_2.Version())
28+
29+
// v25.2 is the last version to interpret RangeKeysInOrder. 25.3+ ignores
30+
// the field on incoming snapshots but continues to set it on outgoing
31+
// snapshots for compatibility reasons. This can be removed when the below
32+
// check fires.
33+
//
34+
// See https://github.com/cockroachdb/cockroach/pull/144613.
35+
//
36+
// NB: the below comparison intentionally minimizes the number of assumptions
37+
// on what release follows 25.2.
38+
if !msv.LessEq(v25dot2) {
39+
_ = kvserverpb.SnapshotRequest_Header{}.RangeKeysInOrder
40+
t.Fatalf("SnapshotRequest_Header.RangeKeysInOrder can be removed")
41+
}
42+
}

0 commit comments

Comments
 (0)