Skip to content

Commit aac8216

Browse files
committed
fixes
1 parent c6a3dbe commit aac8216

File tree

4 files changed

+47
-71
lines changed

4 files changed

+47
-71
lines changed

internal/table/client.go

+36-5
Original file line numberDiff line numberDiff line change
@@ -666,9 +666,30 @@ func (c *Client) Do(ctx context.Context, op table.Operation, opts ...table.Optio
666666
onDone(attempts, finalErr)
667667
}()
668668

669-
err := do(ctx, c, c.config, op, func(err error) {
670-
attempts++
671-
}, config.RetryOptions...)
669+
err := retryBackoff(ctx, c,
670+
func(ctx context.Context, s table.Session) (err error) {
671+
attempts++
672+
673+
err = func() error {
674+
if panicCallback := c.config.PanicCallback(); panicCallback != nil {
675+
defer func() {
676+
if e := recover(); e != nil {
677+
panicCallback(e)
678+
}
679+
}()
680+
}
681+
682+
return op(xcontext.MarkRetryCall(ctx), s)
683+
}()
684+
685+
if err != nil {
686+
return xerrors.WithStackTrace(err)
687+
}
688+
689+
return nil
690+
},
691+
config.RetryOptions...,
692+
)
672693
if err != nil {
673694
return xerrors.WithStackTrace(err)
674695
}
@@ -695,14 +716,19 @@ func (c *Client) DoTx(ctx context.Context, op table.TxOperation, opts ...table.O
695716
onDone(attempts, finalErr)
696717
}()
697718

