Skip to content

Commit db24653

Browse files
asmyasnikovasmyasnikov
authored
Merge pull request #850 from ydb-platform/retry-id
refactoring of retry options for `retry.Retry`, `retry.Do`, `retry.DoTx`, `table.Do`, `table.DoTx`
2 parents a9598a0 + 3905634 commit db24653

30 files changed

+493
-282
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
* Marked as deprecated `retry.WithDoRetryOptions` and `retry.WithDoTxRetryOptions`
12
* Added receiving first result set on construct `internal/table/scanner.NewStream()`
23
* Added experimental package `metrics` with SDK metrics
34
* Fixed redundant trace call for finished `database/sql` transactions

‎SQL.md‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ err := retry.Do(context.TODO(), db, func(ctx context.Context, cc *sql.Conn) erro
270270
}
271271
...
272272
return nil // good final of retry operation
273-
}, retry.WithDoRetryOptions(retry.WithIdempotent(true)))
273+
}, retry.WithIdempotent(true))
274274

275275
```
276276

@@ -296,9 +296,7 @@ err := retry.DoTx(context.TODO(), db, func(ctx context.Context, tx *sql.Tx) erro
296296
}
297297
...
298298
return nil // good final of retry tx operation
299-
}, retry.WithDoTxRetryOptions(
300-
retry.WithIdempotent(true),
301-
), retry.WithTxOptions(&sql.TxOptions{
299+
}, retry.WithIdempotent(true), retry.WithTxOptions(&sql.TxOptions{
302300
Isolation: sql.LevelSnapshot,
303301
ReadOnly: true,
304302
}))
@@ -568,7 +566,7 @@ err := retry.Do(context.TODO(), db, func(ctx context.Context, cc *sql.Conn) erro
568566
}
569567
...
570568
return nil // good final of retry operation
571-
}, retry.WithDoRetryOptions(retry.WithIdempotent(true)))
569+
}, retry.WithIdempotent(true))
572570
```
573571
574572
## Troubleshooting <a name="troubleshooting"></a>

‎example_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func Example_databaseSQL() {
8686
}
8787
log.Printf("id=%v, myStr='%s'\n", id, myStr)
8888
return nil
89-
}, retry.WithDoTxRetryOptions(retry.WithIdempotent(true)))
89+
}, retry.WithIdempotent(true))
9090
if err != nil {
9191
log.Printf("query failed: %v", err)
9292
}

‎examples/basic/database_sql/series.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"path"
1010
"time"
1111

12-
"github.com/ydb-platform/ydb-go-sdk/v3"
12+
ydb "github.com/ydb-platform/ydb-go-sdk/v3"
1313
"github.com/ydb-platform/ydb-go-sdk/v3/retry"
1414
"github.com/ydb-platform/ydb-go-sdk/v3/sugar"
1515
"github.com/ydb-platform/ydb-go-sdk/v3/table"
@@ -31,7 +31,7 @@ func selectDefault(ctx context.Context, db *sql.DB) (err error) {
3131
}
3232
log.Printf("AST = %s\n\nPlan = %s", ast, plan)
3333
return nil
34-
}, retry.WithDoRetryOptions(retry.WithIdempotent(true)))
34+
}, retry.WithIdempotent(true))
3535
if err != nil {
3636
return fmt.Errorf("explain query failed: %w", err)
3737
}
@@ -62,7 +62,7 @@ func selectDefault(ctx context.Context, db *sql.DB) (err error) {
6262
)
6363
}
6464
return rows.Err()
65-
}, retry.WithDoRetryOptions(retry.WithIdempotent(true)))
65+
}, retry.WithIdempotent(true))
6666
if err != nil {
6767
return fmt.Errorf("execute data query failed: %w", err)
6868
}
@@ -155,7 +155,7 @@ func selectScan(ctx context.Context, db *sql.DB) (err error) {
155155
)
156156
}
157157
return rows.Err()
158-
}, retry.WithDoRetryOptions(retry.WithIdempotent(true)))
158+
}, retry.WithIdempotent(true))
159159
if err != nil {
160160
return fmt.Errorf("scan query failed: %w", err)
161161
}
@@ -202,7 +202,7 @@ func fillTablesWithData(ctx context.Context, db *sql.DB) (err error) {
202202
return err
203203
}
204204
return nil
205-
}, retry.WithDoTxRetryOptions(retry.WithIdempotent(true)))
205+
}, retry.WithIdempotent(true))
206206
if err != nil {
207207
return fmt.Errorf("upsert query failed: %w", err)
208208
}
@@ -235,7 +235,7 @@ func prepareSchema(ctx context.Context, db *sql.DB) (err error) {
235235
return err
236236
}
237237
return nil
238-
}, retry.WithDoRetryOptions(retry.WithIdempotent(true)))
238+
}, retry.WithIdempotent(true))
239239
if err != nil {
240240
return fmt.Errorf("create table failed: %w", err)
241241
}
@@ -266,7 +266,7 @@ func prepareSchema(ctx context.Context, db *sql.DB) (err error) {
266266
return err
267267
}
268268
return nil
269-
}, retry.WithDoRetryOptions(retry.WithIdempotent(true)))
269+
}, retry.WithIdempotent(true))
270270
if err != nil {
271271
return fmt.Errorf("create table failed: %w", err)
272272
}
@@ -299,7 +299,7 @@ func prepareSchema(ctx context.Context, db *sql.DB) (err error) {
299299
return err
300300
}
301301
return nil
302-
}, retry.WithDoRetryOptions(retry.WithIdempotent(true)))
302+
}, retry.WithIdempotent(true))
303303
if err != nil {
304304
return fmt.Errorf("create table failed: %w", err)
305305
}

