Skip to content

Commit

Permalink
kv,storage,sql: purge snapshot isolation
Browse files Browse the repository at this point in the history
Snapshot isolation was disabled in v2.1, so it is safe to remove all
code paths that handle snapshot isolation in v2.2.

This is a necessary precursor to cockroachdb#32487.

Release note: None
  • Loading branch information
benesch committed Dec 5, 2018
1 parent 91e1d7b commit d94f47c
Show file tree
Hide file tree
Showing 33 changed files with 931 additions and 1,589 deletions.
29 changes: 0 additions & 29 deletions c-deps/libroach/protos/roachpb/data.pb.cc

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

21 changes: 0 additions & 21 deletions c-deps/libroach/protos/roachpb/data.pb.h

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

1 change: 0 additions & 1 deletion c-deps/libroach/protos/storage/engine/enginepb/mvcc3.pb.cc

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

3 changes: 1 addition & 2 deletions c-deps/libroach/protos/storage/engine/enginepb/mvcc3.pb.h

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

42 changes: 12 additions & 30 deletions pkg/internal/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/internal/client"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb"
"github.com/cockroachdb/cockroach/pkg/storage/storagebase"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
Expand Down Expand Up @@ -129,20 +128,15 @@ func TestClientRetryNonTxn(t *testing.T) {

testCases := []struct {
args roachpb.Request
isolation enginepb.IsolationType
canPush bool
expAttempts int
}{
// Write/write conflicts.
{&roachpb.PutRequest{}, enginepb.SNAPSHOT, true, 2},
{&roachpb.PutRequest{}, enginepb.SERIALIZABLE, true, 2},
{&roachpb.PutRequest{}, enginepb.SNAPSHOT, false, 1},
{&roachpb.PutRequest{}, enginepb.SERIALIZABLE, false, 1},
{&roachpb.PutRequest{}, true, 2},
{&roachpb.PutRequest{}, false, 1},
// Read/write conflicts.
{&roachpb.GetRequest{}, enginepb.SNAPSHOT, true, 1},
{&roachpb.GetRequest{}, enginepb.SERIALIZABLE, true, 1},
{&roachpb.GetRequest{}, enginepb.SNAPSHOT, false, 1},
{&roachpb.GetRequest{}, enginepb.SERIALIZABLE, false, 1},
{&roachpb.GetRequest{}, true, 1},
{&roachpb.GetRequest{}, false, 1},
}
// Lay down a write intent using a txn and attempt to access the same
// key from our test client, with priorities set up so that the Push
Expand All @@ -160,12 +154,6 @@ func TestClientRetryNonTxn(t *testing.T) {
t.Fatal(err)
}
}
if test.isolation == enginepb.SNAPSHOT {
if err := txn.SetIsolation(enginepb.SNAPSHOT); err != nil {
return err
}
}