698-
return retryBackoff(ctx, c,
699-
func(ctx context.Context, s table.Session) (err error) {
719+
err := retryBackoff(ctx, c,
720+
func(ctx context.Context, s table.Session) (finalErr error) {
700721
attempts++
701722

702723
tx, err := s.BeginTransaction(ctx, config.TxSettings)
703724
if err != nil {
704725
return xerrors.WithStackTrace(err)
705726
}
727+
defer func() {
728+
if finalErr != nil {
729+
_ = tx.Rollback(ctx)
730+
}
731+
}()
706732

707733
err = func() error {
708734
if panicCallback := c.config.PanicCallback(); panicCallback != nil {
@@ -729,6 +755,11 @@ func (c *Client) DoTx(ctx context.Context, op table.TxOperation, opts ...table.O
729755
},
730756
config.RetryOptions...,
731757
)
758+
if err != nil {
759+
return xerrors.WithStackTrace(err)
760+
}
761+
762+
return nil
732763
}
733764

734765
func (c *Client) internalPoolGCTick(ctx context.Context, idleThreshold time.Duration) {

internal/table/retry.go

-40
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ package table
33
import (
44
"context"
55

6-
"github.com/ydb-platform/ydb-go-sdk/v3/internal/table/config"
7-
"github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext"
86
"github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors"
97
"github.com/ydb-platform/ydb-go-sdk/v3/retry"
108
"github.com/ydb-platform/ydb-go-sdk/v3/table"
@@ -22,44 +20,6 @@ type SessionProvider interface {
2220
Put(ctx context.Context, s *session) (err error)
2321
}
2422

25-
func do(
26-
ctx context.Context,
27-
c SessionProvider,
28-
config *config.Config,
29-
op table.Operation,
30-
onAttempt func(err error),
31-
opts ...retry.Option,
32-
) (err error) {
33-
return retryBackoff(ctx, c,
34-
func(ctx context.Context, s table.Session) (err error) {
35-
defer func() {
36-
if onAttempt != nil {
37-
onAttempt(err)
38-
}
39-
}()
40-
41-
err = func() error {
42-
if panicCallback := config.PanicCallback(); panicCallback != nil {
43-
defer func() {
44-
if e := recover(); e != nil {
45-
panicCallback(e)
46-
}
47-
}()
48-
}
49-
50-
return op(xcontext.MarkRetryCall(ctx), s)
51-
}()
52-
53-
if err != nil {
54-
return xerrors.WithStackTrace(err)
55-
}
56-
57-
return nil
58-
},
59-
opts...,
60-
)
61-
}
62-
6323
func retryBackoff(
6424
ctx context.Context,
6525
p SessionProvider,

internal/table/retry_test.go

+6-26
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
grpcCodes "google.golang.org/grpc/codes"
1212
grpcStatus "google.golang.org/grpc/status"
1313

14-
"github.com/ydb-platform/ydb-go-sdk/v3/internal/table/config"
1514
"github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext"
1615
"github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors"
1716
"github.com/ydb-platform/ydb-go-sdk/v3/internal/xrand"
@@ -41,12 +40,10 @@ func TestRetryerBackoffRetryCancelation(t *testing.T) {
4140
ctx, cancel := xcontext.WithCancel(context.Background())
4241
results := make(chan error)
4342
go func() {
44-
err := do(ctx, p,
45-
config.New(),
43+
err := retryBackoff(ctx, p,
4644
func(ctx context.Context, _ table.Session) error {
4745
return testErr
4846
},
49-
nil,
5047
retry.WithFastBackoff(
5148
testutil.BackoffFunc(func(n int) <-chan time.Time {
5249
ch := make(chan time.Time)
@@ -103,7 +100,7 @@ func TestRetryerBadSession(t *testing.T) {
103100
sessions []table.Session
104101
)
105102
ctx, cancel := xcontext.WithCancel(context.Background())
106-
err := do(ctx, p, config.New(),
103+
err := retryBackoff(ctx, p,
107104
func(ctx context.Context, s table.Session) error {
108105
sessions = append(sessions, s)
109106
i++
@@ -115,7 +112,6 @@ func TestRetryerBadSession(t *testing.T) {
115112
xerrors.WithStatusCode(Ydb.StatusIds_BAD_SESSION),
116113
)
117114
},
118-
func(err error) {},
119115
)
120116
if !xerrors.Is(err, context.Canceled) {
121117
t.Errorf("unexpected error: %v", err)
@@ -154,17 +150,13 @@ func TestRetryerSessionClosing(t *testing.T) {
154150
}
155151
var sessions []table.Session
156152
for i := 0; i < 1000; i++ {
157-
err := do(
158-
context.Background(),
159-
p,
160-
config.New(),
153+
err := retryBackoff(context.Background(), p,
161154
func(ctx context.Context, s table.Session) error {
162155
sessions = append(sessions, s)
163156
s.(*session).SetStatus(table.SessionClosing)
164157

165158
return nil
166159
},
167-
nil,
168160
)
169161
if err != nil {
170162
t.Errorf("unexpected error: %v", err)
@@ -208,14 +200,10 @@ func TestRetryerImmediateReturn(t *testing.T) {
208200
p := SingleSession(
209201
simpleSession(t),
210202
)
211-
err := do(
212-
context.Background(),
213-
p,
214-
config.New(),
203+
err := retryBackoff(context.Background(), p,
215204
func(ctx context.Context, _ table.Session) error {
216205
return testErr
217206
},
218-
nil,
219207
retry.WithFastBackoff(
220208
testutil.BackoffFunc(func(n int) <-chan time.Time {
221209
panic("this code will not be called")
@@ -341,10 +329,7 @@ func TestRetryContextDeadline(t *testing.T) {
341329
t.Run(fmt.Sprintf("Timeout=%v,Sleep=%v", timeout, sleep), func(t *testing.T) {
342330
ctx, cancel := xcontext.WithTimeout(context.Background(), timeout)
343331
defer cancel()
344-
_ = do(
345-
ctx,
346-
p,
347-
config.New(),
332+
_ = retryBackoff(ctx, p,
348333
func(ctx context.Context, _ table.Session) error {
349334
select {
350335
case <-ctx.Done():
@@ -353,7 +338,6 @@ func TestRetryContextDeadline(t *testing.T) {
353338
return errs[r.Int(len(errs))]
354339
}
355340
},
356-
nil,
357341
)
358342
})
359343
}
@@ -442,10 +426,7 @@ func TestRetryWithCustomErrors(t *testing.T) {
442426
i = 0
443427
sessions = make(map[table.Session]int)
444428
)
445-
err := do(
446-
ctx,
447-
p,
448-
config.New(),
429+
err := retryBackoff(ctx, p,
449430
func(ctx context.Context, s table.Session) (err error) {
450431
sessions[s]++
451432
i++
@@ -455,7 +436,6 @@ func TestRetryWithCustomErrors(t *testing.T) {
455436

456437
return nil
457438
},
458-
nil,
459439
)
460440
//nolint:nestif
461441
if test.retriable {

retry/sql.go

+5
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,11 @@ func DoTx(ctx context.Context, db *sql.DB, op func(context.Context, *sql.Tx) err
166166
if err != nil {
167167
return unwrapErrBadConn(xerrors.WithStackTrace(err))
168168
}
169+
defer func() {
170+
if finalErr != nil {
171+
_ = tx.Rollback()
172+
}
173+
}()
169174

170175
if err = op(xcontext.MarkRetryCall(ctx), tx); err != nil {
171176
return unwrapErrBadConn(xerrors.WithStackTrace(err))

0 commit comments

Comments
 (0)