Skip to content

Commit

Permalink
kvserver: fix TestEagerReplication
Browse files Browse the repository at this point in the history
Fixes cockroachdb#54646

PR cockroachdb#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
  • Loading branch information
lunevalex committed Mar 11, 2021
1 parent 3bae381 commit 97f95cb
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 @@ -33,6 +32,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 @@ -46,27 +49,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 97f95cb

Please sign in to comment.