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

kv: remove ShouldWinOriginTimestampTie option fromcput #143096

Merged
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: 1 addition & 4 deletions pkg/kv/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,9 +578,7 @@ func (b *Batch) CPutAllowingIfNotExists(key, value interface{}, expValue []byte)
// This is used by logical data replication and other uses of this API
// are discouraged since the semantics are subject to change as
// required by that feature.
func (b *Batch) CPutWithOriginTimestamp(
key, value interface{}, expValue []byte, ts hlc.Timestamp, shouldWinTie bool,
) {
func (b *Batch) CPutWithOriginTimestamp(key, value interface{}, expValue []byte, ts hlc.Timestamp) {
k, err := marshalKey(key)
if err != nil {
b.initResult(0, 1, notRaw, err)
Expand All @@ -594,7 +592,6 @@ func (b *Batch) CPutWithOriginTimestamp(
}
r := kvpb.NewConditionalPut(k, v, expValue, false)
r.(*kvpb.ConditionalPutRequest).OriginTimestamp = ts
r.(*kvpb.ConditionalPutRequest).ShouldWinOriginTimestampTie = shouldWinTie
b.appendReqs(r)
b.approxMutationReqBytes += len(k) + len(v.RawBytes)
b.initResult(1, 1, notRaw, nil)
Expand Down
8 changes: 1 addition & 7 deletions pkg/kv/kvpb/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -368,13 +368,7 @@ message ConditionalPutRequest {
// Used by logical data replication.
util.hlc.Timestamp origin_timestamp = 8 [(gogoproto.nullable) = false];

// ShouldWinOriginTimestampTie, if true, indicates that if the "comparison
// timestamp" (see comment on OriginTimestamp) and OriginTimestamp are equal,
// then the ConditionalPut should succeed (assuming the expected and actual
// bytes also match).
//
// This must only be used in conjunction with OriginTimestamp.
bool should_win_origin_timestamp_tie = 9;
reserved 9;
}

// A ConditionalPutResponse is the return value from the
Expand Down
5 changes: 2 additions & 3 deletions pkg/kv/kvserver/batcheval/cmd_conditional_put.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,8 @@ func ConditionalPut(
TargetLockConflictBytes: storage.TargetBytesPerLockConflictError.Get(&cArgs.EvalCtx.ClusterSettings().SV),
Category: fs.BatchEvalReadCategory,
},
AllowIfDoesNotExist: storage.CPutMissingBehavior(args.AllowIfDoesNotExist),
OriginTimestamp: args.OriginTimestamp,
ShouldWinOriginTimestampTie: args.ShouldWinOriginTimestampTie,
AllowIfDoesNotExist: storage.CPutMissingBehavior(args.AllowIfDoesNotExist),
OriginTimestamp: args.OriginTimestamp,
}

var err error
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/colenc/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ type noopPutter struct{}

func (n *noopPutter) CPut(key, value interface{}, expValue []byte) {}
func (n *noopPutter) CPutWithOriginTimestamp(
key, value interface{}, expValue []byte, ts hlc.Timestamp, shouldWinTie bool,
key, value interface{}, expValue []byte, ts hlc.Timestamp,
) {
}
func (n *noopPutter) Put(key, value interface{}) {}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/colenc/encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ func (c *capturePutter) CPut(key, value interface{}, expValue []byte) {
}

func (c *capturePutter) CPutWithOriginTimestamp(
key, value interface{}, expValue []byte, ts hlc.Timestamp, shouldWinTie bool,
key, value interface{}, expValue []byte, ts hlc.Timestamp,
) {
colexecerror.InternalError(errors.New("unimplemented"))
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/sql/row/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,6 @@ func (rh *RowHelper) deleteIndexEntry(
// option set.
type OriginTimestampCPutHelper struct {
OriginTimestamp hlc.Timestamp
ShouldWinTie bool
// PreviousWasDeleted is used to indicate that the expected
// value is non-existent. This is helpful in Deleter to
// distinguish between a delete of a value that had no columns
Expand All @@ -551,7 +550,7 @@ func (oh *OriginTimestampCPutHelper) CPutFn(
if traceKV {
log.VEventfDepth(ctx, 1, 2, "CPutWithOriginTimestamp %s -> %s @ %s", *key, value.PrettyPrint(), oh.OriginTimestamp)
}
b.CPutWithOriginTimestamp(key, value, expVal, oh.OriginTimestamp, oh.ShouldWinTie)
b.CPutWithOriginTimestamp(key, value, expVal, oh.OriginTimestamp)
}

func (oh *OriginTimestampCPutHelper) DelWithCPut(
Expand All @@ -560,7 +559,7 @@ func (oh *OriginTimestampCPutHelper) DelWithCPut(
if traceKV {
log.VEventfDepth(ctx, 1, 2, "CPutWithOriginTimestamp %s -> nil (delete) @ %s", key, oh.OriginTimestamp)
}
b.CPutWithOriginTimestamp(key, nil, expVal, oh.OriginTimestamp, oh.ShouldWinTie)
b.CPutWithOriginTimestamp(key, nil, expVal, oh.OriginTimestamp)
}

func FetchSpecRequiresRawMVCCValues(spec fetchpb.IndexFetchSpec) bool {
Expand Down
14 changes: 7 additions & 7 deletions pkg/sql/row/putter.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
// encoding logic to kv.Batch.
type Putter interface {
CPut(key, value interface{}, expValue []byte)
CPutWithOriginTimestamp(key, value interface{}, expValue []byte, ts hlc.Timestamp, shouldWinTie bool)
CPutWithOriginTimestamp(key, value interface{}, expValue []byte, ts hlc.Timestamp)
Put(key, value interface{})
PutMustAcquireExclusiveLock(key, value interface{})
Del(key ...interface{})
Expand All @@ -47,10 +47,10 @@ func (t *TracePutter) CPut(key, value interface{}, expValue []byte) {
}

func (t *TracePutter) CPutWithOriginTimestamp(
key, value interface{}, expValue []byte, ts hlc.Timestamp, shouldWinTie bool,
key, value interface{}, expValue []byte, ts hlc.Timestamp,
) {
log.VEventfDepth(t.Ctx, 1, 2, "CPutWithOriginTimestamp %v -> %v @ %v", key, value, ts)
t.Putter.CPutWithOriginTimestamp(key, value, expValue, ts, shouldWinTie)
t.Putter.CPutWithOriginTimestamp(key, value, expValue, ts)
}

func (t *TracePutter) Put(key, value interface{}) {
Expand Down Expand Up @@ -182,9 +182,9 @@ func (s *SortingPutter) CPut(key, value interface{}, expValue []byte) {
}

func (s *SortingPutter) CPutWithOriginTimestamp(
key, value interface{}, expValue []byte, ts hlc.Timestamp, shouldWinTie bool,
key, value interface{}, expValue []byte, ts hlc.Timestamp,
) {
s.Putter.CPutWithOriginTimestamp(key, value, expValue, ts, shouldWinTie)
s.Putter.CPutWithOriginTimestamp(key, value, expValue, ts)
}

func (s *SortingPutter) Put(key, value interface{}) {
Expand Down Expand Up @@ -279,9 +279,9 @@ type KVBatchAdapter struct {
var _ Putter = &KVBatchAdapter{}

func (k *KVBatchAdapter) CPutWithOriginTimestamp(
key, value interface{}, expValue []byte, originTimestamp hlc.Timestamp, shouldWinTie bool,
key, value interface{}, expValue []byte, originTimestamp hlc.Timestamp,
) {
k.Batch.CPutWithOriginTimestamp(key, value, expValue, originTimestamp, shouldWinTie)
k.Batch.CPutWithOriginTimestamp(key, value, expValue, originTimestamp)
}

func (k *KVBatchAdapter) CPut(key, value interface{}, expValue []byte) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/row/row_converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (c KVInserter) PutMustAcquireExclusiveLock(key, value interface{}) {
panic(errors.AssertionFailedf("unimplemented"))
}
func (c KVInserter) CPutWithOriginTimestamp(
key, value interface{}, expValue []byte, ts hlc.Timestamp, shouldWinTie bool,
key, value interface{}, expValue []byte, ts hlc.Timestamp,
) {
panic(errors.AssertionFailedf("unimplemented"))
}
Expand Down
10 changes: 1 addition & 9 deletions pkg/storage/mvcc.go
Original file line number Diff line number Diff line change
Expand Up @@ -2897,13 +2897,6 @@ type ConditionalPutWriteOptions struct {
// See the comment on the OriginTimestamp field of
// kvpb.ConditionalPutRequest for more details.
OriginTimestamp hlc.Timestamp
// ShouldWinOriginTimestampTie indicates whether the value should be
// accepted if the origin timestamp is the same as the
// origin_timestamp/mvcc_timestamp of the existing value.
//
// See the comment on the ShouldWinOriginTimestampTie field of
// kvpb.ConditionalPutRequest for more details.
ShouldWinOriginTimestampTie bool
}

// MVCCConditionalPut sets the value for a specified key only if the expected
Expand Down Expand Up @@ -3033,8 +3026,7 @@ func mvccConditionalPutUsingIter(
}
} else {
valueFn = func(existVal optionalValue) (roachpb.Value, error) {
originTSWinner, existTS := existVal.isOriginTimestampWinner(opts.OriginTimestamp,
opts.ShouldWinOriginTimestampTie)
originTSWinner, existTS := existVal.isOriginTimestampWinner(opts.OriginTimestamp, false)
if !originTSWinner {
return roachpb.Value{}, &kvpb.ConditionFailedError{
OriginTimestampOlderThan: existTS,
Expand Down