Skip to content

Commit

Permalink
Merge #60429
Browse files Browse the repository at this point in the history
60429: kv: (re-)introduce a stopgap for lack of ReplicaState synchronization r=irfansharif a=irfansharif

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

Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
  • Loading branch information
craig[bot] and irfansharif committed Feb 10, 2021
2 parents 4764a01 + 4880248 commit c133e2a
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkg/kv/kvserver/replica.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/kv/kvserver/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c133e2a

Please sign in to comment.