Skip to content

Commit 78e1866

Browse files
tbgtbg
committed
cluster: remove VersionRangeAppliedStateKey
Release note: None
1 parent 64be50f commit 78e1866

8 files changed

Lines changed: 40 additions & 247 deletions

File tree

‎pkg/settings/cluster/cockroach_versions.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ const (
3636
Version2_0
3737
VersionImportSkipRecords
3838
VersionProposedTSLeaseRequest
39-
VersionRangeAppliedStateKey
4039
VersionImportFormats
4140
VersionSecondaryLookupJoins
4241
VersionColumnarTimeSeries
@@ -230,11 +229,12 @@ var versionsSingleton = keyedVersions([]keyedVersion{
230229
Key: VersionProposedTSLeaseRequest,
231230
Version: roachpb.Version{Major: 2, Minor: 0, Unstable: 2},
232231
},
233-
{
234-
// VersionRangeAppliedStateKey is https://github.com/cockroachdb/cockroach/pull/22317.
235-
Key: VersionRangeAppliedStateKey,
236-
Version: roachpb.Version{Major: 2, Minor: 0, Unstable: 3},
237-
},
232+
// Removed.
233+
// {
234+
// // VersionRangeAppliedStateKey is https://github.com/cockroachdb/cockroach/pull/22317.
235+
// Key: VersionRangeAppliedStateKey,
236+
// Version: roachpb.Version{Major: 2, Minor: 0, Unstable: 3},
237+
// },
238238
{
239239
// VersionImportFormats is https://github.com/cockroachdb/cockroach/pull/25615.
240240
Key: VersionImportFormats,

‎pkg/settings/cluster/versionkey_string.go‎

Lines changed: 26 additions & 27 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/storage/below_raft_protos_test.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,6 @@ var belowRaftGoldenProtos = map[reflect.Type]fixture{
7070
emptySum: 615555020845646359,
7171
populatedSum: 94706924697857278,
7272
},
73-
// MVCCStats is still serialized beneath Raft in tests that use old cluster
74-
// versions before the RangeAppliedState key.
75-
reflect.TypeOf(&enginepb.MVCCStats{}): {
76-
populatedConstructor: func(r *rand.Rand) protoutil.Message {
77-
return enginepb.NewPopulatedMVCCStats(r, false)
78-
},
79-
emptySum: 18064891702890239528,
80-
populatedSum: 4287370248246326846,
81-
},
8273
reflect.TypeOf(&raftpb.HardState{}): {
8374
populatedConstructor: func(r *rand.Rand) protoutil.Message {
8475
type expectedHardState struct {

‎pkg/storage/engine/mvcc.go‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -44,25 +44,6 @@ var (
4444
NilKey = MVCCKey{}
4545
)
4646

47-
// AccountForLegacyMVCCStats adjusts ms to account for the predicted impact it
48-
// will have on the values that it records when the structure is initially stored.
49-
// Specifically, MVCCStats is stored on the RangeStats legacy key, which means
50-
// that its creation will have an impact on system-local data size and key count.
51-
func AccountForLegacyMVCCStats(ms *enginepb.MVCCStats, rangeID roachpb.RangeID) error {
52-
key := keys.RangeStatsLegacyKey(rangeID)
53-
metaKey := MakeMVCCMetadataKey(key)
54-
55-
// MVCCStats is stored inline, so compute MVCCMetadata accordingly.
56-
value := roachpb.Value{}
57-
if err := value.SetProto(ms); err != nil {
58-
return err
59-
}
60-
meta := enginepb.MVCCMetadata{RawBytes: value.RawBytes}
61-
62-
updateStatsForInline(ms, key, 0, 0, int64(metaKey.EncodedSize()), int64(meta.Size()))
63-
return nil
64-
}
65-
6647
// MakeValue returns the inline value.
6748
func MakeValue(meta enginepb.MVCCMetadata) roachpb.Value {
6849
return roachpb.Value{RawBytes: meta.RawBytes}

‎pkg/storage/helpers_test.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -231,10 +231,6 @@ func (r *Replica) ReplicaIDLocked() roachpb.ReplicaID {
231231
return r.mu.replicaID
232232
}
233233

234-
func (r *Replica) DescLocked() *roachpb.RangeDescriptor {
235-
return r.mu.state.Desc
236-
}
237-
238234
func (r *Replica) AssertState(ctx context.Context, reader engine.Reader) {
239235
r.raftMu.Lock()
240236
defer r.raftMu.Unlock()

‎pkg/storage/replica_proposal.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -972,8 +972,13 @@ func (r *Replica) evaluateProposal(
972972
r.mu.RLock()
973973
usingAppliedStateKey := r.mu.state.UsingAppliedStateKey
974974
r.mu.RUnlock()
975-
if !usingAppliedStateKey &&
976-
r.ClusterSettings().Version.IsActive(cluster.VersionRangeAppliedStateKey) {
975+
if !usingAppliedStateKey {
976+
// The range applied state was introduced in v2.1. It's possible to
977+
// still find ranges that haven't activated it. If so, activate it.
978+
// We can remove this code if we introduce a boot-time check that
979+
// fails the startup process when any legacy replicas are found. The
980+
// operator can then run the old binary for a while to upgrade the
981+
// stragglers.
977982
if res.Replicated.State == nil {
978983
res.Replicated.State = &storagepb.ReplicaState{}
979984
}

‎pkg/storage/replica_test.go‎

Lines changed: 0 additions & 168 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import (
3535
"github.com/cockroachdb/cockroach/pkg/keys"
3636
"github.com/cockroachdb/cockroach/pkg/roachpb"
3737
"github.com/cockroachdb/cockroach/pkg/rpc"
38-
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
3938
"github.com/cockroachdb/cockroach/pkg/storage/batcheval"
4039
"github.com/cockroachdb/cockroach/pkg/storage/engine"
4140
"github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb"
@@ -9742,173 +9741,6 @@ func TestReplicaPushed1PC(t *testing.T) {
97429741
}
97439742
}
97449743

9745-
// assertUsingRangeAppliedState asserts that the value of
9746-
// ReplicaState.UsingAppliedStateKey is equal to the expected value.
9747-
func assertUsingRangeAppliedState(t *testing.T, repl *Replica, expSet bool) {
9748-
t.Helper()
9749-
repl.raftMu.Lock()
9750-
defer repl.raftMu.Unlock()
9751-
repl.mu.Lock()
9752-
defer repl.mu.Unlock()
9753-
9754-
usingAppliedStateKey := repl.mu.state.UsingAppliedStateKey
9755-
if usingAppliedStateKey != expSet {
9756-
t.Errorf("expected ReplicaState.UsingAppliedStateKey=%t, found %t",
9757-
expSet, usingAppliedStateKey)
9758-
}
9759-
}
9760-
9761-
// assertRangeAppliedStateRelatedKeysExist performs a series of assertions
9762-
// that each key related to the RangeAppliedState key migration is either
9763-
// present or missing, depending on the expRASK flag.
9764-
func assertRangeAppliedStateRelatedKeysExist(
9765-
ctx context.Context, t *testing.T, repl *Replica, expRASK bool,
9766-
) {
9767-
t.Helper()
9768-
repl.raftMu.Lock()
9769-
defer repl.raftMu.Unlock()
9770-
repl.mu.Lock()
9771-
defer repl.mu.Unlock()
9772-
9773-
assertHasKey := func(key roachpb.Key, expect bool) {
9774-
t.Helper()
9775-
val, _, err := engine.MVCCGet(ctx, repl.store.Engine(), key, hlc.Timestamp{},
9776-
engine.MVCCGetOptions{})
9777-
if err != nil {
9778-
t.Fatal(err)
9779-
}
9780-
9781-
exists := val != nil
9782-
if exists != expect {
9783-
t.Errorf("expected key %s to exist=%t, found %t", key, expect, exists)
9784-
}
9785-
}
9786-
9787-
rsl := repl.mu.stateLoader
9788-
assertHasKey(rsl.RangeAppliedStateKey(), expRASK)
9789-
assertHasKey(rsl.RaftAppliedIndexLegacyKey(), !expRASK)
9790-
assertHasKey(rsl.LeaseAppliedIndexLegacyKey(), !expRASK)
9791-
assertHasKey(rsl.RangeStatsLegacyKey(), !expRASK)
9792-
}
9793-
9794-
// TestReplicaBootstrapRangeAppliedStateKey verifies that a bootstrapped range
9795-
// is only created with a RangeAppliedStateKey if the cluster version is high
9796-
// enough to permit it.
9797-
func TestReplicaBootstrapRangeAppliedStateKey(t *testing.T) {
9798-
defer leaktest.AfterTest(t)()
9799-
9800-
testCases := []struct {
9801-
version roachpb.Version
9802-
expRangeAppliedStateKey bool
9803-
}{
9804-
{
9805-
version: cluster.VersionByKey(cluster.Version2_0),
9806-
expRangeAppliedStateKey: false,
9807-
},
9808-
{
9809-
version: cluster.VersionByKey(cluster.VersionRangeAppliedStateKey),
9810-
expRangeAppliedStateKey: true,
9811-
},
9812-
{
9813-
version: cluster.BinaryServerVersion,
9814-
expRangeAppliedStateKey: true,
9815-
},
9816-
}
9817-
for _, c := range testCases {
9818-
t.Run(fmt.Sprintf("version=%s", c.version), func(t *testing.T) {
9819-
ctx := context.Background()
9820-
stopper := stop.NewStopper()
9821-
defer stopper.Stop(ctx)
9822-
9823-
cfg := TestStoreConfig(nil)
9824-
cfg.Settings = cluster.MakeTestingClusterSettingsWithVersion(
9825-
c.version /* minVersion */, c.version /* serverVersion */)
9826-
tc := testContext{}
9827-
tc.StartWithStoreConfig(t, stopper, cfg)
9828-
repl := tc.repl
9829-
9830-
// Check that that UsingAppliedStateKey flag in ReplicaState is set
9831-
// as expected.
9832-
assertInMemState := func(t *testing.T) {
9833-
t.Helper()
9834-
assertUsingRangeAppliedState(t, repl, c.expRangeAppliedStateKey)
9835-
}
9836-
9837-
// Check that persisted keys agree with the UsingAppliedStateKey flag.
9838-
assertPersistentState := func(t *testing.T) {
9839-
t.Helper()
9840-
assertRangeAppliedStateRelatedKeysExist(ctx, t, repl, c.expRangeAppliedStateKey)
9841-
}
9842-
9843-
// Check that in-mem and persistent state agree.
9844-
assertInMemAndPersistentStateAgree := func(t *testing.T) {
9845-
t.Helper()
9846-
repl.AssertState(ctx, tc.engine)
9847-
}
9848-
9849-
// Check that the MVCCStats are correct.
9850-
computeStatsDelta := func(db *client.DB) (enginepb.MVCCStats, error) {
9851-
var b client.Batch
9852-
b.AddRawRequest(&roachpb.RecomputeStatsRequest{
9853-
RequestHeader: roachpb.RequestHeader{Key: roachpb.KeyMin},
9854-
DryRun: true,
9855-
})
9856-
if err := db.Run(ctx, &b); err != nil {
9857-
return enginepb.MVCCStats{}, err
9858-
}
9859-
resp := b.RawResponse().Responses[0].GetInner().(*roachpb.RecomputeStatsResponse)
9860-
delta := enginepb.MVCCStats(resp.AddedDelta)
9861-
delta.AgeTo(0)
9862-
return delta, nil
9863-
}
9864-
assertEmptyStatsDelta := func(t *testing.T) {
9865-
t.Helper()
9866-
delta, err := computeStatsDelta(repl.DB())
9867-
if err != nil {
9868-
t.Fatal(err)
9869-
}
9870-
if delta != (enginepb.MVCCStats{}) {
9871-
t.Errorf("unexpected stats adjustment of %+v", delta)
9872-
}
9873-
}
9874-
9875-
// Perform initial series of assertions.
9876-
assertInMemState(t)
9877-
assertPersistentState(t)
9878-
assertInMemAndPersistentStateAgree(t)
9879-
assertEmptyStatsDelta(t)
9880-
9881-
// Save the ReplicaState and perform persistent assertions again.
9882-
repl.raftMu.Lock()
9883-
repl.mu.Lock()
9884-
if _, err := repl.mu.stateLoader.Save(
9885-
ctx, tc.engine, repl.mu.state,
9886-
stateloader.TruncatedStateUnreplicated,
9887-
); err != nil {
9888-
t.Fatalf("could not save ReplicaState: %v", err)
9889-
}
9890-
repl.mu.Unlock()
9891-
repl.raftMu.Unlock()
9892-
assertPersistentState(t)
9893-
assertInMemAndPersistentStateAgree(t)
9894-
assertEmptyStatsDelta(t)
9895-
9896-
// Load the ReplicaState and perform in-memory assertions again.
9897-
repl.raftMu.Lock()
9898-
repl.mu.Lock()
9899-
state, err := repl.mu.stateLoader.Load(ctx, tc.engine, repl.DescLocked())
9900-
if err != nil {
9901-
t.Fatalf("could not load ReplicaState: %v", err)
9902-
}
9903-
repl.mu.state = state
9904-
repl.mu.Unlock()
9905-
repl.raftMu.Unlock()
9906-
assertInMemState(t)
9907-
assertInMemAndPersistentStateAgree(t)
9908-
})
9909-
}
9910-
}
9911-
99129744
func TestReplicaShouldCampaignOnWake(t *testing.T) {
99139745
defer leaktest.AfterTest(t)()
99149746

‎pkg/storage/stateloader/initial.go‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"context"
1717

1818
"github.com/cockroachdb/cockroach/pkg/roachpb"
19-
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
2019
"github.com/cockroachdb/cockroach/pkg/storage/engine"
2120
"github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb"
2221
"github.com/cockroachdb/cockroach/pkg/storage/storagepb"
@@ -75,17 +74,7 @@ func WriteInitialReplicaState(
7574
s.Lease = &lease
7675
s.GCThreshold = &gcThreshold
7776
s.TxnSpanGCThreshold = &txnSpanGCThreshold
78-
79-
// If the version is high enough to guarantee that all nodes will understand
80-
// the AppliedStateKey then we can just straight to using it without ever
81-
// writing the legacy stats and index keys.
82-
if !activeVersion.Less(cluster.VersionByKey(cluster.VersionRangeAppliedStateKey)) {
83-
s.UsingAppliedStateKey = true
84-
} else {
85-
if err := engine.AccountForLegacyMVCCStats(s.Stats, desc.RangeID); err != nil {
86-
return enginepb.MVCCStats{}, err
87-
}
88-
}
77+
s.UsingAppliedStateKey = true
8978

9079
if existingLease, err := rsl.LoadLease(ctx, eng); err != nil {
9180
return enginepb.MVCCStats{}, errors.Wrap(err, "error reading lease")

0 commit comments

Comments
 (0)