Skip to content

Commit

Permalink
kv: add PrevLease check in RequestLease
Browse files Browse the repository at this point in the history
This commit adds a check that `args.PrevLease` is equivalent to
`cArgs.EvalCtx.GetLease()` to RequestLease. This ensures that the
validation here is consistent with the validation that was performed
when the lease request was constructed.

Release note: None
Epic: None
  • Loading branch information
nvanbenschoten committed May 2, 2024
1 parent 6dd54b4 commit 58a0a17
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
10 changes: 10 additions & 0 deletions pkg/kv/kvserver/batcheval/cmd_lease_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ package batcheval

import (
"context"
"fmt"
"time"

"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
Expand Down Expand Up @@ -67,6 +68,15 @@ func RequestLease(
Requested: args.Lease,
}

// However, we verify that the current lease's sequence number and proposed
// timestamp match the provided PrevLease. This ensures that the validation
// here is consistent with the validation that was performed when the lease
// request was constructed.
if prevLease.Sequence != args.PrevLease.Sequence || !prevLease.ProposedTS.Equal(args.PrevLease.ProposedTS) {
rErr.Message = fmt.Sprintf("expected previous lease %s, found %s", args.PrevLease, prevLease)
return newFailedLeaseTrigger(false /* isTransfer */), rErr
}

// MIGRATION(tschottdorf): needed to apply Raft commands which got proposed
// before the StartStasis field was introduced.
newLease := args.Lease
Expand Down
5 changes: 3 additions & 2 deletions pkg/kv/kvserver/replica_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,8 @@ func TestReplicaLease(t *testing.T) {
kvpb.AdmissionHeader{},
),
Args: &kvpb.RequestLeaseRequest{
Lease: lease,
Lease: lease,
PrevLease: tc.repl.CurrentLeaseStatus(ctx).Lease,
},
}, &kvpb.RequestLeaseResponse{}); !testutils.IsError(err, "replica not found") {
t.Fatalf("unexpected error: %+v", err)
Expand Down Expand Up @@ -1317,7 +1318,7 @@ func TestReplicaLeaseRejectUnknownRaftNodeID(t *testing.T) {
st := tc.repl.CurrentLeaseStatus(ctx)
ba := &kvpb.BatchRequest{}
ba.Timestamp = tc.repl.store.Clock().Now()
ba.Add(&kvpb.RequestLeaseRequest{Lease: *lease})
ba.Add(&kvpb.RequestLeaseRequest{Lease: *lease, PrevLease: st.Lease})
_, tok := tc.repl.mu.proposalBuf.TrackEvaluatingRequest(ctx, hlc.MinTimestamp)
ch, _, _, _, pErr := tc.repl.evalAndPropose(ctx, ba, allSpansGuard(), &st, uncertainty.Interval{}, tok.Move(ctx))
if pErr == nil {
Expand Down

0 comments on commit 58a0a17

Please sign in to comment.