Skip to content

Commit

Permalink
roachpb: s/CanCommitAtHigherTimestamp/DeprecatedCanCommitAtHigherTime…
Browse files Browse the repository at this point in the history
…stamp/

The field is now deprecated.
  • Loading branch information
nvanbenschoten committed Aug 21, 2020
1 parent 4189938 commit 78a9e17
Show file tree
Hide file tree
Showing 9 changed files with 706 additions and 703 deletions.
22 changes: 11 additions & 11 deletions c-deps/libroach/protos/roachpb/api.pb.cc

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 15 additions & 15 deletions c-deps/libroach/protos/roachpb/api.pb.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 10 additions & 9 deletions pkg/kv/kvclient/kvcoord/dist_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -642,11 +642,11 @@ func splitBatchAndCheckForRefreshSpans(
ba.CanForwardReadTimestamp = false

// If the final part contains an EndTxn request, unset its
// CanCommitAtHigherTimestamp flag as well.
// DeprecatedCanCommitAtHigherTimestamp flag as well.
lastPart := parts[len(parts)-1]
if et := lastPart[len(lastPart)-1].GetEndTxn(); et != nil {
etCopy := *et
etCopy.CanCommitAtHigherTimestamp = false
etCopy.DeprecatedCanCommitAtHigherTimestamp = false
lastPart = append([]roachpb.RequestUnion(nil), lastPart...)
lastPart[len(lastPart)-1].MustSetInner(&etCopy)
parts[len(parts)-1] = lastPart
Expand Down Expand Up @@ -674,13 +674,14 @@ func unsetCanForwardReadTimestampFlag(ctx context.Context, ba *roachpb.BatchRequ
// Unset the flag.
ba.CanForwardReadTimestamp = false

// We would need to also unset the CanCommitAtHigherTimestamp flag
// on any EndTxn request in the batch, but it turns out that because
// we call this function when a batch is split across ranges, we'd
// already have bailed if the EndTxn wasn't a parallel commit — and
// if it was a parallel commit then we must not have any requests
// that need to refresh (see txnCommitter.canCommitInParallel).
// Assert this for our own sanity.
// We would need to also unset the DeprecatedCanCommitAtHigherTimestamp
// flag on any EndTxn request in the batch, but it turns out that
// because we call this function when a batch is split across
// ranges, we'd already have bailed if the EndTxn wasn't a parallel
// commit — and if it was a parallel commit then we must not have
// any requests that need to refresh (see
// txnCommitter.canCommitInParallel). Assert this for our own
// sanity.
if _, ok := ba.GetArg(roachpb.EndTxn); ok {
log.Fatalf(ctx, "batch unexpected contained requests "+
"that need to refresh and an EndTxn request: %s", ba.String())
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvclient/kvcoord/txn_interceptor_committer.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ func makeTxnCommitExplicitLocked(
et := roachpb.EndTxnRequest{Commit: true}
et.Key = txn.Key
et.LockSpans = lockSpans
et.CanCommitAtHigherTimestamp = canFwdRTS
et.DeprecatedCanCommitAtHigherTimestamp = canFwdRTS
ba.Add(&et)

_, pErr := s.SendLocked(ctx, ba)
Expand Down
12 changes: 6 additions & 6 deletions pkg/kv/kvclient/kvcoord/txn_interceptor_committer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,11 +354,11 @@ func TestTxnCommitterAsyncExplicitCommitTask(t *testing.T) {
etArgs.InFlightWrites = []roachpb.SequencedWrite{{Key: keyA, Sequence: 1}}
ba.Add(&putArgs, &etArgs)

// Set the CanForwardReadTimestamp and CanCommitAtHigherTimestamp flags so
// we can make sure that these are propagated to the async explicit commit
// task.
// Set the CanForwardReadTimestamp and DeprecatedCanCommitAtHigherTimestamp
// flags so we can make sure that these are propagated to the async explicit
// commit task.
ba.Header.CanForwardReadTimestamp = true
etArgs.CanCommitAtHigherTimestamp = true
etArgs.DeprecatedCanCommitAtHigherTimestamp = true

explicitCommitCh := make(chan struct{})
mockSender.MockSend(func(ba roachpb.BatchRequest) (*roachpb.BatchResponse, *roachpb.Error) {
Expand All @@ -369,7 +369,7 @@ func TestTxnCommitterAsyncExplicitCommitTask(t *testing.T) {

et := ba.Requests[1].GetInner().(*roachpb.EndTxnRequest)
require.True(t, et.Commit)
require.True(t, et.CanCommitAtHigherTimestamp)
require.True(t, et.DeprecatedCanCommitAtHigherTimestamp)
require.Len(t, et.InFlightWrites, 1)
require.Equal(t, roachpb.SequencedWrite{Key: keyA, Sequence: 1}, et.InFlightWrites[0])

Expand All @@ -388,7 +388,7 @@ func TestTxnCommitterAsyncExplicitCommitTask(t *testing.T) {

et := ba.Requests[0].GetInner().(*roachpb.EndTxnRequest)
require.True(t, et.Commit)
require.True(t, et.CanCommitAtHigherTimestamp)
require.True(t, et.DeprecatedCanCommitAtHigherTimestamp)
require.Len(t, et.InFlightWrites, 0)

br = ba.CreateReply()
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (sr *txnSpanRefresher) SendLocked(
ba.CanForwardReadTimestamp = sr.canForwardReadTimestampWithoutRefresh(ba.Txn)
if rArgs, hasET := ba.GetArg(roachpb.EndTxn); hasET {
et := rArgs.(*roachpb.EndTxnRequest)
et.CanCommitAtHigherTimestamp = ba.CanForwardReadTimestamp
et.DeprecatedCanCommitAtHigherTimestamp = ba.CanForwardReadTimestamp
}

// Attempt a refresh before sending the batch.
Expand Down
27 changes: 14 additions & 13 deletions pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1083,8 +1083,8 @@ func TestTxnSpanRefresherAssignsCanCommitAtHigherTimestamp(t *testing.T) {
keyA, keyB := roachpb.Key("a"), roachpb.Key("b")
keyC, keyD := roachpb.Key("c"), roachpb.Key("d")

// Send an EndTxn request. Should set CanCommitAtHigherTimestamp and
// CanForwardReadTimestamp flags.
// Send an EndTxn request. Should set DeprecatedCanCommitAtHigherTimestamp
// and CanForwardReadTimestamp flags.
var ba roachpb.BatchRequest
ba.Header = roachpb.Header{Txn: &txn}
ba.Add(&roachpb.EndTxnRequest{})
Expand All @@ -1093,7 +1093,7 @@ func TestTxnSpanRefresherAssignsCanCommitAtHigherTimestamp(t *testing.T) {
require.Len(t, ba.Requests, 1)
require.True(t, ba.CanForwardReadTimestamp)
require.IsType(t, &roachpb.EndTxnRequest{}, ba.Requests[0].GetInner())
require.True(t, ba.Requests[0].GetEndTxn().CanCommitAtHigherTimestamp)
require.True(t, ba.Requests[0].GetEndTxn().DeprecatedCanCommitAtHigherTimestamp)

br := ba.CreateReply()
br.Txn = ba.Txn
Expand All @@ -1105,8 +1105,8 @@ func TestTxnSpanRefresherAssignsCanCommitAtHigherTimestamp(t *testing.T) {
require.NotNil(t, br)

// Send an EndTxn request for a transaction with a fixed commit timestamp.
// Should NOT set CanCommitAtHigherTimestamp and CanForwardReadTimestamp
// flags.
// Should NOT set DeprecatedCanCommitAtHigherTimestamp and
// CanForwardReadTimestamp flags.
txnFixed := txn.Clone()
txnFixed.CommitTimestampFixed = true
var baFixed roachpb.BatchRequest
Expand All @@ -1117,7 +1117,7 @@ func TestTxnSpanRefresherAssignsCanCommitAtHigherTimestamp(t *testing.T) {
require.Len(t, ba.Requests, 1)
require.False(t, ba.CanForwardReadTimestamp)
require.IsType(t, &roachpb.EndTxnRequest{}, ba.Requests[0].GetInner())
require.False(t, ba.Requests[0].GetEndTxn().CanCommitAtHigherTimestamp)
require.False(t, ba.Requests[0].GetEndTxn().DeprecatedCanCommitAtHigherTimestamp)

br = ba.CreateReply()
br.Txn = ba.Txn
Expand All @@ -1140,16 +1140,16 @@ func TestTxnSpanRefresherAssignsCanCommitAtHigherTimestamp(t *testing.T) {
require.Equal(t, []roachpb.Span{scanArgs.Span()}, tsr.refreshFootprint.asSlice())
require.False(t, tsr.refreshInvalid)

// Send another EndTxn request. Should NOT set CanCommitAtHigherTimestamp
// and CanForwardReadTimestamp flags.
// Send another EndTxn request. Should NOT set
// DeprecatedCanCommitAtHigherTimestamp and CanForwardReadTimestamp flags.
ba.Requests = nil
ba.Add(&roachpb.EndTxnRequest{})

mockSender.MockSend(func(ba roachpb.BatchRequest) (*roachpb.BatchResponse, *roachpb.Error) {
require.Len(t, ba.Requests, 1)
require.False(t, ba.CanForwardReadTimestamp)
require.IsType(t, &roachpb.EndTxnRequest{}, ba.Requests[0].GetInner())
require.False(t, ba.Requests[0].GetEndTxn().CanCommitAtHigherTimestamp)
require.False(t, ba.Requests[0].GetEndTxn().DeprecatedCanCommitAtHigherTimestamp)

br = ba.CreateReply()
br.Txn = ba.Txn
Expand All @@ -1171,15 +1171,15 @@ func TestTxnSpanRefresherAssignsCanCommitAtHigherTimestamp(t *testing.T) {
require.NotNil(t, br)

// Send another EndTxn request. Still should NOT set
// CanCommitAtHigherTimestamp and CanForwardReadTimestamp flags.
// DeprecatedCanCommitAtHigherTimestamp and CanForwardReadTimestamp flags.
ba.Requests = nil
ba.Add(&roachpb.EndTxnRequest{})

mockSender.MockSend(func(ba roachpb.BatchRequest) (*roachpb.BatchResponse, *roachpb.Error) {
require.Len(t, ba.Requests, 1)
require.False(t, ba.CanForwardReadTimestamp)
require.IsType(t, &roachpb.EndTxnRequest{}, ba.Requests[0].GetInner())
require.False(t, ba.Requests[0].GetEndTxn().CanCommitAtHigherTimestamp)
require.False(t, ba.Requests[0].GetEndTxn().DeprecatedCanCommitAtHigherTimestamp)

br = ba.CreateReply()
br.Txn = ba.Txn
Expand All @@ -1191,15 +1191,16 @@ func TestTxnSpanRefresherAssignsCanCommitAtHigherTimestamp(t *testing.T) {
require.NotNil(t, br)

// Increment the transaction's epoch and send another EndTxn request. Should
// set CanCommitAtHigherTimestamp and CanForwardReadTimestamp flags.
// set DeprecatedCanCommitAtHigherTimestamp and CanForwardReadTimestamp
// flags.
ba.Requests = nil
ba.Add(&roachpb.EndTxnRequest{})

mockSender.MockSend(func(ba roachpb.BatchRequest) (*roachpb.BatchResponse, *roachpb.Error) {
require.Len(t, ba.Requests, 1)
require.True(t, ba.CanForwardReadTimestamp)
require.IsType(t, &roachpb.EndTxnRequest{}, ba.Requests[0].GetInner())
require.True(t, ba.Requests[0].GetEndTxn().CanCommitAtHigherTimestamp)
require.True(t, ba.Requests[0].GetEndTxn().DeprecatedCanCommitAtHigherTimestamp)

br = ba.CreateReply()
br.Txn = ba.Txn
Expand Down
Loading

0 comments on commit 78a9e17

Please sign in to comment.