Skip to content

Commit

Permalink
Merge #61847
Browse files Browse the repository at this point in the history
61847: kvserver: fix TestEagerReplication r=lunevalex a=lunevalex

Fixes #54646

PR #61644 introduced a bug into the TestEagerReplication test.
Using a single TestServer sets the default zone config to require
only one replica, which means the split in this test does not
trigger a replication attempt. This PR sets the PartOfCluster parameter
on the TestServer to disable that behavior.

Release note: None

Co-authored-by: Alex Lunev <[email protected]>
  • Loading branch information
craig[bot] and lunevalex committed Mar 15, 2021
2 parents f3c8f6e + 97f95cb commit 1bec46e
Showing 1 changed file with 11 additions and 9 deletions.
20 changes: 11 additions & 9 deletions pkg/kv/kvserver/replicate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/server"
Expand All @@ -35,6 +34,10 @@ func TestEagerReplication(t *testing.T) {

ctx := context.Background()
serv, _, _ := serverutils.StartServer(t, base.TestServerArgs{
// Need to trick the server to think it's part of a cluster, otherwise it
// will set the default zone config to require 1 replica and the split
// bellow will not trigger a replication attempt.
PartOfCluster: true,
Knobs: base.TestingKnobs{
Store: &kvserver.StoreTestingKnobs{
// Disable the replica scanner so that we rely on the eager replication code
Expand All @@ -48,27 +51,26 @@ func TestEagerReplication(t *testing.T) {
store, err := s.Stores().GetStore(s.GetFirstStoreID())
require.NoError(t, err)

// Make sure everything goes through the replicate queue, so the start count
// is accurate.
require.NoError(t, store.ForceReplicationScanAndProcess())

// After bootstrap, all of the system ranges should be present in replicate
// queue purgatory (because we only have a single store in the test and thus
// replication cannot succeed).
purgatoryStartCount := store.ReplicateQueuePurgatoryLength()

t.Logf("purgatory start count is %d", purgatoryStartCount)
// Perform a split and check that there's one more range in the purgatory.

key := roachpb.Key("a")
args := adminSplitArgs(key)
_, pErr := kv.SendWrapped(ctx, store.TestSender(), args)
if pErr != nil {
t.Fatal(pErr)
}
_, _, err = s.SplitRange(roachpb.Key("a"))
require.NoError(t, err)

// The addition of replicas to the replicateQueue after a split
// occurs happens after the update of the descriptors in meta2
// leaving a tiny window of time in which the newly split replicas
// will not have been added to purgatory. Thus we loop.
testutils.SucceedsSoon(t, func() error {
expected := purgatoryStartCount + 2
expected := purgatoryStartCount + 1
if n := store.ReplicateQueuePurgatoryLength(); expected != n {
return errors.Errorf("expected %d replicas in purgatory, but found %d", expected, n)
}
Expand Down

0 comments on commit 1bec46e

Please sign in to comment.