Skip to content

Commit e5b30e4

Browse files
committed
*: remove all version checks against 23.2
Now that `MinSupported` is `V23_2`, we no longer need to check if 23.2 is active. This change removes all these checks. Epic: none Release note: None
1 parent d7bae3f commit e5b30e4

37 files changed

+86
-512
lines changed

pkg/ccl/backupccl/restore_planning.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,11 +1229,6 @@ func restorePlanHook(
12291229
return nil, nil, nil, false, err
12301230
}
12311231

1232-
if restoreStmt.Options.RemoveRegions && !p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.V23_2) {
1233-
return nil, nil, nil, false,
1234-
errors.Newf("to set the remove_regions option, cluster version must be >= %s", clusterversion.V23_2.String())
1235-
}
1236-
12371232
if !restoreStmt.Options.SchemaOnly && restoreStmt.Options.VerifyData {
12381233
return nil, nil, nil, false,
12391234
errors.New("to set the verify_backup_table_data option, the schema_only option must be set")

pkg/clusterversion/README.md

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,13 +147,19 @@ avoid large changes on `master` which might cause merge conflicts for backports.
147147

148148
- [ ] File issue(s) to remove `TODO_Delete_` uses and simplify related code; assign
149149
to relevant teams (depending on which top-level packages use such gates).
150+
Version checks against what is now the minimum version (e.g. `V23_2`) should
151+
also be removed, as this version will always be active.
150152
<details><summary>For the overachiever</summary>
151153
Historically, these cleanup issues have not received much attention and the code
152154
was carried over for a long time. Feel free to ping individuals or do some of
153-
the cleanup directly (in separate PRs). The cleanup comes down to simplifying the
154-
code based on the knowledge that `IsActive()` will always return `true` for
155-
obsolete gates. If simplifying the code becomes non-trivial, error on the side
156-
of just removing `IsActive()` calls and leaving TODOs for further cleanup.
155+
the cleanup directly (in separate PRs).
156+
157+
The cleanup comes down to simplifying the code based on the knowledge that
158+
`IsActive()` will always return `true` for obsolete gates. If simplifying the
159+
code becomes non-trivial, error on the side of just removing `IsActive()` calls
160+
and leaving TODOs for further cleanup.
161+
162+
Example PRs: #124013, #124286
157163
</details>
158164

159165
- [ ] Regenerate expected test data results as needed; file issues for any

pkg/kv/kvclient/kvcoord/dist_sender_mux_rangefeed.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"sync/atomic"
1818
"unsafe"
1919

20-
"github.com/cockroachdb/cockroach/pkg/clusterversion"
2120
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangecache"
2221
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
2322
"github.com/cockroachdb/cockroach/pkg/roachpb"
@@ -645,9 +644,6 @@ func (c *muxStream) close() (toRestart []*activeMuxRangeFeed) {
645644
func NewCloseStreamRequest(
646645
ctx context.Context, st *cluster.Settings, streamID int64,
647646
) (*kvpb.RangeFeedRequest, error) {
648-
if !st.Version.IsActive(ctx, clusterversion.V23_2) {
649-
return nil, errors.Newf("CloseStream request requires cluster version 23.2 or above, found %s", st.Version)
650-
}
651647
return &kvpb.RangeFeedRequest{
652648
StreamID: streamID,
653649
CloseStream: true,

pkg/kv/kvserver/batcheval/cmd_delete_range.go

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,31 +14,20 @@ import (
1414
"context"
1515
"time"
1616

17-
"github.com/cockroachdb/cockroach/pkg/clusterversion"
1817
"github.com/cockroachdb/cockroach/pkg/keys"
1918
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
2019
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval/result"
2120
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
2221
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/lockspanset"
2322
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/spanset"
2423
"github.com/cockroachdb/cockroach/pkg/roachpb"
25-
"github.com/cockroachdb/cockroach/pkg/settings"
2624
"github.com/cockroachdb/cockroach/pkg/storage"
2725
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
2826
"github.com/cockroachdb/cockroach/pkg/storage/fs"
2927
"github.com/cockroachdb/cockroach/pkg/util/hlc"
3028
"github.com/cockroachdb/errors"
3129
)
3230

33-
// enableStickyGCHint controls whether the sticky GCHint is enabled.
34-
var enableStickyGCHint = settings.RegisterBoolSetting(
35-
settings.SystemOnly,
36-
"kv.gc.sticky_hint.enabled",
37-
"enable writing sticky GC hints which expedite garbage collection after schema changes"+
38-
" (ignored and assumed 'true' in 23.2)",
39-
false,
40-
)
41-
4231
func init() {
4332
RegisterReadWriteCommand(kvpb.DeleteRange, declareKeysDeleteRange, DeleteRange)
4433
}
@@ -152,14 +141,7 @@ func DeleteRange(
152141
return err
153142
}
154143

155-
updated := false
156-
// TODO(pavelkalinnikov): deprecate the cluster setting and call
157-
// ScheduleGCFor unconditionally when min supported version is 23.2.
158-
if cArgs.EvalCtx.ClusterSettings().Version.IsActive(ctx, clusterversion.V23_2) ||
159-
enableStickyGCHint.Get(&cArgs.EvalCtx.ClusterSettings().SV) {
160-
// Add the timestamp to GCHint to guarantee that GC eventually clears it.
161-
updated = hint.ScheduleGCFor(h.Timestamp)
162-
}
144+
updated := hint.ScheduleGCFor(h.Timestamp)
163145
// If the range tombstone covers the whole Range key span, update the
164146
// corresponding timestamp in GCHint to enable ClearRange optimization.
165147
if args.Key.Equal(desc.StartKey.AsRawKey()) && args.EndKey.Equal(desc.EndKey.AsRawKey()) {

pkg/kv/kvserver/replica_raft.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"strings"
1818
"time"
1919

20-
"github.com/cockroachdb/cockroach/pkg/clusterversion"
2120
"github.com/cockroachdb/cockroach/pkg/keys"
2221
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
2322
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/apply"
@@ -2728,14 +2727,7 @@ func shouldCampaignAfterConfChange(
27282727
return false
27292728
}
27302729
}
2731-
// Prior to 23.2, the first voter in the descriptor campaigned, so we do
2732-
// the same in mixed-version clusters to avoid ties.
2733-
if !st.Version.IsActive(ctx, clusterversion.V23_2) {
2734-
if storeID != desc.Replicas().VoterDescriptors()[0].StoreID {
2735-
// We're not the designated campaigner.
2736-
return false
2737-
}
2738-
} else if !leaseStatus.OwnedBy(storeID) || !leaseStatus.IsValid() {
2730+
if !leaseStatus.OwnedBy(storeID) || !leaseStatus.IsValid() {
27392731
// We're not the leaseholder.
27402732
return false
27412733
}

pkg/kv/kvserver/store.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -378,19 +378,11 @@ func newRaftConfig(
378378
Storage: strg,
379379
Logger: logger,
380380

381-
// StepDownOnRemoval requires 23.2. Otherwise, in a mixed-version cluster, a
382-
// 23.2 leader may step down when it demotes itself to learner, but a
383-
// designated follower (first in the range) running 23.1 will only campaign
384-
// once the leader is entirely removed from the range descriptor (see
385-
// shouldCampaignOnConfChange). This would leave the range without a leader,
386-
// having to wait out an election timeout.
387-
//
388381
// We only set this on replica initialization, so replicas without
389382
// StepDownOnRemoval may remain on 23.2 nodes until they restart. That's
390383
// totally fine, we just can't rely on this behavior until 24.1, but
391384
// we currently don't either.
392-
StepDownOnRemoval: storeCfg.Settings.Version.IsActive(ctx, clusterversion.V23_2) &&
393-
raftStepDownOnRemoval,
385+
StepDownOnRemoval: raftStepDownOnRemoval,
394386

395387
PreVote: true,
396388
CheckQuorum: storeCfg.RaftEnableCheckQuorum,

pkg/server/job_profiler.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"fmt"
1616
"strconv"
1717

18-
"github.com/cockroachdb/cockroach/pkg/clusterversion"
1918
"github.com/cockroachdb/cockroach/pkg/jobs"
2019
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
2120
"github.com/cockroachdb/cockroach/pkg/kv"
@@ -29,7 +28,6 @@ import (
2928
"github.com/cockroachdb/cockroach/pkg/util/log"
3029
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
3130
"github.com/cockroachdb/cockroach/pkg/util/tracing/zipper"
32-
"github.com/cockroachdb/errors"
3331
"google.golang.org/grpc/codes"
3432
"google.golang.org/grpc/status"
3533
)
@@ -67,10 +65,6 @@ func (s *statusServer) RequestJobProfilerExecutionDetails(
6765
// profiler details.
6866
if local {
6967
jobID := jobspb.JobID(req.JobId)
70-
if !execCfg.Settings.Version.IsActive(ctx, clusterversion.V23_2) {
71-
return nil, errors.Newf("execution details can only be requested on a cluster with version >= %s",
72-
clusterversion.V23_2.String())
73-
}
7468
e := makeJobProfilerExecutionDetailsBuilder(execCfg.SQLStatusServer, execCfg.InternalDB, jobID, execCfg.JobRegistry)
7569

7670
// TODO(adityamaru): When we start collecting more information we can consider

pkg/server/node.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2047,9 +2047,6 @@ func (n *Node) MuxRangeFeed(stream kvpb.Internal_MuxRangeFeedServer) error {
20472047

20482048
if err == nil {
20492049
cause := kvpb.RangeFeedRetryError_REASON_RANGEFEED_CLOSED
2050-
if !n.storeCfg.Settings.Version.IsActive(ctx, clusterversion.V23_2) {
2051-
cause = kvpb.RangeFeedRetryError_REASON_REPLICA_REMOVED
2052-
}
20532050
err = kvpb.NewRangeFeedRetryError(cause)
20542051
}
20552052

pkg/settings/registry.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ var retiredSettings = map[InternalKey]struct{}{
243243

244244
// removed as of 24.2
245245
"storage.value_blocks.enabled": {},
246+
"kv.gc.sticky_hint.enabled": {},
246247
}
247248

248249
// sqlDefaultSettings is the list of "grandfathered" existing sql.defaults

pkg/sql/alter_index_visible.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,13 @@ package sql
1313
import (
1414
"context"
1515

16-
"github.com/cockroachdb/cockroach/pkg/clusterversion"
1716
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
1817
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
1918
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
2019
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
2120
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
2221
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
2322
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
24-
"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
2523
"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
2624
)
2725

@@ -86,12 +84,6 @@ func (n *alterIndexVisibleNode) startExec(params runParams) error {
8684
return pgerror.Newf(pgcode.FeatureNotSupported, "primary index cannot be invisible")
8785
}
8886

89-
activeVersion := params.ExecCfg().Settings.Version.ActiveVersion(params.ctx)
90-
if !activeVersion.IsActive(clusterversion.V23_2) &&
91-
n.n.Invisibility.Value > 0.0 && n.n.Invisibility.Value < 1.0 {
92-
return unimplemented.New("partially visible indexes", "partially visible indexes are not yet supported")
93-
}
94-
9587
// Warn if this invisible index may still be used to enforce constraint check
9688
// behind the scene.
9789
if n.n.Invisibility.Value != 0.0 {

0 commit comments

Comments
 (0)