count++
// Lay down the intent.
if err := txn.Put(ctx, key, "txn-value"); err != nil {
Expand Down Expand Up @@ -258,18 +246,15 @@ func TestClientRunTransaction(t *testing.T) {
value := []byte("value")
key := []byte(fmt.Sprintf("%s/key-%t", testUser, commit))

// Use snapshot isolation so non-transactional read can always push.
err := db.Txn(context.TODO(), func(ctx context.Context, txn *client.Txn) error {
if err := txn.SetIsolation(enginepb.SNAPSHOT); err != nil {
return err
}

// Put transactional value.
if err := txn.Put(ctx, key, value); err != nil {
return err
}
// Attempt to read outside of txn.
if gr, err := db.Get(ctx, key); err != nil {
// Attempt to read in another txn.
conflictTxn := client.NewTxn(ctx, db, 0 /* gatewayNodeID */, client.RootTxn)
conflictTxn.InternalSetPriority(roachpb.MaxTxnPriority)
if gr, err := conflictTxn.Get(ctx, key); err != nil {
return err
} else if gr.Value != nil {
return errors.Errorf("expected nil value; got %+v", gr.Value)
Expand Down Expand Up @@ -322,12 +307,7 @@ func TestClientRunConcurrentTransaction(t *testing.T) {
keys[j] = []byte(fmt.Sprintf("%s/key-%t/%s", testUser, commit, string(s)))
}

// Use snapshot isolation so non-transactional read can always push.
err := db.Txn(context.TODO(), func(ctx context.Context, txn *client.Txn) error {
if err := txn.SetIsolation(enginepb.SNAPSHOT); err != nil {
return err
}

// We can't use errgroup here because we need to return any TxnAborted
// errors if we see them.
var wg sync.WaitGroup
Expand All @@ -341,9 +321,11 @@ func TestClientRunConcurrentTransaction(t *testing.T) {
concErrs[i] = err
return
}
// Attempt to read outside of txn. We need to guarantee that the
// Attempt to read in another txn. We need to guarantee that the
// BeginTxnRequest has finished or we risk aborting the transaction.
if gr, err := db.Get(ctx, key); err != nil {
conflictTxn := client.NewTxn(ctx, db, 0 /* gatewayNodeID */, client.RootTxn)
conflictTxn.InternalSetPriority(roachpb.MaxTxnPriority)
if gr, err := conflictTxn.Get(ctx, key); err != nil {
concErrs[i] = err
return
} else if gr.Value != nil {
Expand Down
10 changes: 0 additions & 10 deletions pkg/internal/client/sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"

"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
)

Expand Down Expand Up @@ -136,9 +135,6 @@ type TxnSender interface {
// SetDebugName sets the txn's debug name.
SetDebugName(name string)

// SetIsolation sets the transaction's isolation level.
SetIsolation(isolation enginepb.IsolationType) error

// TxnStatus exports the txn's status.
TxnStatus() roachpb.TransactionStatus

Expand Down Expand Up @@ -314,12 +310,6 @@ func (m *MockTransactionalSender) SetDebugName(name string) {
m.txn.Name = name
}

// SetIsolation is part of the TxnSender interface.
func (m *MockTransactionalSender) SetIsolation(isolation enginepb.IsolationType) error {
m.txn.Isolation = isolation
return nil
}

// OrigTimestamp is part of the TxnSender interface.
func (m *MockTransactionalSender) OrigTimestamp() hlc.Timestamp {
return m.txn.OrigTimestamp
Expand Down
19 changes: 2 additions & 17 deletions pkg/internal/client/txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ type Txn struct {
syncutil.Mutex
ID uuid.UUID
debugName string
isolation enginepb.IsolationType

// userPriority is the transaction's priority. If not set,
// NormalUserPriority will be used.
Expand Down Expand Up @@ -242,25 +241,11 @@ func (txn *Txn) debugNameLocked() string {
return fmt.Sprintf("%s (id: %s)", txn.mu.debugName, txn.mu.ID)
}

// SetIsolation sets the transaction's isolation type. Transactions default to
// serializable isolation. The isolation must be set before any operations are
// performed on the transaction.
func (txn *Txn) SetIsolation(isolation enginepb.IsolationType) error {
txn.mu.Lock()
defer txn.mu.Unlock()

if err := txn.mu.sender.SetIsolation(isolation); err != nil {
return err
}
txn.mu.isolation = isolation
return nil
}

// Isolation returns the transaction's isolation type.
func (txn *Txn) Isolation() enginepb.IsolationType {
txn.mu.Lock()
defer txn.mu.Unlock()
return txn.mu.isolation
return enginepb.SERIALIZABLE
}

// OrigTimestamp returns the transaction's starting timestamp.
Expand Down Expand Up @@ -973,7 +958,7 @@ func (txn *Txn) GenerateForcedRetryableError(ctx context.Context, msg string) er
txn.debugNameLocked(),
nil, // baseKey
txn.mu.userPriority,
txn.mu.isolation,
enginepb.SERIALIZABLE,
now,
txn.db.clock.MaxOffset().Nanoseconds(),
))
Expand Down
25 changes: 0 additions & 25 deletions pkg/kv/txn_coord_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,6 @@ type TxnMetrics struct {

// Counts of restart types.
RestartsWriteTooOld *metric.Counter
RestartsDeleteRange *metric.Counter
RestartsSerializable *metric.Counter
RestartsPossibleReplay *metric.Counter
RestartsAsyncWriteFailure *metric.Counter
Expand Down Expand Up @@ -299,12 +298,6 @@ var (
Measurement: "Restarted Transactions",
Unit: metric.Unit_COUNT,
}
metaRestartsDeleteRange = metric.Metadata{
Name: "txn.restarts.deleterange",
Help: "Number of restarts due to a forwarded commit timestamp and a DeleteRange command",
Measurement: "Restarted Transactions",
Unit: metric.Unit_COUNT,
}
metaRestartsSerializable = metric.Metadata{
Name: "txn.restarts.serializable",
Help: "Number of restarts due to a forwarded commit timestamp and isolation=SERIALIZABLE",
Expand Down Expand Up @@ -336,7 +329,6 @@ func MakeTxnMetrics(histogramWindow time.Duration) TxnMetrics {
Durations: metric.NewLatency(metaDurationsHistograms, histogramWindow),
Restarts: metric.NewHistogram(metaRestartsHistogram, histogramWindow, 100, 3),
RestartsWriteTooOld: metric.NewCounter(metaRestartsWriteTooOld),
RestartsDeleteRange: metric.NewCounter(metaRestartsDeleteRange),
RestartsSerializable: metric.NewCounter(metaRestartsSerializable),
RestartsPossibleReplay: metric.NewCounter(metaRestartsPossibleReplay),
RestartsAsyncWriteFailure: metric.NewCounter(metaRestartsAsyncWriteFailure),
Expand Down Expand Up @@ -825,8 +817,6 @@ func (tc *TxnCoordSender) handleRetryableErrLocked(
switch tErr.Reason {
case roachpb.RETRY_WRITE_TOO_OLD:
tc.metrics.RestartsWriteTooOld.Inc(1)
case roachpb.RETRY_DELETE_RANGE:
tc.metrics.RestartsDeleteRange.Inc(1)
case roachpb.RETRY_SERIALIZABLE:
tc.metrics.RestartsSerializable.Inc(1)
case roachpb.RETRY_POSSIBLE_REPLAY:
Expand Down Expand Up @@ -986,21 +976,6 @@ func (tc *TxnCoordSender) SetDebugName(name string) {
tc.mu.txn.Name = name
}

// SetIsolation is part of the client.TxnSender interface.
func (tc *TxnCoordSender) SetIsolation(isolation enginepb.IsolationType) error {
tc.mu.Lock()
defer tc.mu.Unlock()

if tc.mu.txn.Isolation == isolation {
return nil
}
if tc.mu.active {
return errors.Errorf("cannot change the isolation level of a running transaction")
}
tc.mu.txn.Isolation = isolation
return nil
}

// OrigTimestamp is part of the client.TxnSender interface.
func (tc *TxnCoordSender) OrigTimestamp() hlc.Timestamp {
tc.mu.Lock()
Expand Down
Loading

0 comments on commit d94f47c

Please sign in to comment.