Skip to content

Commit

Permalink
clusterversion,kv: remove PriorReadSummaries
Browse files Browse the repository at this point in the history
Partially addresses cockroachdb#66544 by removing a cluster version and its associated
dependencies which, for any cluster whose version is at least 21.1, is certain
to be active.

Release justification: cluster version cleanup
  • Loading branch information
nvanbenschoten committed Sep 2, 2021
1 parent 78dd7d6 commit ac4cfa5
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 61 deletions.
8 changes: 0 additions & 8 deletions pkg/clusterversion/cockroach_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,6 @@ const (
// are propagated across RPC boundaries independently of their verbosity setting.
// This requires a version gate this violates implicit assumptions in v20.2.
TracingVerbosityIndependentSemantics
// PriorReadSummaries introduces support for the use of read summary objects
// to ship information about reads on a range through lease changes and
// range merges.
PriorReadSummaries
// NonVotingReplicas enables the creation of non-voting replicas.
NonVotingReplicas
// V21_1 is CockroachDB v21.1. It's used for all v21.1.x patch releases.
Expand Down Expand Up @@ -375,10 +371,6 @@ var versionsSingleton = keyedVersions{
Key: TracingVerbosityIndependentSemantics,
Version: roachpb.Version{Major: 20, Minor: 2, Internal: 28},
},
{
Key: PriorReadSummaries,
Version: roachpb.Version{Major: 20, Minor: 2, Internal: 44},
},
{
Key: NonVotingReplicas,
Version: roachpb.Version{Major: 20, Minor: 2, Internal: 46},
Expand Down
71 changes: 35 additions & 36 deletions pkg/clusterversion/key_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions pkg/kv/kvserver/batcheval/cmd_end_transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -1108,8 +1108,7 @@ func mergeTrigger(
// directly in this method. The primary reason why these are different is
// because the RHS's persistent read summary may not be up-to-date, as it is
// not updated by the SubsumeRequest.
readSumActive := rec.ClusterSettings().Version.IsActive(ctx, clusterversion.PriorReadSummaries)
if merge.RightReadSummary != nil && readSumActive {
if merge.RightReadSummary != nil {
mergedSum := merge.RightReadSummary.Clone()
if priorSum, err := readsummary.Load(ctx, batch, rec.GetRangeID()); err != nil {
return result.Result{}, err
Expand Down
3 changes: 1 addition & 2 deletions pkg/kv/kvserver/batcheval/cmd_lease.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,7 @@ func evalNewLease(
// only ever updates in-mem state) but it's easy to get things wrong (in
// which case they could easily take a catastrophic turn) and the benefit is
// low.
readSumActive := rec.ClusterSettings().Version.IsActive(ctx, clusterversion.PriorReadSummaries)
if priorReadSum != nil && readSumActive {
if priorReadSum != nil {
if err := readsummary.Set(ctx, readWriter, rec.GetRangeID(), ms, priorReadSum); err != nil {
return newFailedLeaseTrigger(isTransfer), err
}
Expand Down
8 changes: 0 additions & 8 deletions pkg/kv/kvserver/replica_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"time"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
Expand Down Expand Up @@ -562,13 +561,6 @@ func (r *Replica) AdminMerge(
log.Event(ctx, "merge txn begins")
txn.SetDebugName(mergeTxnName)

// If we aren't certain that all possible nodes in the cluster support a
// range merge transaction refreshing its reads while the RHS range is
// subsumed, observe the commit timestamp to force a client-side retry.
if !r.ClusterSettings().Version.IsActive(ctx, clusterversion.PriorReadSummaries) {
_ = txn.CommitTimestamp()
}

// Pipelining might send QueryIntent requests to the RHS after the RHS has
// noticed the merge and started blocking all traffic. This causes the merge
// transaction to deadlock. Just turn pipelining off; the structure of the
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/sem/tree/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/base",
"//pkg/clusterversion",
"//pkg/geo",
"//pkg/geo/geopb",
"//pkg/keys",
Expand Down
5 changes: 1 addition & 4 deletions pkg/sql/sem/tree/as_of.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"time"

"github.com/cockroachdb/apd/v2"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
Expand Down Expand Up @@ -227,9 +226,7 @@ func DatumToHLC(evalCtx *EvalContext, stmtTimestamp time.Time, d Datum) (hlc.Tim
s := string(*d)
// Parse synthetic flag.
syn := false
if strings.HasSuffix(s, "?") && evalCtx.Settings.Version.IsActive(evalCtx.Context, clusterversion.PriorReadSummaries) {
// NOTE: we don't parse this in mixed-version clusters because v20.2
// nodes will not know how to handle synthetic timestamps.
if strings.HasSuffix(s, "?") {
s = s[:len(s)-1]
syn = true
}
Expand Down

0 comments on commit ac4cfa5

Please sign in to comment.