nop Rollback for committed transactions

Author: asmyasnikov

CHANGELOG.md

@@ -15,7 +15,8 @@
 * Added helpers `MakePath` and `CleanPath` to root of package `ydb-go-sdk`
 * Removed call `types.Scanner.UnmarshalYDB()` inside `scanner.setDefaults()` 
 * Added `DoTx()` API method into `table.Client`
-* Added `String()` method into `ConnectParams` for serialize params to connection string 
+* Added `String()` method into `ConnectParams` for serialize params to connection string
+* Added early exit from Rollback for committed transaction
 
 ## 3.4.4
 * Prefer `ydb.table.types.Scanner` scanner implementation over `sql.Scanner`, when both available.

internal/errors/defaults.go

@@ -10,4 +10,7 @@ var (
 
 	// ErrNilConnection is returned when use nil preferred connection
 	ErrNilConnection = errors.New("nil connection")
+
+	// ErrAlreadyCommited returns if transaction Commit called twice
+	ErrAlreadyCommited = errors.New("already committed")
 )

internal/table/session.go

@@ -841,6 +841,8 @@ type Transaction struct {
 	id string
 	s  *session
 	c  *table.TransactionControl
+
+	committed bool
 }
 
 func (tx *Transaction) ID() string {
@@ -873,6 +875,14 @@ func (tx *Transaction) ExecuteStatement(
 
 // CommitTx commits specified active transaction.
 func (tx *Transaction) CommitTx(ctx context.Context, opts ...options.CommitTransactionOption) (r result.Result, err error) {
+	if tx.committed {
+		return nil, errors.ErrAlreadyCommited
+	}
+	defer func() {
+		if err == nil {
+			tx.committed = true
+		}
+	}()
 	onDone := trace.TableOnSessionTransactionCommit(tx.s.trace, &ctx, tx.s, tx)
 	defer func() {
 		onDone(err)
@@ -904,6 +914,9 @@ func (tx *Transaction) CommitTx(ctx context.Context, opts ...options.CommitTrans
 
 // Rollback performs a rollback of the specified active transaction.
 func (tx *Transaction) Rollback(ctx context.Context) (err error) {
+	if tx.committed {
+		return nil
+	}
 	onDone := trace.TableOnSessionTransactionRollback(tx.s.trace, &ctx, tx.s, tx)
 	defer func() {
 		onDone(err)

internal/table/session_test.go

@@ -8,9 +8,11 @@ import (
 	"time"
 
 	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/anypb"
 
 	"github.com/ydb-platform/ydb-go-genproto/Ydb_Table_V1"
 	"github.com/ydb-platform/ydb-go-genproto/protos/Ydb"
+	"github.com/ydb-platform/ydb-go-genproto/protos/Ydb_Operations"
 	"github.com/ydb-platform/ydb-go-genproto/protos/Ydb_Scheme"
 	"github.com/ydb-platform/ydb-go-genproto/protos/Ydb_Table"
 
@@ -471,3 +473,140 @@ func TestQueryCachePolicyKeepInCache(t *testing.T) {
 		})
 	}
 }
+
+func TestTxSkipRollbackForCommitted(t *testing.T) {
+	var (
+		begin    = 0
+		commit   = 0
+		rollback = 0
+	)
+	b := StubBuilder{
+		T: t,
+		Cluster: testutil.NewCluster(
+			testutil.WithInvokeHandlers(
+				testutil.InvokeHandlers{
+					// nolint:unparam
+					testutil.TableBeginTransaction: func(request interface{}) (proto.Message, error) {
+						_, ok := request.(*Ydb_Table.BeginTransactionRequest)
+						if !ok {
+							t.Fatalf("cannot cast request '%T' to *Ydb_Table.BeginTransactionRequest", request)
+						}
+						result, err := anypb.New(
+							&Ydb_Table.BeginTransactionResult{
+								TxMeta: &Ydb_Table.TransactionMeta{
+									Id: "",
+								},
+							},
+						)
+						if err != nil {
+							return nil, err
+						}
+						begin++
+						return &Ydb_Table.BeginTransactionResponse{
+							Operation: &Ydb_Operations.Operation{
+								Ready:  true,
+								Status: Ydb.StatusIds_SUCCESS,
+								Result: result,
+							},
+						}, nil
+					},
+					// nolint:unparam
+					testutil.TableCommitTransaction: func(request interface{}) (proto.Message, error) {
+						_, ok := request.(*Ydb_Table.CommitTransactionRequest)
+						if !ok {
+							t.Fatalf("cannot cast request '%T' to *Ydb_Table.CommitTransactionRequest", request)
+						}
+						result, err := anypb.New(
+							&Ydb_Table.CommitTransactionResult{},
+						)
+						if err != nil {
+							return nil, err
+						}
+						commit++
+						return &Ydb_Table.CommitTransactionResponse{
+							Operation: &Ydb_Operations.Operation{
+								Ready:  true,
+								Status: Ydb.StatusIds_SUCCESS,
+								Result: result,
+							},
+						}, nil
+					},
+					// nolint:unparam
+					testutil.TableRollbackTransaction: func(request interface{}) (proto.Message, error) {
+						_, ok := request.(*Ydb_Table.RollbackTransactionRequest)
+						if !ok {
+							t.Fatalf("cannot cast request '%T' to *Ydb_Table.RollbackTransactionRequest", request)
+						}
+						rollback++
+						return &Ydb_Table.RollbackTransactionResponse{
+							Operation: &Ydb_Operations.Operation{
+								Ready:  true,
+								Status: Ydb.StatusIds_SUCCESS,
+							},
+						}, nil
+					},
+					// nolint:unparam
+					testutil.TableCreateSession: func(request interface{}) (result proto.Message, err error) {
+						return &Ydb_Table.CreateSessionResult{
+							SessionId: testutil.SessionID(),
+						}, nil
+					},
+				},
+			),
+		),
+	}
+	s, err := b.createSession(context.Background())
+	if err != nil {
+		t.Fatal(err)
+	}
+	{
+		x, err := s.BeginTransaction(context.Background(), table.TxSettings())
+		if err != nil {
+			t.Fatal(err)
+		}
+		if begin != 1 {
+			t.Fatalf("unexpected begin: %d", begin)
+		}
+		_, err = x.CommitTx(context.Background())
+		if err != nil {
+			t.Fatal(err)
+		}
+		if commit != 1 {
+			t.Fatalf("unexpected commit: %d", begin)
+		}
+		_, _ = x.CommitTx(context.Background())
+		if commit != 1 {
+			t.Fatalf("unexpected commit: %d", begin)
+		}
+		err = x.Rollback(context.Background())
+		if err != nil {
+			t.Fatal(err)
+		}
+		if rollback != 0 {
+			t.Fatalf("unexpected rollback: %d", begin)
+		}
+	}
+	{
+		x, err := s.BeginTransaction(context.Background(), table.TxSettings())
+		if err != nil {
+			t.Fatal(err)
+		}
+		if begin != 2 {
+			t.Fatalf("unexpected begin: %d", begin)
+		}
+		err = x.Rollback(context.Background())
+		if err != nil {
+			t.Fatal(err)
+		}
+		if rollback != 1 {
+			t.Fatalf("unexpected rollback: %d", begin)
+		}
+		_, err = x.CommitTx(context.Background())
+		if err != nil {
+			t.Fatal(err)
+		}
+		if commit != 2 {
+			t.Fatalf("unexpected commit: %d", begin)
+		}
+	}
+}