‎internal/table/client.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -241,31 +241,31 @@ func (c *Client) CreateSession(ctx context.Context, opts ...table.Option) (_ tab
241241
}
242242
return s, nil
243243
}
244-
options := retryOptions(c.config.Trace(), opts...)
245-
err = retry.Retry(
246-
ctx,
244+
err = retry.Retry(ctx,
247245
func(ctx context.Context) (err error) {
248246
s, err = createSession(ctx)
249247
if err != nil {
250248
return xerrors.WithStackTrace(err)
251249
}
252250
return nil
253251
},
254-
retry.WithIdempotent(true),
255-
retry.WithID("CreateSession"),
256-
retry.WithFastBackoff(options.FastBackoff),
257-
retry.WithSlowBackoff(options.SlowBackoff),
258-
retry.WithTrace(trace.Retry{
259-
OnRetry: func(info trace.RetryLoopStartInfo) func(trace.RetryLoopIntermediateInfo) func(trace.RetryLoopDoneInfo) {
260-
onIntermediate := trace.TableOnCreateSession(c.config.Trace(), info.Context)
261-
return func(info trace.RetryLoopIntermediateInfo) func(trace.RetryLoopDoneInfo) {
262-
onDone := onIntermediate(info.Error)
263-
return func(info trace.RetryLoopDoneInfo) {
264-
onDone(s, info.Attempts, info.Error)
265-
}
266-
}
267-
},
268-
}),
252+
append(
253+
[]retry.Option{
254+
retry.WithIdempotent(true),
255+
retry.WithID("CreateSession"),
256+
retry.WithTrace(trace.Retry{
257+
OnRetry: func(info trace.RetryLoopStartInfo) func(trace.RetryLoopIntermediateInfo) func(trace.RetryLoopDoneInfo) {
258+
onIntermediate := trace.TableOnCreateSession(c.config.Trace(), info.Context)
259+
return func(info trace.RetryLoopIntermediateInfo) func(trace.RetryLoopDoneInfo) {
260+
onDone := onIntermediate(info.Error)
261+
return func(info trace.RetryLoopDoneInfo) {
262+
onDone(s, info.Attempts, info.Error)
263+
}
264+
}
265+
},
266+
}),
267+
}, retryOptions(c.config.Trace(), opts...).RetryOptions...,
268+
)...,
269269
)
270270
if err != nil {
271271
return nil, xerrors.WithStackTrace(err)

‎internal/table/retry.go‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package table
33
import (
44
"context"
55

6-
"github.com/ydb-platform/ydb-go-sdk/v3/internal/backoff"
76
"github.com/ydb-platform/ydb-go-sdk/v3/internal/table/config"
87
"github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors"
98
"github.com/ydb-platform/ydb-go-sdk/v3/retry"
@@ -50,18 +49,14 @@ func doTx(
5049
attempts, onIntermediate := 0, trace.TableOnDoTx(
5150
opts.Trace,
5251
&ctx,
52+
opts.ID,
5353
opts.Idempotent,
5454
isRetryCalledAbove(ctx),
5555
)
5656
defer func() {
5757
onIntermediate(err)(attempts, err)
5858
}()
59-
err = retryBackoff(
60-
ctx,
61-
c,
62-
opts.FastBackoff,
63-
opts.SlowBackoff,
64-
opts.Idempotent,
59+
err = retryBackoff(ctx, c,
6560
func(ctx context.Context, s table.Session) (err error) {
6661
attempts++
6762

@@ -110,6 +105,7 @@ func doTx(
110105

111106
return nil
112107
},
108+
opts.RetryOptions...,
113109
)
114110
if err != nil {
115111
return xerrors.WithStackTrace(err)
@@ -127,11 +123,11 @@ func do(
127123
if opts.Trace == nil {
128124
opts.Trace = &trace.Table{}
129125
}
130-
attempts, onIntermediate := 0, trace.TableOnDo(opts.Trace, &ctx, opts.Idempotent, isRetryCalledAbove(ctx))
126+
attempts, onIntermediate := 0, trace.TableOnDo(opts.Trace, &ctx, opts.ID, opts.Idempotent, isRetryCalledAbove(ctx))
131127
defer func() {
132128
onIntermediate(err)(attempts, err)
133129
}()
134-
return retryBackoff(ctx, c, opts.FastBackoff, opts.SlowBackoff, opts.Idempotent,
130+
return retryBackoff(ctx, c,
135131
func(ctx context.Context, s table.Session) (err error) {
136132
attempts++
137133

@@ -156,16 +152,15 @@ func do(
156152

157153
return nil
158154
},
155+
opts.RetryOptions...,
159156
)
160157
}
161158

162159
func retryBackoff(
163160
ctx context.Context,
164161
p SessionProvider,
165-
fastBackoff backoff.Backoff,
166-
slowBackoff backoff.Backoff,
167-
isOperationIdempotent bool,
168162
op table.Operation,
163+
opts ...retry.Option,
169164
) (err error) {
170165
err = retry.Retry(markRetryCall(ctx),
171166
func(ctx context.Context) (err error) {
@@ -188,9 +183,7 @@ func retryBackoff(
188183

189184
return nil
190185
},
191-
retry.WithFastBackoff(fastBackoff),
192-
retry.WithSlowBackoff(slowBackoff),
193-
retry.WithIdempotent(isOperationIdempotent),
186+
opts...,
194187
)
195188
if err != nil {
196189
return xerrors.WithStackTrace(err)
@@ -200,16 +193,14 @@ func retryBackoff(
200193

201194
func retryOptions(trace *trace.Table, opts ...table.Option) *table.Options {
202195
options := &table.Options{
203-
Trace: trace,
204-
FastBackoff: backoff.Fast,
205-
SlowBackoff: backoff.Slow,
196+
Trace: trace,
206197
TxSettings: table.TxSettings(
207198
table.WithSerializableReadWrite(),
208199
),
209200
}
210-
for _, o := range opts {
211-
if o != nil {
212-
o(options)
201+
for _, opt := range opts {
202+
if opt != nil {
203+
opt.ApplyTableOption(options)
213204
}
214205
}
215206
return options

‎internal/table/retry_test.go‎

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,22 @@ func TestRetryerBackoffRetryCancelation(t *testing.T) {
4949
return testErr
5050
},
5151
&table.Options{
52-
FastBackoff: testutil.BackoffFunc(func(n int) <-chan time.Time {
53-
ch := make(chan time.Time)
54-
backoff <- ch
55-
return ch
56-
}),
57-
SlowBackoff: testutil.BackoffFunc(func(n int) <-chan time.Time {
58-
ch := make(chan time.Time)
59-
backoff <- ch
60-
return ch
61-
}),
52+
RetryOptions: []retry.Option{
53+
retry.WithFastBackoff(
54+
testutil.BackoffFunc(func(n int) <-chan time.Time {
55+
ch := make(chan time.Time)
56+
backoff <- ch
57+
return ch
58+
}),
59+
),
60+
retry.WithSlowBackoff(
61+
testutil.BackoffFunc(func(n int) <-chan time.Time {
62+
ch := make(chan time.Time)
63+
backoff <- ch
64+
return ch
65+
}),
66+
),
67+
},
6268
},
6369
)
6470
results <- err
@@ -207,12 +213,18 @@ func TestRetryerImmediateReturn(t *testing.T) {
207213
return testErr
208214
},
209215
&table.Options{
210-
FastBackoff: testutil.BackoffFunc(func(n int) <-chan time.Time {
211-
panic("this code will not be called")
212-
}),
213-
SlowBackoff: testutil.BackoffFunc(func(n int) <-chan time.Time {
214-
panic("this code will not be called")
215-
}),
216+
RetryOptions: []retry.Option{
217+
retry.WithFastBackoff(
218+
testutil.BackoffFunc(func(n int) <-chan time.Time {
219+
panic("this code will not be called")
220+
}),
221+
),
222+
retry.WithSlowBackoff(
223+
testutil.BackoffFunc(func(n int) <-chan time.Time {
224+
panic("this code will not be called")
225+
}),
226+
),
227+
},
216228
},
217229
)
218230
if !xerrors.Is(err, testErr) {

0 commit comments

Comments
 (0)