Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvserver: fix a reproposal check #59502

Merged
merged 1 commit into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions pkg/kv/kvserver/replica_application_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +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
// NB: p.Request.Timestamp reflects the action of ba.SetActiveTimestamp.
if p.Request.Timestamp.Less(minTS) {
// 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) {
// 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/roachpb/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func IsRange(args Request) bool {
return (args.flags() & isRange) != 0
}

// ConsultsTimestampCache returns whether the command must consult
// 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.
Expand Down
21 changes: 21 additions & 0 deletions pkg/roachpb/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,19 @@ func (ba BatchRequest) EarliestActiveTimestamp() hlc.Timestamp {
return ts
}

// WriteTimestamp returns the timestamps at which this request is writing. For
// non-transactional requests, this is the same as the read timestamp. For
// transactional requests, the write timestamp can be higher until commit time.
//
// This should only be called after SetActiveTimestamp().
func (ba *BatchRequest) WriteTimestamp() hlc.Timestamp {
ts := ba.Timestamp
if ba.Txn != nil {
ts.Forward(ba.Txn.WriteTimestamp)
}
return ts
}

// UpdateTxn updates the batch transaction from the supplied one in
// a copy-on-write fashion, i.e. without mutating an existing
// Transaction struct.
Expand Down Expand Up @@ -156,6 +169,14 @@ 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