Skip to content

Commit

Permalink
Merge #60548
Browse files Browse the repository at this point in the history
60548: kvserver: unify two request flags r=andreimatei a=andreimatei

isIntentWrite and consultsTSCache are always used in conjunction. It
seems that they must always be used in conjunction. Remove
consultsTSCache.

Release note: None

Co-authored-by: Andrei Matei <[email protected]>
  • Loading branch information
craig[bot] and andreimatei committed Feb 16, 2021
2 parents aa8f949 + b956057 commit e1911bc
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 26 deletions.
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/replica_application_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ func (r *Replica) tryReproposeWithNewLeaseIndex(

minTS, untrack := r.store.cfg.ClosedTimestamp.Tracker.Track(ctx)
defer untrack(ctx, 0, 0, 0) // covers all error paths below
// The ConsultsTimestampCache condition matches the similar logic for caring
// The IsIntentWrite 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 @@ -285,7 +285,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 e1911bc

Please sign in to comment.