From dd017ee2767f7de8d3588977dad1e74d11ce5f6e Mon Sep 17 00:00:00 2001 From: Viktor Pentyukhov Date: Wed, 13 Aug 2025 17:25:22 +0300 Subject: [PATCH 1/5] Refactored / and / option handling to unify Option usage and simplify implementation --- CHANGELOG.md | 2 + retry/retry.go | 72 ------------------------------ retry/sql.go | 119 +++++++++++++++---------------------------------- 3 files changed, 37 insertions(+), 156 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40ee8f394..22c176e3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +* Refactored `retry.Do`/`retry.DoWithResult` and `retry.DoTx`/`retry.DoTxWithResult` option handling to unify Option usage and simplify implementation + ## v3.113.6 * Removed experimental label from `retry.DoWithResult` and `retry.DoTxWithResult` diff --git a/retry/retry.go b/retry/retry.go index 48f15dd41..04816a5fd 100644 --- a/retry/retry.go +++ b/retry/retry.go @@ -39,14 +39,6 @@ var _ Option = labelOption("") type labelOption string -func (label labelOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, WithLabel(string(label))) -} - -func (label labelOption) ApplyDoTxOption(opts *doTxOptions) { - opts.retryOptions = append(opts.retryOptions, WithLabel(string(label))) -} - func (label labelOption) ApplyRetryOption(opts *retryOptions) { opts.label = string(label) } @@ -62,14 +54,6 @@ type callOption struct { call } -func (call callOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, withCaller(call)) -} - -func (call callOption) ApplyDoTxOption(opts *doTxOptions) { - opts.retryOptions = append(opts.retryOptions, withCaller(call)) -} - func (call callOption) ApplyRetryOption(opts *retryOptions) { opts.call = call } @@ -90,14 +74,6 @@ func (stackTraceOption) ApplyRetryOption(opts *retryOptions) { opts.stackTrace = true } -func (stackTraceOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, WithStackTrace()) -} - -func (stackTraceOption) ApplyDoTxOption(opts *doTxOptions) { - opts.retryOptions = append(opts.retryOptions, WithStackTrace()) -} - // WithStackTrace wraps errors with stacktrace from Retry call func WithStackTrace() stackTraceOption { return stackTraceOption{} @@ -113,14 +89,6 @@ func (t traceOption) ApplyRetryOption(opts *retryOptions) { opts.trace = opts.trace.Compose(t.t) } -func (t traceOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, WithTrace(t.t)) -} - -func (t traceOption) ApplyDoTxOption(opts *doTxOptions) { - opts.retryOptions = append(opts.retryOptions, WithTrace(t.t)) -} - // WithTrace returns trace option func WithTrace(t *trace.Retry) traceOption { return traceOption{t: t} @@ -136,14 +104,6 @@ func (b budgetOption) ApplyRetryOption(opts *retryOptions) { opts.budget = b.b } -func (b budgetOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, WithBudget(b.b)) -} - -func (b budgetOption) ApplyDoTxOption(opts *doTxOptions) { - opts.retryOptions = append(opts.retryOptions, WithBudget(b.b)) -} - // WithBudget returns budget option // // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental @@ -159,14 +119,6 @@ func (idempotent idempotentOption) ApplyRetryOption(opts *retryOptions) { opts.idempotent = bool(idempotent) } -func (idempotent idempotentOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, WithIdempotent(bool(idempotent))) -} - -func (idempotent idempotentOption) ApplyDoTxOption(opts *doTxOptions) { - opts.retryOptions = append(opts.retryOptions, WithIdempotent(bool(idempotent))) -} - // WithIdempotent applies idempotent flag to retry operation func WithIdempotent(idempotent bool) idempotentOption { return idempotentOption(idempotent) @@ -184,14 +136,6 @@ func (o fastBackoffOption) ApplyRetryOption(opts *retryOptions) { } } -func (o fastBackoffOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, WithFastBackoff(o.backoff)) -} - -func (o fastBackoffOption) ApplyDoTxOption(opts *doTxOptions) { - opts.retryOptions = append(opts.retryOptions, WithFastBackoff(o.backoff)) -} - // WithFastBackoff replaces default fast backoff func WithFastBackoff(b backoff.Backoff) fastBackoffOption { return fastBackoffOption{backoff: b} @@ -209,14 +153,6 @@ func (o slowBackoffOption) ApplyRetryOption(opts *retryOptions) { } } -func (o slowBackoffOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, WithSlowBackoff(o.backoff)) -} - -func (o slowBackoffOption) ApplyDoTxOption(opts *doTxOptions) { - opts.retryOptions = append(opts.retryOptions, WithSlowBackoff(o.backoff)) -} - // WithSlowBackoff replaces default slow backoff func WithSlowBackoff(b backoff.Backoff) slowBackoffOption { return slowBackoffOption{backoff: b} @@ -232,14 +168,6 @@ func (o panicCallbackOption) ApplyRetryOption(opts *retryOptions) { opts.panicCallback = o.callback } -func (o panicCallbackOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, WithPanicCallback(o.callback)) -} - -func (o panicCallbackOption) ApplyDoTxOption(opts *doTxOptions) { - opts.retryOptions = append(opts.retryOptions, WithPanicCallback(o.callback)) -} - // WithPanicCallback returns panic callback option // If not defined - panic would not intercept with driver func WithPanicCallback(panicCallback func(e interface{})) panicCallbackOption { diff --git a/retry/sql.go b/retry/sql.go index 652c9c100..271dfaaeb 100644 --- a/retry/sql.go +++ b/retry/sql.go @@ -14,36 +14,8 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) -type doOptions struct { - retryOptions []Option -} - -// doTxOption defines option for redefine default Retry behavior -type doOption interface { - ApplyDoOption(opts *doOptions) -} - -var ( - _ doOption = doRetryOptionsOption(nil) - _ doOption = labelOption("") -) - -type doRetryOptionsOption []Option - -func (retryOptions doRetryOptionsOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, retryOptions...) -} - -// WithDoRetryOptions specified retry options -// Deprecated: use explicit options instead. -// Will be removed after Oct 2024. -// Read about versioning policy: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#deprecated -func WithDoRetryOptions(opts ...Option) doRetryOptionsOption { - return opts -} - // Do is a retryer of database/sql conn with fallbacks on errors -func Do(ctx context.Context, db *sql.DB, op func(ctx context.Context, cc *sql.Conn) error, opts ...doOption) error { +func Do(ctx context.Context, db *sql.DB, op func(ctx context.Context, cc *sql.Conn) error, opts ...Option) error { _, err := DoWithResult(ctx, db, func(ctx context.Context, cc *sql.Conn) (*struct{}, error) { err := op(ctx, cc) if err != nil { @@ -62,29 +34,25 @@ func Do(ctx context.Context, db *sql.DB, op func(ctx context.Context, cc *sql.Co // DoWithResult is a retryer of database/sql conn with fallbacks on errors func DoWithResult[T any](ctx context.Context, db *sql.DB, op func(ctx context.Context, cc *sql.Conn) (T, error), - opts ...doOption, + opts ...Option, ) (T, error) { var ( zeroValue T - options = doOptions{ - retryOptions: []Option{ - withCaller(stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/v3/retry.DoWithResult")), - }, + options = []Option{ + withCaller(stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/v3/retry.DoWithResult")), } attempts = 0 ) if tracer, has := db.Driver().(interface { TraceRetry() *trace.Retry }); has { - options.retryOptions = append(options.retryOptions, nil) - copy(options.retryOptions[1:], options.retryOptions) - options.retryOptions[0] = WithTrace(tracer.TraceRetry()) - } - for _, opt := range opts { - if opt != nil { - opt.ApplyDoOption(&options) - } + options = append(options, nil) + copy(options[1:], options) + options[0] = WithTrace(tracer.TraceRetry()) } + + options = append(options, opts...) + v, err := RetryWithResult(ctx, func(ctx context.Context) (_ T, finalErr error) { attempts++ cc, err := db.Conn(ctx) @@ -106,7 +74,7 @@ func DoWithResult[T any](ctx context.Context, db *sql.DB, } return v, nil - }, options.retryOptions...) + }, options...) if err != nil { return zeroValue, xerrors.WithStackTrace( fmt.Errorf("operation failed with %d attempts: %w", attempts, err), @@ -116,30 +84,9 @@ func DoWithResult[T any](ctx context.Context, db *sql.DB, return v, nil } -type doTxOptions struct { - txOptions *sql.TxOptions - retryOptions []Option -} - // doTxOption defines option for redefine default Retry behavior type doTxOption interface { - ApplyDoTxOption(o *doTxOptions) -} - -var _ doTxOption = doTxRetryOptionsOption(nil) - -type doTxRetryOptionsOption []Option - -func (doTxRetryOptions doTxRetryOptionsOption) ApplyDoTxOption(o *doTxOptions) { - o.retryOptions = append(o.retryOptions, doTxRetryOptions...) -} - -// WithDoTxRetryOptions specified retry options -// Deprecated: use explicit options instead. -// Will be removed after Oct 2024. -// Read about versioning policy: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#deprecated -func WithDoTxRetryOptions(opts ...Option) doTxRetryOptionsOption { - return opts + ApplyDoTxOption(txOptions *sql.TxOptions) } var _ doTxOption = txOptionsOption{} @@ -148,8 +95,10 @@ type txOptionsOption struct { txOptions *sql.TxOptions } -func (txOptions txOptionsOption) ApplyDoTxOption(o *doTxOptions) { - o.txOptions = txOptions.txOptions +func (t txOptionsOption) ApplyDoTxOption(txOptions *sql.TxOptions) { + if t.txOptions != nil { + *txOptions = *t.txOptions + } } // WithTxOptions specified transaction options @@ -160,7 +109,7 @@ func WithTxOptions(txOptions *sql.TxOptions) txOptionsOption { } // DoTx is a retryer of database/sql transactions with fallbacks on errors -func DoTx(ctx context.Context, db *sql.DB, op func(context.Context, *sql.Tx) error, opts ...doTxOption) error { +func DoTx(ctx context.Context, db *sql.DB, op func(context.Context, *sql.Tx) error, opts ...Option) error { _, err := DoTxWithResult(ctx, db, func(ctx context.Context, tx *sql.Tx) (*struct{}, error) { err := op(ctx, tx) if err != nil { @@ -179,18 +128,16 @@ func DoTx(ctx context.Context, db *sql.DB, op func(context.Context, *sql.Tx) err // DoTxWithResult is a retryer of database/sql transactions with fallbacks on errors func DoTxWithResult[T any](ctx context.Context, db *sql.DB, op func(context.Context, *sql.Tx) (T, error), - opts ...doTxOption, + opts ...Option, ) (T, error) { var ( zeroValue T - options = doTxOptions{ - retryOptions: []Option{ - withCaller(stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/v3/retry.DoTxWithResult")), - }, - txOptions: &sql.TxOptions{ - Isolation: sql.LevelDefault, - ReadOnly: false, - }, + options = []Option{ + withCaller(stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/v3/retry.DoTxWithResult")), + } + txOptions = &sql.TxOptions{ + Isolation: sql.LevelDefault, + ReadOnly: false, } attempts = 0 ) @@ -198,19 +145,23 @@ func DoTxWithResult[T any](ctx context.Context, db *sql.DB, TraceRetry() *trace.Retry RetryBudget() budget.Budget }); has { - options.retryOptions = append(options.retryOptions, nil, nil) - copy(options.retryOptions[2:], options.retryOptions) - options.retryOptions[0] = WithTrace(d.TraceRetry()) - options.retryOptions[1] = WithBudget(d.RetryBudget()) + options = append(options, nil, nil) + copy(options[2:], options) + options[0] = WithTrace(d.TraceRetry()) + options[1] = WithBudget(d.RetryBudget()) } for _, opt := range opts { if opt != nil { - opt.ApplyDoTxOption(&options) + if txOpt, ok := opt.(doTxOption); ok { + txOpt.ApplyDoTxOption(txOptions) + } else { + options = append(options, opt) + } } } v, err := RetryWithResult(ctx, func(ctx context.Context) (_ T, finalErr error) { attempts++ - tx, err := db.BeginTx(ctx, options.txOptions) + tx, err := db.BeginTx(ctx, txOptions) if err != nil { return zeroValue, xerrors.WithStackTrace(err) } @@ -226,7 +177,7 @@ func DoTxWithResult[T any](ctx context.Context, db *sql.DB, } return v, nil - }, options.retryOptions...) + }, options...) if err != nil { return zeroValue, xerrors.WithStackTrace( fmt.Errorf("tx operation failed with %d attempts: %w", attempts, err), From 8daa2f3a9b2ab864a949fed5ece812967d3a5cad Mon Sep 17 00:00:00 2001 From: Viktor Pentyukhov Date: Wed, 13 Aug 2025 17:50:22 +0300 Subject: [PATCH 2/5] Fixed doTxOption interface to support WithTxOptions --- retry/sql.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/retry/sql.go b/retry/sql.go index 271dfaaeb..dff3d577a 100644 --- a/retry/sql.go +++ b/retry/sql.go @@ -89,12 +89,16 @@ type doTxOption interface { ApplyDoTxOption(txOptions *sql.TxOptions) } +var _ Option = txOptionsOption{} var _ doTxOption = txOptionsOption{} type txOptionsOption struct { txOptions *sql.TxOptions } +func (t txOptionsOption) ApplyRetryOption(_ *retryOptions) { +} + func (t txOptionsOption) ApplyDoTxOption(txOptions *sql.TxOptions) { if t.txOptions != nil { *txOptions = *t.txOptions From eb7fcf4ee562be4d1b23eac12b96a0fccf1b8e3b Mon Sep 17 00:00:00 2001 From: Viktor Pentyukhov Date: Wed, 13 Aug 2025 17:57:10 +0300 Subject: [PATCH 3/5] Fixed linter --- retry/sql.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/retry/sql.go b/retry/sql.go index dff3d577a..7a4938ee9 100644 --- a/retry/sql.go +++ b/retry/sql.go @@ -89,8 +89,10 @@ type doTxOption interface { ApplyDoTxOption(txOptions *sql.TxOptions) } -var _ Option = txOptionsOption{} -var _ doTxOption = txOptionsOption{} +var ( + _ Option = txOptionsOption{} + _ doTxOption = txOptionsOption{} +) type txOptionsOption struct { txOptions *sql.TxOptions @@ -154,6 +156,7 @@ func DoTxWithResult[T any](ctx context.Context, db *sql.DB, options[0] = WithTrace(d.TraceRetry()) options[1] = WithBudget(d.RetryBudget()) } + for _, opt := range opts { if opt != nil { if txOpt, ok := opt.(doTxOption); ok { From 8699ccf7fea18c80dfe08cac7436b39f581ca3c3 Mon Sep 17 00:00:00 2001 From: Viktor Pentyukhov Date: Wed, 13 Aug 2025 18:08:03 +0300 Subject: [PATCH 4/5] Fixed linter --- retry/sql.go | 1 - 1 file changed, 1 deletion(-) diff --git a/retry/sql.go b/retry/sql.go index 7a4938ee9..c1637744d 100644 --- a/retry/sql.go +++ b/retry/sql.go @@ -156,7 +156,6 @@ func DoTxWithResult[T any](ctx context.Context, db *sql.DB, options[0] = WithTrace(d.TraceRetry()) options[1] = WithBudget(d.RetryBudget()) } - for _, opt := range opts { if opt != nil { if txOpt, ok := opt.(doTxOption); ok { From ef2d2ed4f91c5baa654695f26085c538b968bfa5 Mon Sep 17 00:00:00 2001 From: Viktor Pentyukhov Date: Wed, 13 Aug 2025 18:34:25 +0300 Subject: [PATCH 5/5] Refactored Changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22c176e3f..b4cafd593 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -* Refactored `retry.Do`/`retry.DoWithResult` and `retry.DoTx`/`retry.DoTxWithResult` option handling to unify Option usage and simplify implementation +* Changed `retry.Do`/`retry.DoWithResult` and `retry.DoTx`/`retry.DoTxWithResult` option handling to unify Option usage and simplify implementation ## v3.113.6 * Removed experimental label from `retry.DoWithResult` and `retry.DoTxWithResult`