Skip to content

Commit

Permalink
storage: Remove Store.canCampaignIdleReplica
Browse files Browse the repository at this point in the history
With PreVote, it is less disruptive to campaign unnecessarily, so
there's no need for this additional check.

Release note: None
  • Loading branch information
bdarnell committed May 2, 2018
1 parent 43e76a0 commit 2b9c931
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 111 deletions.
8 changes: 0 additions & 8 deletions pkg/server/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,14 +412,6 @@ func (n *Node) start(
return err
}

if n.initialBoot {
// The cluster was just bootstrapped by this node, explicitly notify the
// stores that they were bootstrapped.
for _, s := range stores {
s.NotifyBootstrapped()
}
}

if err := n.startStores(ctx, stores, n.stopper); err != nil {
return err
}
Expand Down
7 changes: 0 additions & 7 deletions pkg/storage/replica.go
Original file line number Diff line number Diff line change
Expand Up @@ -541,13 +541,6 @@ func (r *Replica) withRaftGroupLocked(
return nil
}

if shouldCampaignOnCreation {
// Special handling of idle replicas: we campaign their Raft group upon
// creation if we gossiped our store descriptor more than the election
// timeout in the past.
shouldCampaignOnCreation = (r.mu.internalRaftGroup == nil) && r.store.canCampaignIdleReplica()
}

ctx := r.AnnotateCtx(context.TODO())

if r.mu.internalRaftGroup == nil {
Expand Down
54 changes: 1 addition & 53 deletions pkg/storage/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -397,11 +397,6 @@ type Store struct {
nodeDesc *roachpb.NodeDescriptor
initComplete sync.WaitGroup // Signaled by async init tasks

idleReplicaElectionTime struct {
syncutil.Mutex
at time.Time
}

// Semaphore to limit concurrent non-empty snapshot application and replica
// data destruction.
snapshotApplySem chan struct{}
Expand Down Expand Up @@ -1543,36 +1538,7 @@ func (s *Store) GossipStore(ctx context.Context) error {
// Unique gossip key per store.
gossipStoreKey := gossip.MakeStoreKey(storeDesc.StoreID)
// Gossip store descriptor.
if err := s.cfg.Gossip.AddInfoProto(gossipStoreKey, storeDesc, gossip.StoreTTL); err != nil {
return err
}
// Once we have gossiped the store descriptor the first time, other nodes
// will know that this node has restarted and will start sending Raft
// heartbeats for active ranges. We compute the time in the future where a
// replica on this store which receives a command for an idle range can
// campaign the associated Raft group immediately instead of waiting for the
// normal Raft election timeout.
//
// Note that computing this timestamp here is conservative. We really care
// that the node descriptor has been gossiped as that is how remote nodes
// locate this one to send Raft messages. The initialization sequence is:
// 1. gossip node descriptor
// 2. wait for gossip to be connected
// 3. gossip store descriptors (where we're at here)
s.idleReplicaElectionTime.Lock()
if s.idleReplicaElectionTime.at == (time.Time{}) {
// Raft uses a randomized election timeout in the range
// [electionTimeout,2*electionTimeout]. Using the lower bound here means
// that elections are somewhat more likely to be contested (assuming
// traffic is distributed evenly across a cluster that is restarted
// simultaneously). That's OK; it just adds a network round trip or two to
// the process since a contested election just restarts the clock to where
// it would have been anyway if we weren't doing idle replica campaigning.
electionTimeout := s.cfg.RaftTickInterval * time.Duration(s.cfg.RaftElectionTimeoutTicks)
s.idleReplicaElectionTime.at = s.Clock().PhysicalTime().Add(electionTimeout)
}
s.idleReplicaElectionTime.Unlock()
return nil
return s.cfg.Gossip.AddInfoProto(gossipStoreKey, storeDesc, gossip.StoreTTL)
}

type capacityChangeEvent int
Expand Down Expand Up @@ -1613,15 +1579,6 @@ func (s *Store) recordNewWritesPerSecond(newVal float64) {
}
}

func (s *Store) canCampaignIdleReplica() bool {
s.idleReplicaElectionTime.Lock()
defer s.idleReplicaElectionTime.Unlock()
if s.idleReplicaElectionTime.at == (time.Time{}) {
return false
}
return !s.Clock().PhysicalTime().Before(s.idleReplicaElectionTime.at)
}

// GossipDeadReplicas broadcasts the store's dead replicas on the gossip
// network.
func (s *Store) GossipDeadReplicas(ctx context.Context) error {
Expand Down Expand Up @@ -1674,7 +1631,6 @@ func (s *Store) Bootstrap(
return errors.Wrap(err, "persisting bootstrap data")
}

s.NotifyBootstrapped()
return nil
}

Expand Down Expand Up @@ -1795,14 +1751,6 @@ func checkEngineEmpty(ctx context.Context, eng engine.Engine) error {
return nil
}

// NotifyBootstrapped tells the store that it was bootstrapped and allows idle
// replicas to campaign immediately. This primarily affects tests.
func (s *Store) NotifyBootstrapped() {
s.idleReplicaElectionTime.Lock()
s.idleReplicaElectionTime.at = s.Clock().PhysicalTime()
s.idleReplicaElectionTime.Unlock()
}

// GetReplica fetches a replica by Range ID. Returns an error if no replica is found.
func (s *Store) GetReplica(rangeID roachpb.RangeID) (*Replica, error) {
if value, ok := s.mu.replicas.Load(int64(rangeID)); ok {
Expand Down
43 changes: 0 additions & 43 deletions pkg/storage/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2868,49 +2868,6 @@ func TestRemovedReplicaTombstone(t *testing.T) {
}
}

func TestCanCampaignIdleReplica(t *testing.T) {
defer leaktest.AfterTest(t)()
tc := testContext{}
stopper := stop.NewStopper()
defer stopper.Stop(context.TODO())
tc.Start(t, stopper)
s := tc.store
ctx := context.Background()

s.idleReplicaElectionTime.Lock()
s.idleReplicaElectionTime.at = time.Time{}
s.idleReplicaElectionTime.Unlock()

// Bump the clock by the election timeout. Idle replicas can't campaign
// eagerly yet because we haven't gossiped the store descriptor.
electionTimeout := int64(s.cfg.RaftTickInterval * time.Duration(s.cfg.RaftElectionTimeoutTicks))
tc.manualClock.Increment(electionTimeout)
if s.canCampaignIdleReplica() {
t.Fatalf("idle replica can unexpectedly campaign")
}

// Gossip the store descriptor.
if err := s.GossipStore(ctx); err != nil {
t.Fatal(err)
}
if s.canCampaignIdleReplica() {
t.Fatalf("idle replica can unexpectedly campaign")
}

// Bump the clock to just before the idle election time.
tc.manualClock.Increment(electionTimeout - 1)
if s.canCampaignIdleReplica() {
t.Fatalf("idle replica can unexpectedly campaign")
}

// One more nanosecond bump and idle replicas should be able to campaign
// eagerly.
tc.manualClock.Increment(1)
if !s.canCampaignIdleReplica() {
t.Fatalf("idle replica unexpectedly cannot campaign")
}
}

type fakeSnapshotStream struct {
nextResp *SnapshotResponse
nextErr error
Expand Down

0 comments on commit 2b9c931

Please sign in to comment.