Skip to content

Commit

Permalink
logstore: drop found param from LoadRaftReplicaID
Browse files Browse the repository at this point in the history
It now has to be there, so turn this into an assertion failure.
See cockroachdb#95513.

Epic: CRDB-220
Release note: None
tbg committed Feb 8, 2023
1 parent 5984059 commit 845ec92
Showing 6 changed files with 19 additions and 20 deletions.
6 changes: 2 additions & 4 deletions pkg/kv/kvserver/client_store_test.go
Original file line number Diff line number Diff line change
@@ -91,8 +91,7 @@ func TestStoreRaftReplicaID(t *testing.T) {
require.NoError(t, err)
repl, err := store.GetReplica(desc.RangeID)
require.NoError(t, err)
replicaID, found, err := stateloader.Make(desc.RangeID).LoadRaftReplicaID(ctx, store.Engine())
require.True(t, found)
replicaID, err := stateloader.Make(desc.RangeID).LoadRaftReplicaID(ctx, store.Engine())
require.NoError(t, err)
require.Equal(t, repl.ReplicaID(), replicaID.ReplicaID)

@@ -102,9 +101,8 @@ func TestStoreRaftReplicaID(t *testing.T) {
require.NoError(t, err)
rhsRepl, err := store.GetReplica(rhsDesc.RangeID)
require.NoError(t, err)
rhsReplicaID, found, err :=
rhsReplicaID, err :=
stateloader.Make(rhsDesc.RangeID).LoadRaftReplicaID(ctx, store.Engine())
require.True(t, found)
require.NoError(t, err)
require.Equal(t, rhsRepl.ReplicaID(), rhsReplicaID.ReplicaID)
}
8 changes: 3 additions & 5 deletions pkg/kv/kvserver/kvstorage/replica_state.go
Original file line number Diff line number Diff line change
@@ -46,13 +46,11 @@ func LoadReplicaState(
replicaID roachpb.ReplicaID,
) (LoadedReplicaState, error) {
sl := stateloader.Make(desc.RangeID)
id, found, err := sl.LoadRaftReplicaID(ctx, eng)
id, err := sl.LoadRaftReplicaID(ctx, eng)
if err != nil {
return LoadedReplicaState{}, err
} else if !found {
return LoadedReplicaState{}, errors.AssertionFailedf(
"r%d: RaftReplicaID not found", desc.RangeID)
} else if loaded := id.ReplicaID; loaded != replicaID {
}
if loaded := id.ReplicaID; loaded != replicaID {
return LoadedReplicaState{}, errors.AssertionFailedf(
"r%d: loaded RaftReplicaID %d does not match %d", desc.RangeID, loaded, replicaID)
}
13 changes: 10 additions & 3 deletions pkg/kv/kvserver/logstore/stateloader.go
Original file line number Diff line number Diff line change
@@ -203,8 +203,15 @@ func (sl StateLoader) SetRaftReplicaID(
// LoadRaftReplicaID loads the RaftReplicaID.
func (sl StateLoader) LoadRaftReplicaID(
ctx context.Context, reader storage.Reader,
) (replicaID roachpb.RaftReplicaID, found bool, err error) {
found, err = storage.MVCCGetProto(ctx, reader, sl.RaftReplicaIDKey(),
) (*roachpb.RaftReplicaID, error) {
var replicaID roachpb.RaftReplicaID
found, err := storage.MVCCGetProto(ctx, reader, sl.RaftReplicaIDKey(),
hlc.Timestamp{}, &replicaID, storage.MVCCGetOptions{})
return
if err != nil {
return nil, err
}
if !found {
return nil, errors.AssertionFailedf("no replicaID persisted")
}
return &replicaID, nil
}
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/loqrecovery/recovery_env_test.go
Original file line number Diff line number Diff line change
@@ -747,7 +747,7 @@ func (e *quorumRecoveryEnv) handleDumpStore(t *testing.T, d datadriven.TestData)
descriptorViews = append(descriptorViews, descriptorView(desc))

sl := stateloader.Make(desc.RangeID)
raftReplicaID, _, err := sl.LoadRaftReplicaID(ctx, store.engine)
raftReplicaID, err := sl.LoadRaftReplicaID(ctx, store.engine)
if err != nil {
t.Fatalf("failed to load Raft replica ID: %v", err)
}
5 changes: 1 addition & 4 deletions pkg/kv/kvserver/replica.go
Original file line number Diff line number Diff line change
@@ -1400,13 +1400,10 @@ func (r *Replica) assertStateRaftMuLockedReplicaMuRLocked(
log.Fatalf(ctx, "replica's replicaID %d diverges from descriptor %+v", r.replicaID, r.mu.state.Desc)
}
}
diskReplID, found, err := r.mu.stateLoader.LoadRaftReplicaID(ctx, reader)
diskReplID, err := r.mu.stateLoader.LoadRaftReplicaID(ctx, reader)
if err != nil {
log.Fatalf(ctx, "%s", err)
}
if !found {
log.Fatalf(ctx, "no replicaID persisted")
}
if diskReplID.ReplicaID != r.replicaID {
log.Fatalf(ctx, "disk replicaID %d does not match in-mem %d", diskReplID, r.replicaID)
}
5 changes: 2 additions & 3 deletions pkg/kv/kvserver/store_test.go
Original file line number Diff line number Diff line change
@@ -3784,10 +3784,9 @@ func TestStoreGetOrCreateReplicaWritesRaftReplicaID(t *testing.T) {
})
require.NoError(t, err)
require.True(t, created)
replicaID, found, err := repl.mu.stateLoader.LoadRaftReplicaID(ctx, tc.store.Engine())
replicaID, err := repl.mu.stateLoader.LoadRaftReplicaID(ctx, tc.store.Engine())
require.NoError(t, err)
require.True(t, found)
require.Equal(t, roachpb.RaftReplicaID{ReplicaID: 7}, replicaID)
require.Equal(t, &roachpb.RaftReplicaID{ReplicaID: 7}, replicaID)
}

func BenchmarkStoreGetReplica(b *testing.B) {

0 comments on commit 845ec92

Please sign in to comment.