Skip to content

Commit

Permalink
kvserver: unify two request flags
Browse files Browse the repository at this point in the history
isIntentWrite and consultsTSCache are always used in conjunction. It
seems that they must always be used in conjunction. Remove
consultsTSCache.

Release note: None
  • Loading branch information
andreimatei committed Feb 12, 2021
1 parent 0bf4147 commit 456c53f
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 25 deletions.
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/replica_application_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func (r *Replica) tryReproposeWithNewLeaseIndex(
defer untrack(ctx, 0, 0, 0) // covers all error paths below
// The ConsultsTimestampCache condition matches the similar logic for caring
// about the closed timestamp cache in applyTimestampCache().
if p.Request.ConsultsTimestampCache() && p.Request.WriteTimestamp().LessEq(minTS) {
if p.Request.IsIntentWrite() && p.Request.WriteTimestamp().LessEq(minTS) {
// The tracker wants us to forward the request timestamp, but we can't
// do that without re-evaluating, so give up. The error returned here
// will go to back to DistSender, so send something it can digest.
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/replica_tscache.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ func (r *Replica) applyTimestampCache(

for _, union := range ba.Requests {
args := union.GetInner()
if roachpb.ConsultsTimestampCache(args) {
if roachpb.IsIntentWrite(args) {
header := args.Header()

// Forward the timestamp if there's been a more recent read (by someone else).
Expand Down
21 changes: 6 additions & 15 deletions pkg/roachpb/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ const (
isPrefix // requests which should be grouped with the next request in a batch
isUnsplittable // range command that must not be split during sending
skipLeaseCheck // commands which skip the check that the evaluting replica has a valid lease
consultsTSCache // mutating commands which write data at a timestamp
updatesTSCache // commands which update the timestamp cache
updatesTSCacheOnErr // commands which make read data available on errors
needsRefresh // commands which require refreshes to avoid serializable retries
Expand Down Expand Up @@ -166,14 +165,6 @@ func IsRange(args Request) bool {
return (args.flags() & isRange) != 0
}

// ConsultsTimestampCache returns whether the request must consult
// the timestamp cache to determine whether a mutation is safe at
// a proposed timestamp or needs to move to a higher timestamp to
// avoid re-writing history.
func ConsultsTimestampCache(args Request) bool {
return (args.flags() & consultsTSCache) != 0
}

// UpdatesTimestampCache returns whether the command must update
// the timestamp cache in order to set a low water mark for the
// timestamp at which mutations to overlapping key(s) can write
Expand Down Expand Up @@ -1174,7 +1165,7 @@ func (gr *GetRequest) flags() int {
}

func (*PutRequest) flags() int {
return isWrite | isTxn | isLocking | isIntentWrite | consultsTSCache | canBackpressure
return isWrite | isTxn | isLocking | isIntentWrite | canBackpressure
}

// ConditionalPut effectively reads without writing if it hits a
Expand All @@ -1183,7 +1174,7 @@ func (*PutRequest) flags() int {
// they return an error immediately instead of continuing a serializable
// transaction to be retried at end transaction.
func (*ConditionalPutRequest) flags() int {
return isRead | isWrite | isTxn | isLocking | isIntentWrite | consultsTSCache | updatesTSCache | updatesTSCacheOnErr | canBackpressure
return isRead | isWrite | isTxn | isLocking | isIntentWrite | updatesTSCache | updatesTSCacheOnErr | canBackpressure
}

// InitPut, like ConditionalPut, effectively reads without writing if it hits a
Expand All @@ -1192,7 +1183,7 @@ func (*ConditionalPutRequest) flags() int {
// return an error immediately instead of continuing a serializable transaction
// to be retried at end transaction.
func (*InitPutRequest) flags() int {
return isRead | isWrite | isTxn | isLocking | isIntentWrite | consultsTSCache | updatesTSCache | updatesTSCacheOnErr | canBackpressure
return isRead | isWrite | isTxn | isLocking | isIntentWrite | updatesTSCache | updatesTSCacheOnErr | canBackpressure
}

// Increment reads the existing value, but always leaves an intent so
Expand All @@ -1201,11 +1192,11 @@ func (*InitPutRequest) flags() int {
// error immediately instead of continuing a serializable transaction
// to be retried at end transaction.
func (*IncrementRequest) flags() int {
return isRead | isWrite | isTxn | isLocking | isIntentWrite | consultsTSCache | canBackpressure
return isRead | isWrite | isTxn | isLocking | isIntentWrite | canBackpressure
}

func (*DeleteRequest) flags() int {
return isWrite | isTxn | isLocking | isIntentWrite | consultsTSCache | canBackpressure
return isWrite | isTxn | isLocking | isIntentWrite | canBackpressure
}

func (drr *DeleteRangeRequest) flags() int {
Expand All @@ -1232,7 +1223,7 @@ func (drr *DeleteRangeRequest) flags() int {
// it. Note that, even if we didn't update the ts cache, deletes of keys
// that exist would not be lost (since the DeleteRange leaves intents on
// those keys), but deletes of "empty space" would.
return isRead | isWrite | isTxn | isLocking | isIntentWrite | isRange | consultsTSCache | updatesTSCache | needsRefresh | canBackpressure
return isRead | isWrite | isTxn | isLocking | isIntentWrite | isRange | updatesTSCache | needsRefresh | canBackpressure
}

// Note that ClearRange commands cannot be part of a transaction as
Expand Down
8 changes: 0 additions & 8 deletions pkg/roachpb/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,6 @@ func (ba *BatchRequest) IsUnsplittable() bool {
return ba.hasFlag(isUnsplittable)
}

// ConsultsTimestampCache returns whether the request must consult
// the timestamp cache to determine whether a mutation is safe at
// a proposed timestamp or needs to move to a higher timestamp to
// avoid re-writing history.
func (ba *BatchRequest) ConsultsTimestampCache() bool {
return ba.hasFlag(consultsTSCache)
}

// IsSingleRequest returns true iff the BatchRequest contains a single request.
func (ba *BatchRequest) IsSingleRequest() bool {
return len(ba.Requests) == 1
Expand Down

0 comments on commit 456c53f

Please sign in to comment.