From 86a961e4f30e89e63c9f00bb0e05b0a5c1bb7825 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Thu, 27 Jun 2019 22:28:13 -0400 Subject: [PATCH 1/2] storage: remove stale code from TestReplicaRefreshPendingCommandsTicks These two comments haven't been true since 41526e2 and e653798, respectively. Release note: None --- pkg/storage/replica_test.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/pkg/storage/replica_test.go b/pkg/storage/replica_test.go index 1dc3e093ec1a..2a979789db86 100644 --- a/pkg/storage/replica_test.go +++ b/pkg/storage/replica_test.go @@ -7533,23 +7533,12 @@ func TestReplicaRefreshPendingCommandsTicks(t *testing.T) { defer stopper.Stop(context.TODO()) tc.StartWithStoreConfig(t, stopper, cfg) - // Grab Replica.raftMu in order to block normal raft replica processing. This - // test is ticking the replica manually and doesn't want the store to be - // doing so concurrently. r := tc.repl - repDesc, err := tc.repl.GetReplicaDescriptor() if err != nil { t.Fatal(err) } - // Only followers refresh pending commands during tick events. Change the - // replica that the range thinks is the leader so that the replica thinks - // it's a follower. - r.mu.Lock() - r.mu.leaderID = 2 - r.mu.Unlock() - electionTicks := tc.store.cfg.RaftElectionTimeoutTicks { // The verifications of the reproposal counts below rely on r.mu.ticks From 7d3583d11106096d96ebecd803a837a786338051 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Thu, 27 Jun 2019 22:44:52 -0400 Subject: [PATCH 2/2] storage: deflake TestReplicaRefreshPendingCommandsTicks Fixes #38525. The test was occasionally failing because the proposals map was being refreshed due to a `reasonNewLeaderOrConfigChange`. This was caused by 1ff3556, which replaced `submitProposalFn` with `proposalBuf.testing.submitProposalFilter`. The new approach to mocking out this function doesn't prevent a Raft ready iteration from being scheduled, so `handleRaftReady` was being called and noticing the leadership change, resulting in the refresh. I've stressed this for 200,000 iterations without a failure. Release note: None --- pkg/storage/replica_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/storage/replica_test.go b/pkg/storage/replica_test.go index 2a979789db86..7a0e463f8552 100644 --- a/pkg/storage/replica_test.go +++ b/pkg/storage/replica_test.go @@ -7539,6 +7539,15 @@ func TestReplicaRefreshPendingCommandsTicks(t *testing.T) { t.Fatal(err) } + // Flush a write all the way through the Raft proposal pipeline. This + // ensures that leadership settles down before we start manually submitting + // proposals and that we don't see any unexpected proposal refreshes due to + // reasons like reasonNewLeaderOrConfigChange. + args := incrementArgs([]byte("a"), 1) + if _, pErr := tc.SendWrapped(&args); pErr != nil { + t.Fatal(pErr) + } + electionTicks := tc.store.cfg.RaftElectionTimeoutTicks { // The verifications of the reproposal counts below rely on r.mu.ticks