From 488024875f2b282b8928870a71c5f2445479fb9a Mon Sep 17 00:00:00 2001 From: irfan sharif Date: Wed, 10 Feb 2021 11:38:06 -0500 Subject: [PATCH] kv: (re-)introduce a stopgap for lack of ReplicaState synchronization See #59194 and #58489 for more details. In #58489 we observed a scary lack of synchronization around how we set the ReplicaState for a given replica, and how we mark a replica as "initialized". What this meant is that it was possible for the entry in Store.mu.replicas to be both "initialized" and have an empty ReplicaState. This is now more likely to bite us given the migrations infrastructure attempts to purge outdated replicas at start up time (when replicas are being initialized, and we're iterating through extan replicas in the Store.mu.replicas map). We believed this was addressed as part of #58378, but that appears not to be the case. Lets re-introduce this stop-gap while we investigate. Release note: None --- pkg/kv/kvserver/replica.go | 5 +++++ pkg/kv/kvserver/store.go | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/pkg/kv/kvserver/replica.go b/pkg/kv/kvserver/replica.go index 55890415809a..411f39dec194 100644 --- a/pkg/kv/kvserver/replica.go +++ b/pkg/kv/kvserver/replica.go @@ -809,6 +809,11 @@ func (r *Replica) GetGCThreshold() hlc.Timestamp { // Version returns the replica version. func (r *Replica) Version() roachpb.Version { + if r.mu.state.Version == nil { + // TODO(irfansharif,tbg): This is a stop-gap for #58523. + return roachpb.Version{} + } + r.mu.RLock() defer r.mu.RUnlock() return *r.mu.state.Version diff --git a/pkg/kv/kvserver/store.go b/pkg/kv/kvserver/store.go index 8bc6755177ba..abed71933eec 100644 --- a/pkg/kv/kvserver/store.go +++ b/pkg/kv/kvserver/store.go @@ -2719,6 +2719,10 @@ func (s *Store) PurgeOutdatedReplicas(ctx context.Context, version roachpb.Versi qp := quotapool.NewIntPool("purge-outdated-replicas", 50) g := ctxgroup.WithContext(ctx) s.VisitReplicas(func(repl *Replica) (wantMore bool) { + if (repl.Version() == roachpb.Version{}) { + // TODO(irfansharif,tbg): This is a stop gap for #58523. + return true + } if !repl.Version().Less(version) { // Nothing to do here. return true