Skip to content

Commit

Permalink
kv: alter error for SET TRANSACTION AS OF SYSTEM TIME
Browse files Browse the repository at this point in the history
if reads or writes  are already performed

When a txn performed a read or write before setting up a historical timestamp a crash
report was generated. This commit handles the error case.

Release note: None
  • Loading branch information
e-mbrown committed Apr 21, 2022
1 parent 5c1ae72 commit a5aab2e
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 0 deletions.
17 changes: 17 additions & 0 deletions pkg/kv/kvclient/kvcoord/txn_coord_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -1403,3 +1403,20 @@ func (tc *TxnCoordSender) ClearTxnRetryableErr(ctx context.Context) {
tc.mu.txnState = txnPending
}
}

// HasPerformedReads is part of the TxnSender interface.
func (tc *TxnCoordSender) HasPerformedReads() bool {
tc.mu.Lock()
defer tc.mu.Unlock()
if !tc.interceptorAlloc.txnSpanRefresher.refreshFootprint.empty() {
return true
}
return false
}

// HasPerformedWrites is part of the TxnSender interface.
func (tc *TxnCoordSender) HasPerformedWrites() bool {
tc.mu.Lock()
defer tc.mu.Unlock()
return tc.mu.txn.Sequence != 0
}
10 changes: 10 additions & 0 deletions pkg/kv/mock_transactional_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,16 @@ func (m *MockTransactionalSender) GetTxnRetryableErr(
func (m *MockTransactionalSender) ClearTxnRetryableErr(ctx context.Context) {
}

// HasPerformedReads is part of TxnSenderFactory.
func (m *MockTransactionalSender) HasPerformedReads() bool {
panic("unimplemented")
}

// HasPerformedWrites is part of TxnSenderFactory.
func (m *MockTransactionalSender) HasPerformedWrites() bool {
panic("unimplemented")
}

// MockTxnSenderFactory is a TxnSenderFactory producing MockTxnSenders.
type MockTxnSenderFactory struct {
senderFunc func(context.Context, *roachpb.Transaction, roachpb.BatchRequest) (
Expand Down
6 changes: 6 additions & 0 deletions pkg/kv/sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,12 @@ type TxnSender interface {

// ClearTxnRetryableErr clears the retryable error, if any.
ClearTxnRetryableErr(ctx context.Context)

// HasPerformedReads returns true if a read has been performed.
HasPerformedReads() bool

// HasPerformedWrites returns true if a write has been performed.
HasPerformedWrites() bool
}

// SteppingMode is the argument type to ConfigureStepping.
Expand Down
24 changes: 24 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/as_of
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,27 @@ SELECT * FROM t AS OF SYSTEM TIME with_min_timestamp(statement_timestamp())
skipif config 3node-tenant
statement error pgcode XXC01 with_max_staleness can only be used with a CCL distribution
SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('1s'::interval)

statement ok
BEGIN

statement ok
SELECT * from t

statement error cannot set fixed timestamp, .* already performed reads
SET TRANSACTION AS OF system time '-1s'

statement ok
ROLLBACK

statement ok
BEGIN

statement ok
INSERT INTO t VALUES(1)

statement error cannot set fixed timestamp, .* already performed writes
SET TRANSACTION AS OF system time '-1s'

statement ok
ROLLBACK
17 changes: 17 additions & 0 deletions pkg/sql/txn_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ package sql

import (
"context"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"time"

"github.com/cockroachdb/cockroach/pkg/kv"
Expand Down Expand Up @@ -287,6 +289,21 @@ func (ts *txnState) setHistoricalTimestamp(
) error {
ts.mu.Lock()
defer ts.mu.Unlock()

if ts.mu.txn.Sender().HasPerformedReads() {
return pgerror.Newf(
pgcode.InvalidTransactionState,
"cannot set fixed timestamp, txn %s already performed reads",
ts.mu.txn)
}

if ts.mu.txn.Sender().HasPerformedWrites() {
return pgerror.Newf(
pgcode.InvalidTransactionState,
"cannot set fixed timestamp, txn %s already performed writes",
ts.mu.txn)
}

if err := ts.mu.txn.SetFixedTimestamp(ctx, historicalTimestamp); err != nil {
return err
}
Expand Down

0 comments on commit a5aab2e

Please sign in to comment.