Skip to content

Commit

Permalink
kv: clean up BatchRequest.IsLeaseRequest
Browse files Browse the repository at this point in the history
This commit cleans up `BatchRequest.IsLeaseRequest` by replacing its
internals with a call to `isSingleRequestWithMethod(RequestLease)` and
renaming it to mirror the rest of the `BatchRequest.IsSingleXYZRequest`
methods.
  • Loading branch information
nvanbenschoten committed Jun 12, 2022
1 parent 30a55f4 commit 09e60c0
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 19 deletions.
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/replica_application_state_machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -1478,7 +1478,7 @@ func (s *closedTimestampSetterInfo) record(cmd *replicatedCmd, lease *roachpb.Le
s.merge = true
}
}
} else if req.IsLeaseRequest() {
} else if req.IsSingleRequestLeaseRequest() {
// Make a deep copy since we're not allowed to hold on to request
// memory.
lr, _ := req.GetArg(roachpb.RequestLease)
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/replica_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ func (r *Replica) evaluateProposal(

// Set the proposal's replicated result, which contains metadata and
// side-effects that are to be replicated to all replicas.
res.Replicated.IsLeaseRequest = ba.IsLeaseRequest()
res.Replicated.IsLeaseRequest = ba.IsSingleRequestLeaseRequest()
if ba.AppliesTimestampCache() {
res.Replicated.WriteTimestamp = ba.WriteTimestamp()
} else {
Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvserver/replica_proposal_buf.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ func (b *propBuf) FlushLockedWithRaftGroup(
// Lease extensions for a currently held lease always go through, to
// keep the lease alive until the normal lease transfer mechanism can
// colocate it with the leader.
if !leaderInfo.iAmTheLeader && p.Request.IsLeaseRequest() {
if !leaderInfo.iAmTheLeader && p.Request.IsSingleRequestLeaseRequest() {
leaderKnownAndEligible := leaderInfo.leaderKnown && leaderInfo.leaderEligibleForLease
ownsCurrentLease := b.p.ownsValidLeaseRLocked(ctx, b.clock.NowAsClockTimestamp())
if leaderKnownAndEligible && !ownsCurrentLease && !b.testing.allowLeaseProposalWhenNotLeader {
Expand Down Expand Up @@ -618,7 +618,7 @@ func (b *propBuf) allocateLAIAndClosedTimestampLocked(
// replays (though they could in principle also be handled like lease
// requests).
var lai uint64
if !p.Request.IsLeaseRequest() {
if !p.Request.IsSingleRequestLeaseRequest() {
b.assignedLAI++
lai = b.assignedLAI
}
Expand Down Expand Up @@ -673,7 +673,7 @@ func (b *propBuf) allocateLAIAndClosedTimestampLocked(
// timestamps carried by lease requests, make sure to resurrect the old
// TestRejectedLeaseDoesntDictateClosedTimestamp and protect against that
// scenario.
if p.Request.IsLeaseRequest() {
if p.Request.IsSingleRequestLeaseRequest() {
return lai, hlc.Timestamp{}, nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/replica_rangefeed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,7 @@ func TestRangefeedCheckpointsRecoverFromLeaseExpiration(t *testing.T) {
return nil
}
nudged := atomic.LoadInt64(&nudgeSeen)
if ba.IsLeaseRequest() && (nudged == 1) {
if ba.IsSingleRequestLeaseRequest() && (nudged == 1) {
return nil
}
log.Infof(ctx, "test rejecting request: %s", ba)
Expand Down Expand Up @@ -1374,7 +1374,7 @@ func TestNewRangefeedForceLeaseRetry(t *testing.T) {
return nil
}
nudged := atomic.LoadInt64(&nudgeSeen)
if ba.IsLeaseRequest() && (nudged == 1) {
if ba.IsSingleRequestLeaseRequest() && (nudged == 1) {
return nil
}
log.Infof(ctx, "test rejecting request: %s", ba)
Expand Down
21 changes: 9 additions & 12 deletions pkg/roachpb/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,18 +147,6 @@ func (ba *BatchRequest) UpdateTxn(o *Transaction) {
ba.Txn = clonedTxn
}

// IsLeaseRequest returns whether the batch consists of a single RequestLease
// request. Note that TransferLease requests return false.
// RequestLease requests are special because they're the only type of requests a
// non-lease-holder can propose.
func (ba *BatchRequest) IsLeaseRequest() bool {
if !ba.IsSingleRequest() {
return false
}
_, ok := ba.GetArg(RequestLease)
return ok
}

// AppliesTimestampCache returns whether the command is a write that applies the
// timestamp cache (and closed timestamp), possibly pushing its write timestamp
// into the future to avoid re-writing history.
Expand Down Expand Up @@ -228,6 +216,15 @@ func (ba *BatchRequest) isSingleRequestWithMethod(m Method) bool {
return ba.IsSingleRequest() && ba.Requests[0].GetInner().Method() == m
}

// IsSingleRequestLeaseRequest returns true iff the batch contains a single
// request, and that request is a RequestLease. Note that TransferLease requests
// return false.
// RequestLease requests are special because they're the only type of requests a
// non-lease-holder can propose.
func (ba *BatchRequest) IsSingleRequestLeaseRequest() bool {
return ba.isSingleRequestWithMethod(RequestLease)
}

// IsSingleTransferLeaseRequest returns true iff the batch contains a single
// request, and that request is a TransferLease.
func (ba *BatchRequest) IsSingleTransferLeaseRequest() bool {
Expand Down

0 comments on commit 09e60c0

Please sign in to comment.