Skip to content

Commit

Permalink
kvserver: improve test utilities for lease info
Browse files Browse the repository at this point in the history
The TestServer/TestCluster have facilities for querying a range's lease.
This patch addresses two problems with this functionality:
1) If the node being queried has a lease proposal in flight, the
respective functions would return it at the detriment of the current
lease. This behavior is dubious, and indeed not what a test that I'm
writing wants. This patch adds a function that returns both the current
and the prospective next lease.
2) The functions let you ask for the state of a particular node to be
queried. This was buggy, though, because the LeaseInfoRequest could
be transparently forwarded to a different node in case the queried node
doesn't have a replica or has a learner replica. That's confusing as
hell. This patch optionally dissallows that, giving the caller
confidence that they got the state from the node it was interested in.

Release note: None
  • Loading branch information
andreimatei committed Apr 16, 2021
1 parent 4a31ade commit 5f2c296
Show file tree
Hide file tree
Showing 6 changed files with 734 additions and 548 deletions.
2 changes: 2 additions & 0 deletions pkg/kv/kvserver/batcheval/cmd_lease_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ func LeaseInfo(
// If there's a lease request in progress, speculatively return that future
// lease.
reply.Lease = nextLease
reply.CurrentLease = &lease
} else {
reply.Lease = lease
}
reply.EvaluatedBy = cArgs.EvalCtx.StoreID()
return result.Result{}, nil
}
17 changes: 9 additions & 8 deletions pkg/kv/kvserver/client_replica_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1424,10 +1424,11 @@ func TestLeaseNotUsedAfterRestart(t *testing.T) {
// Test that a lease extension (a RequestLeaseRequest that doesn't change the
// lease holder) is not blocked by ongoing reads. The test relies on the fact
// that RequestLeaseRequest does not declare to touch the whole key span of the
// range, and thus don't conflict through the command queue with other reads.
// range, and thus don't conflict through latches with other reads.
func TestLeaseExtensionNotBlockedByRead(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
ctx := context.Background()
readBlocked := make(chan struct{})
cmdFilter := func(fArgs kvserverbase.FilterArgs) *roachpb.Error {
if fArgs.Hdr.UserPriority == 42 {
Expand All @@ -1449,7 +1450,7 @@ func TestLeaseExtensionNotBlockedByRead(t *testing.T) {
},
})
s := srv.(*server.TestServer)
defer s.Stopper().Stop(context.Background())
defer s.Stopper().Stop(ctx)

store, err := s.GetStores().(*kvserver.Stores).GetStore(s.GetFirstStoreID())
if err != nil {
Expand All @@ -1465,7 +1466,7 @@ func TestLeaseExtensionNotBlockedByRead(t *testing.T) {
Key: key,
},
}
if _, pErr := kv.SendWrappedWith(context.Background(), s.DB().NonTransactionalSender(),
if _, pErr := kv.SendWrappedWith(ctx, s.DB().NonTransactionalSender(),
roachpb.Header{UserPriority: 42},
&getReq); pErr != nil {
errChan <- pErr.GoError()
Expand Down Expand Up @@ -1502,21 +1503,21 @@ func TestLeaseExtensionNotBlockedByRead(t *testing.T) {
}

for {
curLease, _, err := s.GetRangeLease(context.Background(), key)
leaseInfo, _, err := s.GetRangeLease(ctx, key, server.AllowQueryToBeForwardedToDifferentNode)
if err != nil {
t.Fatal(err)
}
leaseReq.PrevLease = curLease
leaseReq.PrevLease = leaseInfo.CurrentOrProspective()

_, pErr := kv.SendWrapped(context.Background(), s.DB().NonTransactionalSender(), &leaseReq)
_, pErr := kv.SendWrapped(ctx, s.DB().NonTransactionalSender(), &leaseReq)
if _, ok := pErr.GetDetail().(*roachpb.AmbiguousResultError); ok {
log.Infof(context.Background(), "retrying lease after %s", pErr)
log.Infof(ctx, "retrying lease after %s", pErr)
continue
}
if _, ok := pErr.GetDetail().(*roachpb.LeaseRejectedError); ok {
// Lease rejected? Try again. The extension should work because
// extending is idempotent (assuming the PrevLease matches).
log.Infof(context.Background(), "retrying lease after %s", pErr)
log.Infof(ctx, "retrying lease after %s", pErr)
continue
}
if pErr != nil {
Expand Down
Loading

0 comments on commit 5f2c296

Please sign in to comment.