Skip to content

Commit

Permalink
Merge pull request #875 from ydb-platform/with-label
Browse files Browse the repository at this point in the history
* Renamed `{retry,table}.WithID` option to `{retry,table}.WithLabel`
  • Loading branch information
asmyasnikov authored Oct 24, 2023
2 parents 43bf888 + 7f56a00 commit eae4c63
Show file tree
Hide file tree
Showing 13 changed files with 86 additions and 58 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
* Renamed `{retry,table}.WithID` option to `{retry,table}.WithLabel`
* Added `ydb.WithTraceRetry` option
* Moved `internal/allocator.Buffers` to package `internal/xstring`
* Bumped `golang.org/x/sync` to `v0.3.0`
Expand Down
12 changes: 5 additions & 7 deletions internal/table/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,8 @@ func doTx(
if opts.Trace == nil {
opts.Trace = &trace.Table{}
}
attempts, onIntermediate := 0, trace.TableOnDoTx(
opts.Trace,
&ctx,
opts.ID,
opts.Idempotent,
isRetryCalledAbove(ctx),
attempts, onIntermediate := 0, trace.TableOnDoTx(opts.Trace, &ctx,
opts.Label, opts.Label, opts.Idempotent, isRetryCalledAbove(ctx),
)
defer func() {
onIntermediate(err)(attempts, err)
Expand Down Expand Up @@ -119,7 +115,9 @@ func do(
if opts.Trace == nil {
opts.Trace = &trace.Table{}
}
attempts, onIntermediate := 0, trace.TableOnDo(opts.Trace, &ctx, opts.ID, opts.Idempotent, isRetryCalledAbove(ctx))
attempts, onIntermediate := 0, trace.TableOnDo(opts.Trace, &ctx,
opts.Label, opts.Label, opts.Idempotent, isRetryCalledAbove(ctx),
)
defer func() {
onIntermediate(err)(attempts, err)
}()
Expand Down
12 changes: 6 additions & 6 deletions log/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ func internalRetry(l *wrapper, d trace.Detailer) (t trace.Retry) {
return nil
}
ctx := with(*info.Context, TRACE, "ydb", "retry")
id := info.ID
label := info.Label
idempotent := info.Idempotent
l.Log(ctx, "start",
String("id", id),
String("label", label),
Bool("idempotent", idempotent),
)
start := time.Now()
return func(info trace.RetryLoopIntermediateInfo) func(trace.RetryLoopDoneInfo) {
if info.Error == nil {
l.Log(ctx, "attempt done",
String("id", id),
String("label", label),
latencyField(start),
)
} else {
Expand All @@ -46,7 +46,7 @@ func internalRetry(l *wrapper, d trace.Detailer) (t trace.Retry) {
m := retry.Check(info.Error)
l.Log(WithLevel(ctx, lvl), "attempt failed",
Error(info.Error),
String("id", id),
String("label", label),
latencyField(start),
Bool("retryable", m.MustRetry(idempotent)),
Int64("code", m.StatusCode()),
Expand All @@ -57,7 +57,7 @@ func internalRetry(l *wrapper, d trace.Detailer) (t trace.Retry) {
return func(info trace.RetryLoopDoneInfo) {
if info.Error == nil {
l.Log(ctx, "done",
String("id", id),
String("label", label),
latencyField(start),
Int("attempts", info.Attempts),
)
Expand All @@ -69,7 +69,7 @@ func internalRetry(l *wrapper, d trace.Detailer) (t trace.Retry) {
m := retry.Check(info.Error)
l.Log(WithLevel(ctx, lvl), "failed",
Error(info.Error),
String("id", id),
String("label", label),
latencyField(start),
Int("attempts", info.Attempts),
Bool("retryable", m.MustRetry(idempotent)),
Expand Down
12 changes: 12 additions & 0 deletions log/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,18 @@ func internalTable(l *wrapper, d trace.Detailer) (t trace.Table) {
}
ctx := with(*info.Context, TRACE, "ydb", "table", "do")
idempotent := info.Idempotent
label := info.Label
l.Log(ctx, "start",
Bool("idempotent", idempotent),
String("label", label),
)
start := time.Now()
return func(info trace.TableDoIntermediateInfo) func(trace.TableDoDoneInfo) {
if info.Error == nil {
l.Log(ctx, "done",
latencyField(start),
Bool("idempotent", idempotent),
String("label", label),
)
} else {
lvl := WARN
Expand All @@ -47,6 +50,7 @@ func internalTable(l *wrapper, d trace.Detailer) (t trace.Table) {
l.Log(WithLevel(ctx, lvl), "failed",
latencyField(start),
Bool("idempotent", idempotent),
String("label", label),
Error(info.Error),
Bool("retryable", m.MustRetry(idempotent)),
Int64("code", m.StatusCode()),
Expand All @@ -59,6 +63,7 @@ func internalTable(l *wrapper, d trace.Detailer) (t trace.Table) {
l.Log(ctx, "done",
latencyField(start),
Bool("idempotent", idempotent),
String("label", label),
Int("attempts", info.Attempts),
)
} else {
Expand All @@ -70,6 +75,7 @@ func internalTable(l *wrapper, d trace.Detailer) (t trace.Table) {
l.Log(WithLevel(ctx, lvl), "done",
latencyField(start),
Bool("idempotent", idempotent),
String("label", label),
Int("attempts", info.Attempts),
Error(info.Error),
Bool("retryable", m.MustRetry(idempotent)),
Expand All @@ -93,15 +99,18 @@ func internalTable(l *wrapper, d trace.Detailer) (t trace.Table) {
}
ctx := with(*info.Context, TRACE, "ydb", "table", "do", "tx")
idempotent := info.Idempotent
label := info.Label
l.Log(ctx, "start",
Bool("idempotent", idempotent),
String("label", label),
)
start := time.Now()
return func(info trace.TableDoTxIntermediateInfo) func(trace.TableDoTxDoneInfo) {
if info.Error == nil {
l.Log(ctx, "done",
latencyField(start),
Bool("idempotent", idempotent),
String("label", label),
)
} else {
lvl := ERROR
Expand All @@ -112,6 +121,7 @@ func internalTable(l *wrapper, d trace.Detailer) (t trace.Table) {
l.Log(WithLevel(ctx, lvl), "done",
latencyField(start),
Bool("idempotent", idempotent),
String("label", label),
Error(info.Error),
Bool("retryable", m.MustRetry(idempotent)),
Int64("code", m.StatusCode()),
Expand All @@ -124,6 +134,7 @@ func internalTable(l *wrapper, d trace.Detailer) (t trace.Table) {
l.Log(ctx, "done",
latencyField(start),
Bool("idempotent", idempotent),
String("label", label),
Int("attempts", info.Attempts),
)
} else {
Expand All @@ -135,6 +146,7 @@ func internalTable(l *wrapper, d trace.Detailer) (t trace.Table) {
l.Log(WithLevel(ctx, lvl), "done",
latencyField(start),
Bool("idempotent", idempotent),
String("label", label),
Int("attempts", info.Attempts),
Error(info.Error),
Bool("retryable", m.MustRetry(idempotent)),
Expand Down
16 changes: 8 additions & 8 deletions metrics/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,26 @@ func table(config Config) (t trace.Table) {
trace.TableDoDoneInfo,
) {
var (
name = info.ID
label = info.Label
start = time.Now()
)
return func(info trace.TableDoIntermediateInfo) func(trace.TableDoDoneInfo) {
if info.Error != nil && config.Details()&trace.TableEvents != 0 {
doIntermediateErrors.With(map[string]string{
"status": errorBrief(info.Error),
"name": name,
"label": label,
}).Inc()
}
return func(info trace.TableDoDoneInfo) {
if config.Details()&trace.TableEvents != 0 {
doAttempts.With(nil).Record(float64(info.Attempts))
doErrors.With(map[string]string{
"status": errorBrief(info.Error),
"name": name,
"label": label,
}).Inc()
doLatency.With(map[string]string{
"status": errorBrief(info.Error),
"name": name,
"label": label,
}).Record(time.Since(start))
}
}
Expand All @@ -67,26 +67,26 @@ func table(config Config) (t trace.Table) {
trace.TableDoTxDoneInfo,
) {
var (
name = info.ID
label = info.Label
start = time.Now()
)
return func(info trace.TableDoTxIntermediateInfo) func(trace.TableDoTxDoneInfo) {
if info.Error != nil && config.Details()&trace.TableEvents != 0 {
doTxIntermediateErrors.With(map[string]string{
"status": errorBrief(info.Error),
"name": name,
"label": label,
}).Inc()
}
return func(info trace.TableDoTxDoneInfo) {
if config.Details()&trace.TableEvents != 0 {
doTxAttempts.With(nil).Record(float64(info.Attempts))
doTxErrors.With(map[string]string{
"status": errorBrief(info.Error),
"name": name,
"label": label,
}).Inc()
doTxLatency.With(map[string]string{
"status": errorBrief(info.Error),
"name": name,
"label": label,
}).Record(time.Since(start))
}
}
Expand Down
28 changes: 15 additions & 13 deletions retry/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
type retryOperation func(context.Context) (err error)

type retryOptions struct {
id string
label string
trace *trace.Retry
idempotent bool
stackTrace bool
Expand All @@ -31,25 +31,25 @@ type Option interface {
ApplyRetryOption(opts *retryOptions)
}

var _ Option = idOption("")
var _ Option = labelOption("")

type idOption string
type labelOption string

func (id idOption) ApplyDoOption(opts *doOptions) {
opts.retryOptions = append(opts.retryOptions, WithID(string(id)))
func (label labelOption) ApplyDoOption(opts *doOptions) {
opts.retryOptions = append(opts.retryOptions, WithLabel(string(label)))
}

func (id idOption) ApplyDoTxOption(opts *doTxOptions) {
opts.retryOptions = append(opts.retryOptions, WithID(string(id)))
func (label labelOption) ApplyDoTxOption(opts *doTxOptions) {
opts.retryOptions = append(opts.retryOptions, WithLabel(string(label)))
}

func (id idOption) ApplyRetryOption(opts *retryOptions) {
opts.id = string(id)
func (label labelOption) ApplyRetryOption(opts *retryOptions) {
opts.label = string(label)
}

// WithID applies id for identification call Retry in trace.Retry.OnRetry
func WithID(id string) idOption {
return idOption(id)
// WithLabel applies label for identification call Retry in trace.Retry.OnRetry
func WithLabel(label string) labelOption {
return labelOption(label)
}

var _ Option = stackTraceOption{}
Expand Down Expand Up @@ -241,7 +241,9 @@ func Retry(ctx context.Context, op retryOperation, opts ...Option) (err error) {
attempts int

code = int64(0)
onIntermediate = trace.RetryOnRetry(options.trace, &ctx, options.id, options.idempotent, isRetryCalledAbove(ctx))
onIntermediate = trace.RetryOnRetry(options.trace, &ctx,
options.label, options.label, options.idempotent, isRetryCalledAbove(ctx),
)
)
defer func() {
onIntermediate(err)(attempts, err)
Expand Down
2 changes: 1 addition & 1 deletion retry/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type doOption interface {

var (
_ doOption = doRetryOptionsOption(nil)
_ doOption = idOption("")
_ doOption = labelOption("")
)

type doRetryOptionsOption []Option
Expand Down
16 changes: 8 additions & 8 deletions table/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ func ValueParam(name string, v types.Value) ParameterOption {
}

type Options struct {
ID string
Label string
Idempotent bool
TxSettings *TransactionSettings
TxCommitOptions []options.CommitTransactionOption
Expand All @@ -572,17 +572,17 @@ type Option interface {
ApplyTableOption(opts *Options)
}

var _ Option = idOption("")
var _ Option = labelOption("")

type idOption string
type labelOption string

func (id idOption) ApplyTableOption(opts *Options) {
opts.ID = string(id)
opts.RetryOptions = append(opts.RetryOptions, retry.WithID(string(id)))
func (label labelOption) ApplyTableOption(opts *Options) {
opts.Label = string(label)
opts.RetryOptions = append(opts.RetryOptions, retry.WithLabel(string(label)))
}

func WithID(id string) idOption {
return idOption(id)
func WithLabel(label string) labelOption {
return labelOption(label)
}

var _ Option = retryOptionsOption{}
Expand Down
12 changes: 6 additions & 6 deletions tests/integration/with_trace_retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestWithTraceRetry(t *testing.T) {
OnRetry: func(
info trace.RetryLoopStartInfo,
) func(trace.RetryLoopIntermediateInfo) func(trace.RetryLoopDoneInfo) {
retryCalled[info.ID] = true
retryCalled[info.Label] = true
return nil
},
}),
Expand All @@ -40,14 +40,14 @@ func TestWithTraceRetry(t *testing.T) {
func(ctx context.Context, s table.Session) error {
return nil
},
table.WithID("db.Table().Do"),
table.WithLabel("db.Table().Do"),
))

require.NoError(t, db.Table().DoTx(ctx,
func(ctx context.Context, tx table.TransactionActor) error {
return nil
},
table.WithID("db.Table().DoTx"),
table.WithLabel("db.Table().DoTx"),
))

for _, key := range []string{
Expand All @@ -67,7 +67,7 @@ func TestWithTraceRetry(t *testing.T) {
OnRetry: func(
info trace.RetryLoopStartInfo,
) func(trace.RetryLoopIntermediateInfo) func(trace.RetryLoopDoneInfo) {
retryCalled[info.ID] = true
retryCalled[info.Label] = true
return nil
},
}),
Expand All @@ -78,14 +78,14 @@ func TestWithTraceRetry(t *testing.T) {
func(ctx context.Context, cc *sql.Conn) error {
return nil
},
retry.WithID("retry.Do"),
retry.WithLabel("retry.Do"),
))

require.NoError(t, retry.DoTx(ctx, db,
func(ctx context.Context, tx *sql.Tx) error {
return nil
},
retry.WithID("retry.DoTx"),
retry.WithLabel("retry.DoTx"),
))

for _, key := range []string{
Expand Down
8 changes: 6 additions & 2 deletions trace/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,12 @@ type (
// Pointer to context provide replacement of context in trace callback function.
// Warning: concurrent access to pointer on client side must be excluded.
// Safe replacement of context are provided only inside callback function
Context *context.Context
ID string
Context *context.Context

// Deprecated: use Label field instead
ID string

Label string
Idempotent bool
NestedCall bool // flag when Retry called inside head Retry
}
Expand Down
Loading

0 comments on commit eae4c63

Please sign in to comment.