Skip to content

Commit

Permalink
storage: factor out common test function
Browse files Browse the repository at this point in the history
  • Loading branch information
@andreimatei
andreimatei committed Nov 7, 2016
1 parent eee24c0 commit 0b4aad8
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 95 deletions.
18 changes: 11 additions & 7 deletions pkg/storage/replica_command.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2368,7 +2368,7 @@ func (r *Replica) AdminSplit(
{
b := txn.NewBatch()
leftDescKey := keys.RangeDescriptorKey(leftDesc.StartKey)
if err := updateRangeDescriptor(b, leftDescKey, desc, &leftDesc); err != nil {
if err := updateRangeDescriptor(b, leftDescKey, desc, leftDesc); err != nil {
return err
}
// Commit this batch first to ensure that the transaction record
Expand Down Expand Up @@ -2406,7 +2406,7 @@ func (r *Replica) AdminSplit(

// Create range descriptor for right hand side of the split.
rightDescKey := keys.RangeDescriptorKey(rightDesc.StartKey)
if err := updateRangeDescriptor(b, rightDescKey, nil, rightDesc); err != nil {
if err := updateRangeDescriptor(b, rightDescKey, nil, *rightDesc); err != nil {
return err
}

Expand Down Expand Up @@ -2840,7 +2840,7 @@ func (r *Replica) AdminMerge(
{
b := txn.NewBatch()
leftDescKey := keys.RangeDescriptorKey(updatedLeftDesc.StartKey)
if err := updateRangeDescriptor(b, leftDescKey, origLeftDesc, &updatedLeftDesc); err != nil {
if err := updateRangeDescriptor(b, leftDescKey, origLeftDesc, updatedLeftDesc); err != nil {
return err
}
// Commit this batch on its own to ensure that the transaction record
Expand Down Expand Up @@ -3248,7 +3248,7 @@ func (r *Replica) ChangeReplicas(

// Important: the range descriptor must be the first thing touched in the transaction
// so the transaction record is co-located with the range being modified.
if err := updateRangeDescriptor(b, descKey, desc, &updatedDesc); err != nil {
if err := updateRangeDescriptor(b, descKey, desc, updatedDesc); err != nil {
return err
}

Expand Down Expand Up @@ -3336,15 +3336,19 @@ func replicaSetsEqual(a, b []roachpb.ReplicaDescriptor) bool {
// range descriptor. This is a last line of defense; other mechanisms should
// prevent rogue replicas from getting this far (see #768).
//
// oldDesc can be nil, meaning that the key is expected to not exist.
//
// Note that in addition to using this method to update the on-disk range
// descriptor, a CommitTrigger must be used to update the in-memory
// descriptor; it will not automatically be copied from newDesc.
// TODO(bdarnell): store the entire RangeDescriptor in the CommitTrigger
// and load it automatically instead of reconstructing individual
// changes.
func updateRangeDescriptor(
b *client.Batch, descKey roachpb.Key, oldDesc,
newDesc *roachpb.RangeDescriptor,
b *client.Batch,
descKey roachpb.Key,
oldDesc *roachpb.RangeDescriptor,
newDesc roachpb.RangeDescriptor,
) error {
if err := newDesc.Validate(); err != nil {
return err
Expand All @@ -3360,7 +3364,7 @@ func updateRangeDescriptor(
}
oldValue = oldBytes
}
newValue, err := protoutil.Marshal(newDesc)
newValue, err := protoutil.Marshal(&newDesc)
if err != nil {
return err
}
Expand Down
149 changes: 61 additions & 88 deletions pkg/storage/replica_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,43 @@ func (tc *testContext) initConfigs(realRange bool, t testing.TB) error {
return nil
}

// addBogusReplicaToRangeDesc modifies the range descriptor to include a second
// replica. This is useful for tests that want to pretend they're transferring
// the range lease away, as the lease can only be obtained by Replicas which are
// part of the range descriptor.
// This is a workaround, but it's sufficient for the purposes of several tests.
func (tc *testContext) addBogusReplicaToRangeDesc(
ctx context.Context,
) (roachpb.ReplicaDescriptor, error) {
secondReplica := roachpb.ReplicaDescriptor{
NodeID: 2,
StoreID: 2,
ReplicaID: 2,
}
oldDesc := *tc.repl.Desc()
newDesc := oldDesc
newDesc.Replicas = append(newDesc.Replicas, secondReplica)
newDesc.NextReplicaID = 3

// Update the "on-disk" replica state, so that it doesn't diverge from what we
// have in memory. At the time of this writing, this is not actually required
// by the tests using this functionality, but it seems sane to do.
ba := client.Batch{
Header: roachpb.Header{Timestamp: tc.Clock().Now()},
}
descKey := keys.RangeDescriptorKey(oldDesc.StartKey)
if err := updateRangeDescriptor(&ba, descKey, &oldDesc, newDesc); err != nil {
return roachpb.ReplicaDescriptor{}, err
}
if err := tc.store.DB().Run(ctx, &ba); err != nil {
return roachpb.ReplicaDescriptor{}, err
}

tc.repl.setDescWithoutProcessUpdate(&newDesc)
tc.repl.assertState(tc.engine)
return secondReplica, nil
}

func newTransaction(
name string,
baseKey roachpb.Key,
Expand Down Expand Up @@ -410,18 +447,10 @@ func TestReplicaReadConsistency(t *testing.T) {
tc := testContext{}
tc.Start(t)
defer tc.Stop()

// Modify range descriptor to include a second replica; range lease can
// only be obtained by Replicas which are part of the range descriptor. This
// workaround is sufficient for the purpose of this test.
secondReplica := roachpb.ReplicaDescriptor{
NodeID: 2,
StoreID: 2,
ReplicaID: 2,
secondReplica, err := tc.addBogusReplicaToRangeDesc(context.TODO())
if err != nil {
t.Fatal(err)
}
rngDesc := *tc.repl.Desc()
rngDesc.Replicas = append(rngDesc.Replicas, secondReplica)
tc.repl.setDescWithoutProcessUpdate(&rngDesc)

gArgs := getArgs(roachpb.Key("a"))

Expand Down Expand Up @@ -457,11 +486,7 @@ func TestReplicaReadConsistency(t *testing.T) {
Start: start,
StartStasis: start.Add(10, 0),
Expiration: start.Add(10, 0),
Replica: roachpb.ReplicaDescriptor{ // a different node
ReplicaID: 2,
NodeID: 2,
StoreID: 2,
},
Replica: secondReplica,
}); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -489,18 +514,10 @@ func TestApplyCmdLeaseError(t *testing.T) {
tc := testContext{}
tc.Start(t)
defer tc.Stop()

// Modify range descriptor to include a second replica; range lease can
// only be obtained by Replicas which are part of the range descriptor. This
// workaround is sufficient for the purpose of this test.
secondReplica := roachpb.ReplicaDescriptor{
NodeID: 2,
StoreID: 2,
ReplicaID: 2,
secondReplica, err := tc.addBogusReplicaToRangeDesc(context.TODO())
if err != nil {
t.Fatal(err)
}
rngDesc := *tc.repl.Desc()
rngDesc.Replicas = append(rngDesc.Replicas, secondReplica)
tc.repl.setDescWithoutProcessUpdate(&rngDesc)

pArgs := putArgs(roachpb.Key("a"), []byte("asd"))

Expand All @@ -511,11 +528,7 @@ func TestApplyCmdLeaseError(t *testing.T) {
Start: start,
StartStasis: start.Add(10, 0),
Expiration: start.Add(10, 0),
Replica: roachpb.ReplicaDescriptor{ // a different node
ReplicaID: 2,
NodeID: 2,
StoreID: 2,
},
Replica: secondReplica,
}); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -568,18 +581,10 @@ func TestReplicaLease(t *testing.T) {
tc := testContext{}
tc.Start(t)
defer tc.Stop()

// Modify range descriptor to include a second replica; leader lease can
// only be obtained by Replicas which are part of the range descriptor. This
// workaround is sufficient for the purpose of this test.
secondReplica := roachpb.ReplicaDescriptor{
NodeID: 2,
StoreID: 2,
ReplicaID: 2,
secondReplica, err := tc.addBogusReplicaToRangeDesc(context.TODO())
if err != nil {
t.Fatal(err)
}
rngDesc := *tc.repl.Desc()
rngDesc.Replicas = append(rngDesc.Replicas, secondReplica)
tc.repl.setDescWithoutProcessUpdate(&rngDesc)

// Test that leases with invalid times are rejected.
// Start leases at a point that avoids overlapping with the existing lease.
Expand Down Expand Up @@ -653,30 +658,18 @@ func TestReplicaNotLeaseHolderError(t *testing.T) {
tc := testContext{}
tc.Start(t)
defer tc.Stop()

// Modify range descriptor to include a second replica; range lease can
// only be obtained by Replicas which are part of the range descriptor. This
// workaround is sufficient for the purpose of this test.
secondReplica := roachpb.ReplicaDescriptor{
NodeID: 2,
StoreID: 2,
ReplicaID: 2,
secondReplica, err := tc.addBogusReplicaToRangeDesc(context.TODO())
if err != nil {
t.Fatal(err)
}
rngDesc := *tc.repl.Desc()
rngDesc.Replicas = append(rngDesc.Replicas, secondReplica)
tc.repl.setDescWithoutProcessUpdate(&rngDesc)

tc.manualClock.Set(leaseExpiry(tc.repl))
now := tc.Clock().Now()
if err := sendLeaseRequest(tc.repl, &roachpb.Lease{
Start: now,
StartStasis: now.Add(10, 0),
Expiration: now.Add(10, 0),
Replica: roachpb.ReplicaDescriptor{
ReplicaID: 2,
NodeID: 2,
StoreID: 2,
},
Replica: secondReplica,
}); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -784,18 +777,10 @@ func TestReplicaGossipConfigsOnLease(t *testing.T) {
tc := testContext{}
tc.Start(t)
defer tc.Stop()

// Modify range descriptor to include a second replica; range lease can
// only be obtained by Replicas which are part of the range descriptor. This
// workaround is sufficient for the purpose of this test.
secondReplica := roachpb.ReplicaDescriptor{
NodeID: 2,
StoreID: 2,
ReplicaID: 2,
secondReplica, err := tc.addBogusReplicaToRangeDesc(context.TODO())
if err != nil {
t.Fatal(err)
}
rngDesc := *tc.repl.Desc()
rngDesc.Replicas = append(rngDesc.Replicas, secondReplica)
tc.repl.setDescWithoutProcessUpdate(&rngDesc)

// Write some arbitrary data in the system config span.
key := keys.MakeTablePrefix(keys.MaxSystemConfigDescID)
Expand Down Expand Up @@ -823,11 +808,7 @@ func TestReplicaGossipConfigsOnLease(t *testing.T) {
Start: now,
StartStasis: now.Add(10, 0),
Expiration: now.Add(10, 0),
Replica: roachpb.ReplicaDescriptor{
ReplicaID: 2,
NodeID: 2,
StoreID: 2,
},
Replica: secondReplica,
}); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -880,18 +861,10 @@ func TestReplicaTSCacheLowWaterOnLease(t *testing.T) {
defer tc.Stop()
// Disable raft log truncation which confuses this test.
tc.store.SetRaftLogQueueActive(false)

// Modify range descriptor to include a second replica; range lease can
// only be obtained by Replicas which are part of the range descriptor. This
// workaround is sufficient for the purpose of this test.
secondReplica := roachpb.ReplicaDescriptor{
NodeID: 2,
StoreID: 2,
ReplicaID: 2,
secondReplica, err := tc.addBogusReplicaToRangeDesc(context.TODO())
if err != nil {
t.Fatal(err)
}
rngDesc := *tc.repl.Desc()
rngDesc.Replicas = append(rngDesc.Replicas, secondReplica)
tc.repl.setDescWithoutProcessUpdate(&rngDesc)

tc.manualClock.Set(leaseExpiry(tc.repl))
now := tc.Clock().Now()
Expand All @@ -914,11 +887,11 @@ func TestReplicaTSCacheLowWaterOnLease(t *testing.T) {
start: now.Add(16, 0), expiration: now.Add(25, 0)},
// Another Store attempts to get the lease, but overlaps. If the
// previous lease expiration had worked, this would have too.
{storeID: tc.store.StoreID() + 1,
{storeID: secondReplica.StoreID,
start: now.Add(29, 0), expiration: now.Add(50, 0),
expErr: "overlaps previous"},
// The other store tries again, this time without the overlap.
{storeID: tc.store.StoreID() + 1,
{storeID: secondReplica.StoreID,
start: now.Add(31, 0), expiration: now.Add(50, 0),
// The cache now moves to this other store, and we can't query that.
expLowWater: 0},
Expand Down

0 comments on commit 0b4aad8

Please sign in to comment.