From 05674818fc655be5ecb4173d2bda5f12a4478a06 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 18 Apr 2024 13:55:30 +0300 Subject: [PATCH 01/33] Fixed data race in logger ydb/log/WithNames --- CHANGELOG.md | 2 ++ log/context.go | 12 +++++++++--- log/context_test.go | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03957b2fc..608699669 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +* Fixed data race in logger ydb/log/WithNames + ## v3.65.1 * Updated dependency `ydb-go-genproto` * Added processing of `Ydb.StatusIds_EXTERNAL_ERROR` in `retry.Retry` diff --git a/log/context.go b/log/context.go index 81231df01..b724e5ec3 100644 --- a/log/context.go +++ b/log/context.go @@ -1,6 +1,8 @@ package log -import "context" +import ( + "context" +) type ( ctxLevelKey struct{} @@ -18,7 +20,11 @@ func LevelFromContext(ctx context.Context) Level { } func WithNames(ctx context.Context, names ...string) context.Context { - return context.WithValue(ctx, ctxNamesKey{}, append(NamesFromContext(ctx), names...)) + // trim capacity for force allocate new memory while append and prevent data race + oldNames := NamesFromContext(ctx) + oldNames = oldNames[:len(oldNames):len(oldNames)] + + return context.WithValue(ctx, ctxNamesKey{}, append(oldNames, names...)) } func NamesFromContext(ctx context.Context) []string { @@ -27,7 +33,7 @@ func NamesFromContext(ctx context.Context) []string { return []string{} } - return v + return v[:len(v):len(v)] // prevent re } func with(ctx context.Context, lvl Level, names ...string) context.Context { diff --git a/log/context_test.go b/log/context_test.go index 3d8c05067..92882b887 100644 --- a/log/context_test.go +++ b/log/context_test.go @@ -2,9 +2,13 @@ package log import ( "context" + "strconv" "testing" + "time" "github.com/stretchr/testify/require" + + "github.com/ydb-platform/ydb-go-sdk/v3/internal/xtest" ) func TestLevelFromContext(t *testing.T) { @@ -54,3 +58,35 @@ func TestNamesFromContext(t *testing.T) { }) } } + +func TestWithNamesRaceRegression(t *testing.T) { + count := 100 + xtest.TestManyTimes(t, func(t testing.TB) { + ctx := WithNames(context.Background(), "test") + ctx = WithNames(ctx, "test") + ctx = WithNames(ctx, "test") + res := make([]context.Context, count) + + start := make(chan bool) + finished := make(chan bool) + for i := 0; i < count; i++ { + go func(index int) { + <-start + res[index] = WithNames(ctx, strconv.Itoa(index)) + finished <- true + }(i) + } + + time.Sleep(time.Microsecond) + close(start) + + for i := 0; i < count; i++ { + <-finished + } + + for i := 0; i < count; i++ { + expected := []string{"test", "test", "test", strconv.Itoa(i)} + require.Equal(t, expected, NamesFromContext(res[i])) + } + }) +} From 7f832c8859c26c298c6008e2ea2109996f10897f Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Thu, 18 Apr 2024 15:15:48 +0300 Subject: [PATCH 02/33] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 608699669..bd29af1ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -* Fixed data race in logger ydb/log/WithNames +* Fixed data race using `log.WithNames` ## v3.65.1 * Updated dependency `ydb-go-genproto` From bf0fd3377f62047575760db6f82c2ead8b207e61 Mon Sep 17 00:00:00 2001 From: robot Date: Thu, 18 Apr 2024 12:22:27 +0000 Subject: [PATCH 03/33] Release v3.65.2 --- CHANGELOG.md | 1 + internal/version/version.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd29af1ca..4c5bd059a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,4 @@ +## v3.65.2 * Fixed data race using `log.WithNames` ## v3.65.1 diff --git a/internal/version/version.go b/internal/version/version.go index 0238c9656..1238601f4 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -3,7 +3,7 @@ package version const ( Major = "3" Minor = "65" - Patch = "1" + Patch = "2" Prefix = "ydb-go-sdk" ) From b8474db57b11abb0e0dcb15082aa33db1c786f9c Mon Sep 17 00:00:00 2001 From: Roman Golov Date: Thu, 18 Apr 2024 21:40:33 +0300 Subject: [PATCH 04/33] Fix data race in grpc Client stream. --- CHANGELOG.md | 2 ++ internal/conn/grpc_client_stream.go | 11 +++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c5bd059a..ab9c9c2d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +* Fixed data race in `grpcClientStream` + ## v3.65.2 * Fixed data race using `log.WithNames` diff --git a/internal/conn/grpc_client_stream.go b/internal/conn/grpc_client_stream.go index 32377e5ab..96ab744fc 100644 --- a/internal/conn/grpc_client_stream.go +++ b/internal/conn/grpc_client_stream.go @@ -25,7 +25,8 @@ type grpcClientStream struct { } func (s *grpcClientStream) CloseSend() (err error) { - onDone := trace.DriverOnConnStreamCloseSend(s.c.config.Trace(), &s.ctx, + ctx := s.ctx + onDone := trace.DriverOnConnStreamCloseSend(s.c.config.Trace(), &ctx, stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/conn.(*grpcClientStream).CloseSend"), ) defer func() { @@ -59,7 +60,8 @@ func (s *grpcClientStream) CloseSend() (err error) { } func (s *grpcClientStream) SendMsg(m interface{}) (err error) { - onDone := trace.DriverOnConnStreamSendMsg(s.c.config.Trace(), &s.ctx, + ctx := s.ctx + onDone := trace.DriverOnConnStreamSendMsg(s.c.config.Trace(), &ctx, stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/conn.(*grpcClientStream).SendMsg"), ) defer func() { @@ -101,7 +103,8 @@ func (s *grpcClientStream) SendMsg(m interface{}) (err error) { } func (s *grpcClientStream) RecvMsg(m interface{}) (err error) { - onDone := trace.DriverOnConnStreamRecvMsg(s.c.config.Trace(), &s.ctx, + ctx := s.ctx + onDone := trace.DriverOnConnStreamRecvMsg(s.c.config.Trace(), &ctx, stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/conn.(*grpcClientStream).RecvMsg"), ) defer func() { @@ -114,7 +117,7 @@ func (s *grpcClientStream) RecvMsg(m interface{}) (err error) { defer func() { if err != nil { md := s.ClientStream.Trailer() - s.onDone(s.ctx, md) + s.onDone(ctx, md) } }() From ecc181139d2a3e2d894943be35ae2f23e6a3efa2 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Thu, 18 Apr 2024 23:48:54 +0300 Subject: [PATCH 05/33] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab9c9c2d9..796256177 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -* Fixed data race in `grpcClientStream` +* Fixed data race in `internal/conn.grpcClientStream` ## v3.65.2 * Fixed data race using `log.WithNames` From 75ccf7e44a58aa46fd65c27d97a03bf9927d1abf Mon Sep 17 00:00:00 2001 From: robot Date: Thu, 18 Apr 2024 20:55:06 +0000 Subject: [PATCH 06/33] Release v3.65.3 --- CHANGELOG.md | 1 + internal/version/version.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 796256177..a24369100 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,4 @@ +## v3.65.3 * Fixed data race in `internal/conn.grpcClientStream` ## v3.65.2 diff --git a/internal/version/version.go b/internal/version/version.go index 1238601f4..d89575acc 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -3,7 +3,7 @@ package version const ( Major = "3" Minor = "65" - Patch = "2" + Patch = "3" Prefix = "ydb-go-sdk" ) From 7eb926048624cbf3c317e40fe224eb5b76a7bb14 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 19 Apr 2024 12:16:46 +0000 Subject: [PATCH 07/33] Bump golang.org/x/net from 0.17.0 to 0.23.0 in /tests/slo Bumps [golang.org/x/net](https://github.com/golang/net) from 0.17.0 to 0.23.0. - [Commits](https://github.com/golang/net/compare/v0.17.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] --- tests/slo/go.mod | 4 ++-- tests/slo/go.sum | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/slo/go.mod b/tests/slo/go.mod index f0f99850d..31a355a50 100644 --- a/tests/slo/go.mod +++ b/tests/slo/go.mod @@ -39,8 +39,8 @@ require ( github.com/ydb-platform/ydb-go-sdk-auth-environ v0.2.0 // indirect github.com/ydb-platform/ydb-go-yc v0.10.2 // indirect github.com/ydb-platform/ydb-go-yc-metadata v0.5.3 // indirect - golang.org/x/net v0.17.0 // indirect - golang.org/x/sys v0.16.0 // indirect + golang.org/x/net v0.23.0 // indirect + golang.org/x/sys v0.18.0 // indirect golang.org/x/text v0.14.0 // indirect google.golang.org/genproto v0.0.0-20240102182953-50ed04b92917 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240108191215-35c7eff3a6b1 // indirect diff --git a/tests/slo/go.sum b/tests/slo/go.sum index 59f24a544..901f5c500 100644 --- a/tests/slo/go.sum +++ b/tests/slo/go.sum @@ -1387,8 +1387,9 @@ golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= -golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= +golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs= +golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -1539,8 +1540,8 @@ golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU= -golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= +golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= From 53f8c3e5b40ba184aaef1586505c8ee372073795 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 19 Apr 2024 12:17:18 +0000 Subject: [PATCH 08/33] Bump golang.org/x/net from 0.17.0 to 0.23.0 in /examples Bumps [golang.org/x/net](https://github.com/golang/net) from 0.17.0 to 0.23.0. - [Commits](https://github.com/golang/net/compare/v0.17.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: indirect ... Signed-off-by: dependabot[bot] --- examples/go.mod | 6 +++--- examples/go.sum | 11 ++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/examples/go.mod b/examples/go.mod index 0e3ecce2c..29d8cfcfc 100644 --- a/examples/go.mod +++ b/examples/go.mod @@ -53,10 +53,10 @@ require ( github.com/yandex-cloud/go-genproto v0.0.0-20220815090733-4c139c0154e2 // indirect github.com/ydb-platform/ydb-go-genproto v0.0.0-20240316140903-4a47abca1cca // indirect github.com/ydb-platform/ydb-go-yc-metadata v0.5.4 // indirect - golang.org/x/crypto v0.17.0 // indirect + golang.org/x/crypto v0.21.0 // indirect golang.org/x/mod v0.11.0 // indirect - golang.org/x/net v0.17.0 // indirect - golang.org/x/sys v0.15.0 // indirect + golang.org/x/net v0.23.0 // indirect + golang.org/x/sys v0.18.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/tools v0.7.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20230525234035-dd9d682886f9 // indirect diff --git a/examples/go.sum b/examples/go.sum index f3fbe27f4..e65a7e091 100644 --- a/examples/go.sum +++ b/examples/go.sum @@ -1270,8 +1270,8 @@ golang.org/x/crypto v0.0.0-20211108221036-ceb1ce70b4fa/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58= golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= -golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k= -golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= +golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA= +golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -1401,8 +1401,9 @@ golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= -golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= +golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs= +golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -1552,8 +1553,8 @@ golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= -golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= +golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= From 1debec69670685a46a3f7f5a0b52d6f95eeddc60 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 19 Apr 2024 12:27:19 +0000 Subject: [PATCH 09/33] Bump golang.org/x/net from 0.17.0 to 0.23.0 Bumps [golang.org/x/net](https://github.com/golang/net) from 0.17.0 to 0.23.0. - [Commits](https://github.com/golang/net/compare/v0.17.0...v0.23.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- go.mod | 6 +++--- go.sum | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 6f67d5b64..6b27ea49d 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/google/uuid v1.3.0 github.com/jonboulle/clockwork v0.3.0 github.com/ydb-platform/ydb-go-genproto v0.0.0-20240316140903-4a47abca1cca - golang.org/x/net v0.17.0 + golang.org/x/net v0.23.0 golang.org/x/sync v0.3.0 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 google.golang.org/grpc v1.57.1 @@ -25,8 +25,8 @@ require ( github.com/davecgh/go-spew v1.1.0 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/sys v0.13.0 // indirect - golang.org/x/text v0.13.0 // indirect + golang.org/x/sys v0.18.0 // indirect + golang.org/x/text v0.14.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20230525234030-28d5490b6b19 // indirect gopkg.in/yaml.v3 v3.0.0 // indirect ) diff --git a/go.sum b/go.sum index 8d8988e36..c8dbabbb6 100644 --- a/go.sum +++ b/go.sum @@ -84,8 +84,8 @@ golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20200822124328-c89045814202/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= -golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= +golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs= +golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -100,12 +100,12 @@ golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= -golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= +golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k= -golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= +golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= +golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY= From 8d5b7bde3b78bbc92c5952491bd8b14f32e5c804 Mon Sep 17 00:00:00 2001 From: Vlad Arkhipov Date: Sat, 20 Apr 2024 14:34:17 +0200 Subject: [PATCH 10/33] fix: re-send cancel message on coordination session re-attach If a coordination service session re-attaches, idempotent conversations requests are being re-sent. However, if a conversation has been canceled, only its cancelation request must be re-send. The previous version may re-send original requests for already canceled conversations which may lead to inconsistency between the client and server state. --- CHANGELOG.md | 2 ++ internal/coordination/conversation/conversation.go | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a24369100..1af6422e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +* Fixed the hanging semaphore issue on coordination session reconnect + ## v3.65.3 * Fixed data race in `internal/conn.grpcClientStream` diff --git a/internal/coordination/conversation/conversation.go b/internal/coordination/conversation/conversation.go index c3c8e1863..83e170532 100644 --- a/internal/coordination/conversation/conversation.go +++ b/internal/coordination/conversation/conversation.go @@ -424,7 +424,12 @@ func (c *Controller) OnAttach() { delete(c.conflicts, req.conflictKey) } - req.requestSent = nil + // If the request has been canceled, re-send the cancellation message, otherwise re-send the original one. + if req.canceled { + req.cancelRequestSent = nil + } else { + req.requestSent = nil + } notify = true } } From afc0e8dd7019abbb586589223679a59b482e108a Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Sun, 21 Apr 2024 15:08:45 +0300 Subject: [PATCH 11/33] Enabled containedctx linter --- .golangci.yml | 1 - CHANGELOG.md | 2 + driver.go | 2 - .../topic/topicreader/topicreader_trace.go | 4 +- internal/background/worker.go | 2 +- internal/conn/conn.go | 1 - internal/conn/grpc_client_stream.go | 29 +++++++----- internal/coordination/client_test.go | 2 +- internal/coordination/session.go | 4 +- internal/meta/context_test.go | 2 +- internal/operation/params_test.go | 2 +- internal/query/row.go | 23 +++++++--- internal/table/scanner/result_test.go | 2 +- .../topicreaderinternal/partition_session.go | 2 +- internal/topic/topicreaderinternal/reader.go | 1 - .../topicreaderinternal/stream_reader_impl.go | 46 +++++++++++-------- .../stream_reader_impl_test.go | 10 ++-- .../topicreaderinternal/stream_reconnector.go | 4 -- .../writer_reconnector_test.go | 2 +- internal/xcontext/context_with_cancel.go | 4 +- internal/xcontext/context_with_timeout.go | 7 +-- internal/xcontext/without_deadline.go | 4 +- internal/xsql/conn.go | 21 +++++---- internal/xsql/stmt.go | 43 +++++++++-------- internal/xsql/tx.go | 42 +++++++++-------- internal/xsql/tx_fake.go | 26 +++++++---- log/context_test.go | 4 +- trace/sql.go | 10 ++-- trace/sql_gtrace.go | 2 +- trace/topic.go | 8 ++-- trace/topic_gtrace.go | 8 ++-- 31 files changed, 177 insertions(+), 143 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index e2100ce23..102d0e3ed 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -218,7 +218,6 @@ linters-settings: linters: enable-all: true disable: - - containedctx - contextcheck - cyclop - depguard diff --git a/CHANGELOG.md b/CHANGELOG.md index a24369100..ac6c9f81d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +* Refactored internals for enabling `containedctx` linter + ## v3.65.3 * Fixed data race in `internal/conn.grpcClientStream` diff --git a/driver.go b/driver.go index 592466443..170e2f86d 100644 --- a/driver.go +++ b/driver.go @@ -51,7 +51,6 @@ var _ Connection = (*Driver)(nil) // Driver type provide access to YDB service clients type Driver struct { - ctx context.Context // cancel while Driver.Close called. ctxCancel context.CancelFunc userInfo *dsn.UserInfo @@ -311,7 +310,6 @@ func newConnectionFromOptions(ctx context.Context, opts ...Option) (_ *Driver, e d := &Driver{ children: make(map[uint64]*Driver), - ctx: ctx, ctxCancel: driverCtxCancel, } diff --git a/examples/topic/topicreader/topicreader_trace.go b/examples/topic/topicreader/topicreader_trace.go index b31390fb2..0a332efa8 100644 --- a/examples/topic/topicreader/topicreader_trace.go +++ b/examples/topic/topicreader/topicreader_trace.go @@ -40,7 +40,7 @@ func ExplicitPartitionStartStopHandler(ctx context.Context, db *ydb.Driver) { ) func( trace.TopicReaderPartitionReadStartResponseDoneInfo, ) { - err := externalSystemLock(info.PartitionContext, info.Topic, info.PartitionID) + err := externalSystemLock(*info.PartitionContext, info.Topic, info.PartitionID) if err != nil { stopReader() } @@ -105,7 +105,7 @@ func PartitionStartStopHandlerAndOwnReadProgressStorage(ctx context.Context, db ) func( trace.TopicReaderPartitionReadStartResponseDoneInfo, ) { - err := externalSystemLock(info.PartitionContext, info.Topic, info.PartitionID) + err := externalSystemLock(*info.PartitionContext, info.Topic, info.PartitionID) if err != nil { stopReader() } diff --git a/internal/background/worker.go b/internal/background/worker.go index 91c0d1686..c41ad4b0a 100644 --- a/internal/background/worker.go +++ b/internal/background/worker.go @@ -19,7 +19,7 @@ var ( // A Worker must not be copied after first use type Worker struct { - ctx context.Context + ctx context.Context //nolint:containedctx workers sync.WaitGroup closeReason error tasksCompleted empty.Chan diff --git a/internal/conn/conn.go b/internal/conn/conn.go index 8192e38e7..d7e05bd42 100644 --- a/internal/conn/conn.go +++ b/internal/conn/conn.go @@ -462,7 +462,6 @@ func (c *conn) NewStream( return &grpcClientStream{ ClientStream: s, - ctx: ctx, c: c, wrapping: useWrapping, traceID: traceID, diff --git a/internal/conn/grpc_client_stream.go b/internal/conn/grpc_client_stream.go index 96ab744fc..ea7c08cfe 100644 --- a/internal/conn/grpc_client_stream.go +++ b/internal/conn/grpc_client_stream.go @@ -16,7 +16,6 @@ import ( type grpcClientStream struct { grpc.ClientStream - ctx context.Context c *conn wrapping bool traceID string @@ -25,9 +24,11 @@ type grpcClientStream struct { } func (s *grpcClientStream) CloseSend() (err error) { - ctx := s.ctx - onDone := trace.DriverOnConnStreamCloseSend(s.c.config.Trace(), &ctx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/conn.(*grpcClientStream).CloseSend"), + var ( + ctx = s.Context() + onDone = trace.DriverOnConnStreamCloseSend(s.c.config.Trace(), &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/conn.(*grpcClientStream).CloseSend"), + ) ) defer func() { onDone(err) @@ -60,9 +61,11 @@ func (s *grpcClientStream) CloseSend() (err error) { } func (s *grpcClientStream) SendMsg(m interface{}) (err error) { - ctx := s.ctx - onDone := trace.DriverOnConnStreamSendMsg(s.c.config.Trace(), &ctx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/conn.(*grpcClientStream).SendMsg"), + var ( + ctx = s.Context() + onDone = trace.DriverOnConnStreamSendMsg(s.c.config.Trace(), &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/conn.(*grpcClientStream).SendMsg"), + ) ) defer func() { onDone(err) @@ -79,7 +82,7 @@ func (s *grpcClientStream) SendMsg(m interface{}) (err error) { } defer func() { - s.c.onTransportError(s.Context(), err) + s.c.onTransportError(ctx, err) }() if s.wrapping { @@ -103,9 +106,11 @@ func (s *grpcClientStream) SendMsg(m interface{}) (err error) { } func (s *grpcClientStream) RecvMsg(m interface{}) (err error) { - ctx := s.ctx - onDone := trace.DriverOnConnStreamRecvMsg(s.c.config.Trace(), &ctx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/conn.(*grpcClientStream).RecvMsg"), + var ( + ctx = s.Context() + onDone = trace.DriverOnConnStreamRecvMsg(s.c.config.Trace(), &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/conn.(*grpcClientStream).RecvMsg"), + ) ) defer func() { onDone(err) @@ -130,7 +135,7 @@ func (s *grpcClientStream) RecvMsg(m interface{}) (err error) { defer func() { if !xerrors.Is(err, io.EOF) { - s.c.onTransportError(s.Context(), err) + s.c.onTransportError(ctx, err) } }() diff --git a/internal/coordination/client_test.go b/internal/coordination/client_test.go index d7778be95..dd1b46f76 100644 --- a/internal/coordination/client_test.go +++ b/internal/coordination/client_test.go @@ -128,7 +128,7 @@ func TestDescribeNodeRequest(t *testing.T) { func TestOperationParams(t *testing.T) { for _, tt := range []struct { name string - ctx context.Context + ctx context.Context //nolint:containedctx config interface { OperationTimeout() time.Duration OperationCancelAfter() time.Duration diff --git a/internal/coordination/session.go b/internal/coordination/session.go index 9e7e2bb23..5c6a17ac5 100644 --- a/internal/coordination/session.go +++ b/internal/coordination/session.go @@ -23,7 +23,7 @@ type session struct { options *options.CreateSessionOptions client *Client - ctx context.Context + ctx context.Context //nolint:containedctx cancel context.CancelFunc sessionClosedChan chan struct{} controller *conversation.Controller @@ -37,7 +37,7 @@ type session struct { type lease struct { session *session name string - ctx context.Context + ctx context.Context //nolint:containedctx cancel context.CancelFunc } diff --git a/internal/meta/context_test.go b/internal/meta/context_test.go index 1bfdabcc5..6d3ae817e 100644 --- a/internal/meta/context_test.go +++ b/internal/meta/context_test.go @@ -11,7 +11,7 @@ import ( func TestContext(t *testing.T) { for _, tt := range []struct { name string - ctx context.Context + ctx context.Context //nolint:containedctx header string values []string }{ diff --git a/internal/operation/params_test.go b/internal/operation/params_test.go index 605453279..74b136b96 100644 --- a/internal/operation/params_test.go +++ b/internal/operation/params_test.go @@ -14,7 +14,7 @@ import ( func TestParams(t *testing.T) { for _, tt := range []struct { - ctx context.Context + ctx context.Context //nolint:containedctx preferContextTimeout bool timeout time.Duration cancelAfter time.Duration diff --git a/internal/query/row.go b/internal/query/row.go index 476b6aa14..a9db3f61b 100644 --- a/internal/query/row.go +++ b/internal/query/row.go @@ -14,7 +14,7 @@ import ( var _ query.Row = (*row)(nil) type row struct { - ctx context.Context + ctx context.Context //nolint:containedctx trace *trace.Query indexedScanner scanner.IndexedScanner @@ -35,8 +35,11 @@ func newRow(ctx context.Context, columns []*Ydb.Column, v *Ydb.Value, t *trace.Q } func (r row) Scan(dst ...interface{}) (err error) { - onDone := trace.QueryOnRowScan(r.trace, &r.ctx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/query.row.Scan"), + var ( + ctx = r.ctx + onDone = trace.QueryOnRowScan(r.trace, &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/query.row.Scan"), + ) ) defer func() { onDone(err) @@ -46,8 +49,11 @@ func (r row) Scan(dst ...interface{}) (err error) { } func (r row) ScanNamed(dst ...scanner.NamedDestination) (err error) { - onDone := trace.QueryOnRowScanNamed(r.trace, &r.ctx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/query.row.ScanNamed"), + var ( + ctx = r.ctx + onDone = trace.QueryOnRowScanNamed(r.trace, &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/query.row.ScanNamed"), + ) ) defer func() { onDone(err) @@ -57,8 +63,11 @@ func (r row) ScanNamed(dst ...scanner.NamedDestination) (err error) { } func (r row) ScanStruct(dst interface{}, opts ...scanner.ScanStructOption) (err error) { - onDone := trace.QueryOnRowScanStruct(r.trace, &r.ctx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/query.row.ScanStruct"), + var ( + ctx = r.ctx + onDone = trace.QueryOnRowScanStruct(r.trace, &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/query.row.ScanStruct"), + ) ) defer func() { onDone(err) diff --git a/internal/table/scanner/result_test.go b/internal/table/scanner/result_test.go index dc81b0203..b7619cc11 100644 --- a/internal/table/scanner/result_test.go +++ b/internal/table/scanner/result_test.go @@ -207,7 +207,7 @@ func NewResultSet(a *allocator.Allocator, opts ...ResultSetOption) *Ydb.ResultSe func TestNewStreamWithRecvFirstResultSet(t *testing.T) { for _, tt := range []struct { - ctx context.Context + ctx context.Context //nolint:containedctx recvCounter int err error }{ diff --git a/internal/topic/topicreaderinternal/partition_session.go b/internal/topic/topicreaderinternal/partition_session.go index 9e23c5495..781de1782 100644 --- a/internal/topic/topicreaderinternal/partition_session.go +++ b/internal/topic/topicreaderinternal/partition_session.go @@ -24,7 +24,7 @@ type partitionSession struct { readerID int64 connectionID string - ctx context.Context + ctx context.Context //nolint:containedctx ctxCancel context.CancelFunc partitionSessionID rawtopicreader.PartitionSessionID diff --git a/internal/topic/topicreaderinternal/reader.go b/internal/topic/topicreaderinternal/reader.go index b499221d1..bc2dc865c 100644 --- a/internal/topic/topicreaderinternal/reader.go +++ b/internal/topic/topicreaderinternal/reader.go @@ -102,7 +102,6 @@ func NewReader( cfg.OperationTimeout(), cfg.RetrySettings, cfg.Trace, - cfg.BaseContext, ), defaultBatchConfig: cfg.DefaultBatchConfig, tracer: cfg.Trace, diff --git a/internal/topic/topicreaderinternal/stream_reader_impl.go b/internal/topic/topicreaderinternal/stream_reader_impl.go index fec186ffc..8eb91e0ac 100644 --- a/internal/topic/topicreaderinternal/stream_reader_impl.go +++ b/internal/topic/topicreaderinternal/stream_reader_impl.go @@ -34,7 +34,7 @@ type partitionSessionID = rawtopicreader.PartitionSessionID type topicStreamReaderImpl struct { cfg topicStreamReaderConfig - ctx context.Context + ctx context.Context //nolint:containedctx cancel context.CancelFunc freeBytes chan int @@ -60,7 +60,7 @@ type topicStreamReaderImpl struct { type topicStreamReaderConfig struct { CommitterBatchTimeLag time.Duration CommitterBatchCounterTrigger int - BaseContext context.Context + BaseContext context.Context //nolint:containedctx BufferSizeProtoBytes int Cred credentials.Credentials CredUpdateInterval time.Duration @@ -179,7 +179,7 @@ func (r *topicStreamReaderImpl) ReadMessageBatch( ) (batch *PublicBatch, err error) { onDone := trace.TopicOnReaderReadMessages( r.cfg.Trace, - ctx, + &ctx, opts.MinCount, opts.MaxCount, r.getRestBufferBytes(), @@ -295,15 +295,18 @@ func (r *topicStreamReaderImpl) onStopPartitionSessionRequestFromBuffer( return err } - onDone := trace.TopicOnReaderPartitionReadStopResponse( - r.cfg.Trace, - r.readConnectionID, - session.Context(), - session.Topic, - session.PartitionID, - session.partitionSessionID.ToInt64(), - msg.CommittedOffset.ToInt64(), - msg.Graceful, + var ( + ctx = session.Context() + onDone = trace.TopicOnReaderPartitionReadStopResponse( + r.cfg.Trace, + r.readConnectionID, + &ctx, + session.Topic, + session.PartitionID, + session.partitionSessionID.ToInt64(), + msg.CommittedOffset.ToInt64(), + msg.Graceful, + ) ) defer func() { onDone(err) @@ -357,7 +360,7 @@ func (r *topicStreamReaderImpl) Commit(ctx context.Context, commitRange commitRa session := commitRange.partitionSession onDone := trace.TopicOnReaderCommit( r.cfg.Trace, - ctx, + &ctx, session.Topic, session.PartitionID, session.partitionSessionID.ToInt64(), @@ -768,13 +771,16 @@ func (r *topicStreamReaderImpl) onStartPartitionSessionRequestFromBuffer( return err } - onDone := trace.TopicOnReaderPartitionReadStartResponse( - r.cfg.Trace, - r.readConnectionID, - session.Context(), - session.Topic, - session.PartitionID, - session.partitionSessionID.ToInt64(), + var ( + ctx = session.Context() + onDone = trace.TopicOnReaderPartitionReadStartResponse( + r.cfg.Trace, + r.readConnectionID, + &ctx, + session.Topic, + session.PartitionID, + session.partitionSessionID.ToInt64(), + ) ) respMessage := &rawtopicreader.StartPartitionSessionResponse{ diff --git a/internal/topic/topicreaderinternal/stream_reader_impl_test.go b/internal/topic/topicreaderinternal/stream_reader_impl_test.go index a518c2058..22fb2997a 100644 --- a/internal/topic/topicreaderinternal/stream_reader_impl_test.go +++ b/internal/topic/topicreaderinternal/stream_reader_impl_test.go @@ -379,7 +379,7 @@ func TestStreamReaderImpl_OnPartitionCloseHandle(t *testing.T) { e.reader.cfg.Trace.OnReaderPartitionReadStopResponse = func(info trace.TopicReaderPartitionReadStopResponseStartInfo) func(doneInfo trace.TopicReaderPartitionReadStopResponseDoneInfo) { //nolint:lll expected := trace.TopicReaderPartitionReadStopResponseStartInfo{ ReaderConnectionID: e.reader.readConnectionID, - PartitionContext: e.partitionSession.ctx, + PartitionContext: &e.partitionSession.ctx, Topic: e.partitionSession.Topic, PartitionID: e.partitionSession.PartitionID, PartitionSessionID: e.partitionSession.partitionSessionID.ToInt64(), @@ -388,7 +388,7 @@ func TestStreamReaderImpl_OnPartitionCloseHandle(t *testing.T) { } require.Equal(t, expected, info) - require.NoError(t, info.PartitionContext.Err()) + require.NoError(t, (*info.PartitionContext).Err()) readMessagesCtxCancel() @@ -424,7 +424,7 @@ func TestStreamReaderImpl_OnPartitionCloseHandle(t *testing.T) { e.reader.cfg.Trace.OnReaderPartitionReadStopResponse = func(info trace.TopicReaderPartitionReadStopResponseStartInfo) func(doneInfo trace.TopicReaderPartitionReadStopResponseDoneInfo) { //nolint:lll expected := trace.TopicReaderPartitionReadStopResponseStartInfo{ ReaderConnectionID: e.reader.readConnectionID, - PartitionContext: e.partitionSession.ctx, + PartitionContext: &e.partitionSession.ctx, Topic: e.partitionSession.Topic, PartitionID: e.partitionSession.PartitionID, PartitionSessionID: e.partitionSession.partitionSessionID.ToInt64(), @@ -432,7 +432,7 @@ func TestStreamReaderImpl_OnPartitionCloseHandle(t *testing.T) { Graceful: false, } require.Equal(t, expected, info) - require.Error(t, info.PartitionContext.Err()) + require.Error(t, (*info.PartitionContext).Err()) readMessagesCtxCancel() @@ -954,7 +954,7 @@ func TestTopicStreamReadImpl_CommitWithBadSession(t *testing.T) { } type streamEnv struct { - ctx context.Context + ctx context.Context //nolint:containedctx t testing.TB reader *topicStreamReaderImpl stopReadEvents empty.Chan diff --git a/internal/topic/topicreaderinternal/stream_reconnector.go b/internal/topic/topicreaderinternal/stream_reconnector.go index ec601ba67..f43044624 100644 --- a/internal/topic/topicreaderinternal/stream_reconnector.go +++ b/internal/topic/topicreaderinternal/stream_reconnector.go @@ -31,7 +31,6 @@ type readerConnectFunc func(ctx context.Context) (batchedStreamReader, error) type readerReconnector struct { background background.Worker clock clockwork.Clock - baseContext context.Context retrySettings topic.RetrySettings streamVal batchedStreamReader streamErr error @@ -49,14 +48,12 @@ type readerReconnector struct { initDone bool } -//nolint:revive func newReaderReconnector( readerID int64, connector readerConnectFunc, connectTimeout time.Duration, retrySettings topic.RetrySettings, tracer *trace.Topic, - baseContext context.Context, ) *readerReconnector { res := &readerReconnector{ readerID: readerID, @@ -65,7 +62,6 @@ func newReaderReconnector( streamErr: errUnconnected, connectTimeout: connectTimeout, tracer: tracer, - baseContext: baseContext, retrySettings: retrySettings, } diff --git a/internal/topic/topicwriterinternal/writer_reconnector_test.go b/internal/topic/topicwriterinternal/writer_reconnector_test.go index 4bf18c6ff..68eae0466 100644 --- a/internal/topic/topicwriterinternal/writer_reconnector_test.go +++ b/internal/topic/topicwriterinternal/writer_reconnector_test.go @@ -821,7 +821,7 @@ func isClosed(ch <-chan struct{}) bool { } type testEnv struct { - ctx context.Context + ctx context.Context //nolint:containedctx stream *MockRawTopicWriterStream writer *WriterReconnector sendFromServerChannel chan sendFromServerResponse diff --git a/internal/xcontext/context_with_cancel.go b/internal/xcontext/context_with_cancel.go index 1deeac4bb..f8cc68923 100644 --- a/internal/xcontext/context_with_cancel.go +++ b/internal/xcontext/context_with_cancel.go @@ -16,8 +16,8 @@ func WithCancel(ctx context.Context) (context.Context, context.CancelFunc) { } type cancelCtx struct { - parentCtx context.Context - ctx context.Context + parentCtx context.Context //nolint:containedctx + ctx context.Context //nolint:containedctx ctxCancel context.CancelFunc m sync.Mutex diff --git a/internal/xcontext/context_with_timeout.go b/internal/xcontext/context_with_timeout.go index 5342798fe..b120fbd85 100644 --- a/internal/xcontext/context_with_timeout.go +++ b/internal/xcontext/context_with_timeout.go @@ -2,6 +2,7 @@ package xcontext import ( "context" + "errors" "sync" "time" @@ -19,8 +20,8 @@ func WithTimeout(ctx context.Context, t time.Duration) (context.Context, context } type timeoutCtx struct { - parentCtx context.Context - ctx context.Context + parentCtx context.Context //nolint:containedctx + ctx context.Context //nolint:containedctx ctxCancel context.CancelFunc from string @@ -44,7 +45,7 @@ func (ctx *timeoutCtx) Err() error { return ctx.err } - if ctx.ctx.Err() == context.DeadlineExceeded && ctx.parentCtx.Err() == nil { //nolint:errorlint + if errors.Is(ctx.ctx.Err(), context.DeadlineExceeded) && ctx.parentCtx.Err() == nil { ctx.err = errFrom(context.DeadlineExceeded, ctx.from) return ctx.err diff --git a/internal/xcontext/without_deadline.go b/internal/xcontext/without_deadline.go index 2e80a284f..4e14655f9 100644 --- a/internal/xcontext/without_deadline.go +++ b/internal/xcontext/without_deadline.go @@ -5,7 +5,9 @@ import ( "time" ) -type valueOnlyContext struct{ context.Context } +type valueOnlyContext struct { + context.Context //nolint:containedctx +} func (valueOnlyContext) Deadline() (deadline time.Time, ok bool) { return } diff --git a/internal/xsql/conn.go b/internal/xsql/conn.go index c5d670517..71404066a 100644 --- a/internal/xsql/conn.go +++ b/internal/xsql/conn.go @@ -68,7 +68,7 @@ func withTrace(t *trace.DatabaseSQL) connOption { type beginTxFunc func(ctx context.Context, txOptions driver.TxOptions) (currentTx, error) type conn struct { - openConnCtx context.Context + ctx context.Context //nolint:containedctx connector *Connector trace *trace.DatabaseSQL @@ -129,9 +129,9 @@ var ( func newConn(ctx context.Context, c *Connector, s table.ClosableSession, opts ...connOption) *conn { cc := &conn{ - openConnCtx: ctx, - connector: c, - session: s, + ctx: ctx, + connector: c, + session: s, } cc.beginTxFuncs = map[QueryMode]beginTxFunc{ DataQueryMode: cc.beginTx, @@ -169,7 +169,7 @@ func (c *conn) PrepareContext(ctx context.Context, query string) (_ driver.Stmt, return &stmt{ conn: c, processor: c, - stmtCtx: ctx, + ctx: ctx, query: query, trace: c.trace, }, nil @@ -414,9 +414,12 @@ func (c *conn) Ping(ctx context.Context) (finalErr error) { func (c *conn) Close() (finalErr error) { if c.closed.CompareAndSwap(false, true) { c.connector.detach(c) - onDone := trace.DatabaseSQLOnConnClose( - c.trace, &c.openConnCtx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*conn).Close"), + var ( + ctx = c.ctx + onDone = trace.DatabaseSQLOnConnClose( + c.trace, &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*conn).Close"), + ) ) defer func() { onDone(finalErr) @@ -424,7 +427,7 @@ func (c *conn) Close() (finalErr error) { if c.currentTx != nil { _ = c.currentTx.Rollback() } - err := c.session.Close(xcontext.ValueOnly(c.openConnCtx)) + err := c.session.Close(xcontext.ValueOnly(ctx)) if err != nil { return badconn.Map(xerrors.WithStackTrace(err)) } diff --git a/internal/xsql/stmt.go b/internal/xsql/stmt.go index 75b571acd..6909e55ef 100644 --- a/internal/xsql/stmt.go +++ b/internal/xsql/stmt.go @@ -17,8 +17,8 @@ type stmt struct { driver.ExecerContext driver.QueryerContext } - query string - stmtCtx context.Context + query string + ctx context.Context //nolint:containedctx trace *trace.DatabaseSQL } @@ -29,51 +29,54 @@ var ( _ driver.StmtExecContext = &stmt{} ) -func (s *stmt) QueryContext(ctx context.Context, args []driver.NamedValue) (_ driver.Rows, finalErr error) { - onDone := trace.DatabaseSQLOnStmtQuery(s.trace, &ctx, +func (stmt *stmt) QueryContext(ctx context.Context, args []driver.NamedValue) (_ driver.Rows, finalErr error) { + onDone := trace.DatabaseSQLOnStmtQuery(stmt.trace, &ctx, stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*stmt).QueryContext"), - s.stmtCtx, s.query, + stmt.ctx, stmt.query, ) defer func() { onDone(finalErr) }() - if !s.conn.isReady() { + if !stmt.conn.isReady() { return nil, badconn.Map(xerrors.WithStackTrace(errNotReadyConn)) } - switch m := queryModeFromContext(ctx, s.conn.defaultQueryMode); m { + switch m := queryModeFromContext(ctx, stmt.conn.defaultQueryMode); m { case DataQueryMode: - return s.processor.QueryContext(s.conn.withKeepInCache(ctx), s.query, args) + return stmt.processor.QueryContext(stmt.conn.withKeepInCache(ctx), stmt.query, args) default: return nil, fmt.Errorf("unsupported query mode '%s' for execute query on prepared statement", m) } } -func (s *stmt) ExecContext(ctx context.Context, args []driver.NamedValue) (_ driver.Result, finalErr error) { - onDone := trace.DatabaseSQLOnStmtExec(s.trace, &ctx, +func (stmt *stmt) ExecContext(ctx context.Context, args []driver.NamedValue) (_ driver.Result, finalErr error) { + onDone := trace.DatabaseSQLOnStmtExec(stmt.trace, &ctx, stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*stmt).ExecContext"), - s.stmtCtx, s.query, + stmt.ctx, stmt.query, ) defer func() { onDone(finalErr) }() - if !s.conn.isReady() { + if !stmt.conn.isReady() { return nil, badconn.Map(xerrors.WithStackTrace(errNotReadyConn)) } - switch m := queryModeFromContext(ctx, s.conn.defaultQueryMode); m { + switch m := queryModeFromContext(ctx, stmt.conn.defaultQueryMode); m { case DataQueryMode: - return s.processor.ExecContext(s.conn.withKeepInCache(ctx), s.query, args) + return stmt.processor.ExecContext(stmt.conn.withKeepInCache(ctx), stmt.query, args) default: return nil, fmt.Errorf("unsupported query mode '%s' for execute query on prepared statement", m) } } -func (s *stmt) NumInput() int { +func (stmt *stmt) NumInput() int { return -1 } -func (s *stmt) Close() (finalErr error) { - onDone := trace.DatabaseSQLOnStmtClose(s.trace, &s.stmtCtx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*stmt).Close"), +func (stmt *stmt) Close() (finalErr error) { + var ( + ctx = stmt.ctx + onDone = trace.DatabaseSQLOnStmtClose(stmt.trace, &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*stmt).Close"), + ) ) defer func() { onDone(finalErr) @@ -82,10 +85,10 @@ func (s *stmt) Close() (finalErr error) { return nil } -func (s *stmt) Exec([]driver.Value) (driver.Result, error) { +func (stmt *stmt) Exec([]driver.Value) (driver.Result, error) { return nil, errDeprecated } -func (s *stmt) Query([]driver.Value) (driver.Rows, error) { +func (stmt *stmt) Query([]driver.Value) (driver.Rows, error) { return nil, errDeprecated } diff --git a/internal/xsql/tx.go b/internal/xsql/tx.go index d67813437..2a58e2728 100644 --- a/internal/xsql/tx.go +++ b/internal/xsql/tx.go @@ -14,9 +14,9 @@ import ( ) type tx struct { - conn *conn - txCtx context.Context - tx table.Transaction + conn *conn + ctx context.Context //nolint:containedctx + tx table.Transaction } var ( @@ -45,9 +45,9 @@ func (c *conn) beginTx(ctx context.Context, txOptions driver.TxOptions) (current return nil, badconn.Map(xerrors.WithStackTrace(err)) } c.currentTx = &tx{ - conn: c, - txCtx: ctx, - tx: transaction, + conn: c, + ctx: ctx, + tx: transaction, } return c.currentTx, nil @@ -73,9 +73,12 @@ func (tx *tx) checkTxState() error { } func (tx *tx) Commit() (finalErr error) { - onDone := trace.DatabaseSQLOnTxCommit(tx.conn.trace, &tx.txCtx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*tx).Commit"), - tx, + var ( + ctx = tx.ctx + onDone = trace.DatabaseSQLOnTxCommit(tx.conn.trace, &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*tx).Commit"), + tx, + ) ) defer func() { onDone(finalErr) @@ -86,7 +89,7 @@ func (tx *tx) Commit() (finalErr error) { defer func() { tx.conn.currentTx = nil }() - _, err := tx.tx.CommitTx(tx.txCtx) + _, err := tx.tx.CommitTx(tx.ctx) if err != nil { return badconn.Map(xerrors.WithStackTrace(err)) } @@ -95,9 +98,12 @@ func (tx *tx) Commit() (finalErr error) { } func (tx *tx) Rollback() (finalErr error) { - onDone := trace.DatabaseSQLOnTxRollback(tx.conn.trace, &tx.txCtx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*tx).Rollback"), - tx, + var ( + ctx = tx.ctx + onDone = trace.DatabaseSQLOnTxRollback(tx.conn.trace, &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*tx).Rollback"), + tx, + ) ) defer func() { onDone(finalErr) @@ -108,7 +114,7 @@ func (tx *tx) Rollback() (finalErr error) { defer func() { tx.conn.currentTx = nil }() - err := tx.tx.Rollback(tx.txCtx) + err := tx.tx.Rollback(tx.ctx) if err != nil { return badconn.Map(xerrors.WithStackTrace(err)) } @@ -121,7 +127,7 @@ func (tx *tx) QueryContext(ctx context.Context, query string, args []driver.Name ) { onDone := trace.DatabaseSQLOnTxQuery(tx.conn.trace, &ctx, stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*tx).QueryContext"), - tx.txCtx, tx, query, + tx.ctx, tx, query, ) defer func() { onDone(finalErr) @@ -163,7 +169,7 @@ func (tx *tx) ExecContext(ctx context.Context, query string, args []driver.Named ) { onDone := trace.DatabaseSQLOnTxExec(tx.conn.trace, &ctx, stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*tx).ExecContext"), - tx.txCtx, tx, query, + tx.ctx, tx, query, ) defer func() { onDone(finalErr) @@ -197,7 +203,7 @@ func (tx *tx) ExecContext(ctx context.Context, query string, args []driver.Named func (tx *tx) PrepareContext(ctx context.Context, query string) (_ driver.Stmt, finalErr error) { onDone := trace.DatabaseSQLOnTxPrepare(tx.conn.trace, &ctx, stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*tx).PrepareContext"), - &tx.txCtx, tx, query, + tx.ctx, tx, query, ) defer func() { onDone(finalErr) @@ -209,7 +215,7 @@ func (tx *tx) PrepareContext(ctx context.Context, query string) (_ driver.Stmt, return &stmt{ conn: tx.conn, processor: tx, - stmtCtx: ctx, + ctx: ctx, query: query, trace: tx.conn.trace, }, nil diff --git a/internal/xsql/tx_fake.go b/internal/xsql/tx_fake.go index 459aba718..0fc3efd27 100644 --- a/internal/xsql/tx_fake.go +++ b/internal/xsql/tx_fake.go @@ -12,15 +12,15 @@ import ( ) type txFake struct { - beginCtx context.Context + beginCtx context.Context //nolint:containedctx conn *conn - ctx context.Context + ctx context.Context //nolint:containedctx } func (tx *txFake) PrepareContext(ctx context.Context, query string) (_ driver.Stmt, finalErr error) { onDone := trace.DatabaseSQLOnTxPrepare(tx.conn.trace, &ctx, stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*txFake).PrepareContext"), - &tx.beginCtx, tx, query, + tx.beginCtx, tx, query, ) defer func() { onDone(finalErr) @@ -32,7 +32,7 @@ func (tx *txFake) PrepareContext(ctx context.Context, query string) (_ driver.St return &stmt{ conn: tx.conn, processor: tx, - stmtCtx: ctx, + ctx: ctx, query: query, trace: tx.conn.trace, }, nil @@ -57,9 +57,12 @@ func (tx *txFake) ID() string { } func (tx *txFake) Commit() (err error) { - onDone := trace.DatabaseSQLOnTxCommit(tx.conn.trace, &tx.ctx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*txFake).Commit"), - tx, + var ( + ctx = tx.ctx + onDone = trace.DatabaseSQLOnTxCommit(tx.conn.trace, &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*txFake).Commit"), + tx, + ) ) defer func() { onDone(err) @@ -75,9 +78,12 @@ func (tx *txFake) Commit() (err error) { } func (tx *txFake) Rollback() (err error) { - onDone := trace.DatabaseSQLOnTxRollback(tx.conn.trace, &tx.ctx, - stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*txFake).Rollback"), - tx, + var ( + ctx = tx.ctx + onDone = trace.DatabaseSQLOnTxRollback(tx.conn.trace, &ctx, + stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/xsql.(*txFake).Rollback"), + tx, + ) ) defer func() { onDone(err) diff --git a/log/context_test.go b/log/context_test.go index 92882b887..0f71d9b38 100644 --- a/log/context_test.go +++ b/log/context_test.go @@ -13,7 +13,7 @@ import ( func TestLevelFromContext(t *testing.T) { for _, tt := range []struct { - ctx context.Context + ctx context.Context //nolint:containedctx lvl Level }{ { @@ -37,7 +37,7 @@ func TestLevelFromContext(t *testing.T) { func TestNamesFromContext(t *testing.T) { for _, tt := range []struct { - ctx context.Context + ctx context.Context //nolint:containedctx names []string }{ { diff --git a/trace/sql.go b/trace/sql.go index 9ee597a39..f18e53826 100644 --- a/trace/sql.go +++ b/trace/sql.go @@ -101,7 +101,7 @@ type ( // Safe replacement of context are provided only inside callback function Context *context.Context Call call - TxContext *context.Context + TxContext context.Context //nolint:containedctx Tx tableTransactionInfo Query string } @@ -193,7 +193,7 @@ type ( // Safe replacement of context are provided only inside callback function Context *context.Context Call call - TxContext context.Context + TxContext context.Context //nolint:containedctx Tx tableTransactionInfo Query string } @@ -209,7 +209,7 @@ type ( // Safe replacement of context are provided only inside callback function Context *context.Context Call call - TxContext context.Context + TxContext context.Context //nolint:containedctx Tx tableTransactionInfo Query string } @@ -262,7 +262,7 @@ type ( // Safe replacement of context are provided only inside callback function Context *context.Context Call call - StmtContext context.Context + StmtContext context.Context //nolint:containedctx Query string } // Internals: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#internals @@ -277,7 +277,7 @@ type ( // Safe replacement of context are provided only inside callback function Context *context.Context Call call - StmtContext context.Context + StmtContext context.Context //nolint:containedctx Query string } // Internals: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#internals diff --git a/trace/sql_gtrace.go b/trace/sql_gtrace.go index c4a686e1d..1b7fd0867 100644 --- a/trace/sql_gtrace.go +++ b/trace/sql_gtrace.go @@ -1054,7 +1054,7 @@ func DatabaseSQLOnTxExec(t *DatabaseSQL, c *context.Context, call call, txContex } } // Internals: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#internals -func DatabaseSQLOnTxPrepare(t *DatabaseSQL, c *context.Context, call call, txContext *context.Context, tx tableTransactionInfo, query string) func(error) { +func DatabaseSQLOnTxPrepare(t *DatabaseSQL, c *context.Context, call call, txContext context.Context, tx tableTransactionInfo, query string) func(error) { var p DatabaseSQLTxPrepareStartInfo p.Context = c p.Call = call diff --git a/trace/topic.go b/trace/topic.go index 53077a3b8..ad20bc980 100644 --- a/trace/topic.go +++ b/trace/topic.go @@ -96,7 +96,7 @@ type ( // Internals: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#internals TopicReaderPartitionReadStartResponseStartInfo struct { ReaderConnectionID string - PartitionContext context.Context + PartitionContext *context.Context Topic string PartitionID int64 PartitionSessionID int64 @@ -118,7 +118,7 @@ type ( // Internals: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#internals TopicReaderPartitionReadStopResponseStartInfo struct { ReaderConnectionID string - PartitionContext context.Context + PartitionContext *context.Context Topic string PartitionID int64 PartitionSessionID int64 @@ -197,7 +197,7 @@ type ( // Internals: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#internals TopicReaderReadMessagesStartInfo struct { - RequestContext context.Context + RequestContext *context.Context MinCount int MaxCount int FreeBufferCapacity int @@ -239,7 +239,7 @@ type ( // Internals: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#internals TopicReaderCommitStartInfo struct { - RequestContext context.Context + RequestContext *context.Context Topic string PartitionID int64 PartitionSessionID int64 diff --git a/trace/topic_gtrace.go b/trace/topic_gtrace.go index 5f9dcde80..2f3d4a6b4 100644 --- a/trace/topic_gtrace.go +++ b/trace/topic_gtrace.go @@ -1021,7 +1021,7 @@ func TopicOnReaderReconnectRequest(t *Topic, reason error, wasSent bool) { t.onReaderReconnectRequest(p) } // Internals: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#internals -func TopicOnReaderPartitionReadStartResponse(t *Topic, readerConnectionID string, partitionContext context.Context, topic string, partitionID int64, partitionSessionID int64) func(readOffset *int64, commitOffset *int64, _ error) { +func TopicOnReaderPartitionReadStartResponse(t *Topic, readerConnectionID string, partitionContext *context.Context, topic string, partitionID int64, partitionSessionID int64) func(readOffset *int64, commitOffset *int64, _ error) { var p TopicReaderPartitionReadStartResponseStartInfo p.ReaderConnectionID = readerConnectionID p.PartitionContext = partitionContext @@ -1038,7 +1038,7 @@ func TopicOnReaderPartitionReadStartResponse(t *Topic, readerConnectionID string } } // Internals: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#internals -func TopicOnReaderPartitionReadStopResponse(t *Topic, readerConnectionID string, partitionContext context.Context, topic string, partitionID int64, partitionSessionID int64, committedOffset int64, graceful bool) func(error) { +func TopicOnReaderPartitionReadStopResponse(t *Topic, readerConnectionID string, partitionContext *context.Context, topic string, partitionID int64, partitionSessionID int64, committedOffset int64, graceful bool) func(error) { var p TopicReaderPartitionReadStopResponseStartInfo p.ReaderConnectionID = readerConnectionID p.PartitionContext = partitionContext @@ -1055,7 +1055,7 @@ func TopicOnReaderPartitionReadStopResponse(t *Topic, readerConnectionID string, } } // Internals: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#internals -func TopicOnReaderCommit(t *Topic, requestContext context.Context, topic string, partitionID int64, partitionSessionID int64, startOffset int64, endOffset int64) func(error) { +func TopicOnReaderCommit(t *Topic, requestContext *context.Context, topic string, partitionID int64, partitionSessionID int64, startOffset int64, endOffset int64) func(error) { var p TopicReaderCommitStartInfo p.RequestContext = requestContext p.Topic = topic @@ -1162,7 +1162,7 @@ func TopicOnReaderReceiveDataResponse(t *Topic, readerConnectionID string, local } } // Internals: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#internals -func TopicOnReaderReadMessages(t *Topic, requestContext context.Context, minCount int, maxCount int, freeBufferCapacity int) func(messagesCount int, topic string, partitionID int64, partitionSessionID int64, offsetStart int64, offsetEnd int64, freeBufferCapacity int, _ error) { +func TopicOnReaderReadMessages(t *Topic, requestContext *context.Context, minCount int, maxCount int, freeBufferCapacity int) func(messagesCount int, topic string, partitionID int64, partitionSessionID int64, offsetStart int64, offsetEnd int64, freeBufferCapacity int, _ error) { var p TopicReaderReadMessagesStartInfo p.RequestContext = requestContext p.MinCount = minCount From 35ed58edda892b9983d1a470c39b38d8625e7562 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Sat, 6 Apr 2024 00:01:23 +0300 Subject: [PATCH 12/33] added quoter implementations --- retry/quoter.go | 73 ++++++++++++++++++++++++++++++++++++++++++++ retry/quoter_test.go | 52 +++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 retry/quoter.go create mode 100644 retry/quoter_test.go diff --git a/retry/quoter.go b/retry/quoter.go new file mode 100644 index 000000000..dd186b89d --- /dev/null +++ b/retry/quoter.go @@ -0,0 +1,73 @@ +package retry + +import ( + "context" + "time" + + "github.com/jonboulle/clockwork" +) + +type ( + Limiter interface { + Acquire(ctx context.Context) error + } + rateLimiter struct { + clock clockwork.Clock + ticker clockwork.Ticker + quota chan struct{} + } + rateLimiterOption func(q *rateLimiter) + unlimitedLimiter struct{} +) + +func (unlimitedLimiter) Acquire(ctx context.Context) error { + select { + case <-ctx.Done(): + return ctx.Err() + default: + return nil + } +} + +var ( + _ Limiter = (*rateLimiter)(nil) + _ Limiter = (*unlimitedLimiter)(nil) +) + +func Quoter(rate uint, opts ...rateLimiterOption) *rateLimiter { + q := &rateLimiter{ + clock: clockwork.NewRealClock(), + quota: make(chan struct{}, rate), + } + for range make([]struct{}, rate) { + q.quota <- struct{}{} + } + for _, opt := range opts { + opt(q) + } + q.ticker = q.clock.NewTicker(time.Second / time.Duration(rate)) + go func() { + defer close(q.quota) + for range q.ticker.Chan() { + select { + case q.quota <- struct{}{}: + default: + } + } + }() + + return q +} + +func (q *rateLimiter) Stop() { + q.ticker.Stop() +} + +func (q *rateLimiter) Acquire(ctx context.Context) error { + select { + case <-ctx.Done(): + return ctx.Err() + case <-q.quota: + return nil + } +} diff --git a/retry/quoter_test.go b/retry/quoter_test.go new file mode 100644 index 000000000..cb9f5f23b --- /dev/null +++ b/retry/quoter_test.go @@ -0,0 +1,52 @@ +package retry + +import ( + "context" + "testing" + "time" + + "github.com/jonboulle/clockwork" + "github.com/stretchr/testify/require" + + "github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/xtest" +) + +func TestUnlimitedLimiter(t *testing.T) { + ctx, cancel := xcontext.WithCancel(xtest.Context(t)) + q := unlimitedLimiter{} + require.NoError(t, q.Acquire(ctx)) + cancel() + require.ErrorIs(t, q.Acquire(ctx), context.Canceled) +} + +func TestQuoter(t *testing.T) { + xtest.TestManyTimes(t, func(t testing.TB) { + ctx, cancel := xcontext.WithCancel(xtest.Context(t)) + clock := clockwork.NewFakeClock() + q := Quoter(1, func(q *rateLimiter) { + q.clock = clock + }) + require.NoError(t, q.Acquire(ctx)) + acquireCh := make(chan struct{}) + go func() { + err := q.Acquire(ctx) + acquireCh <- struct{}{} + require.NoError(t, err) + }() + timeCh := make(chan struct{}) + go func() { + clock.Advance(time.Second - time.Nanosecond) + timeCh <- struct{}{} + clock.Advance(time.Nanosecond) + }() + select { + case <-acquireCh: + require.Fail(t, "") + case <-timeCh: + } + <-acquireCh + cancel() + require.ErrorIs(t, q.Acquire(ctx), context.Canceled) + }) +} From 6b6fca4f6693b7d8742981787fc30584e3641cfd Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Sat, 6 Apr 2024 10:14:29 +0300 Subject: [PATCH 13/33] embed limiter in retryers --- internal/backoff/delay.go | 43 ++++++++++ internal/backoff/delay_test.go | 92 +++++++++++++++++++++ internal/query/options/retry.go | 13 +++ internal/wait/wait.go | 52 ------------ internal/xerrors/join.go | 12 +-- internal/xerrors/join_test.go | 2 +- query/client.go | 5 ++ retry/quoter.go | 70 ++++++++-------- retry/quoter_test.go | 12 +-- retry/retry.go | 66 +++++++++++---- retry/retry_test.go | 15 ++++ table/table.go | 4 +- tests/integration/retry_limiter_test.go | 103 ++++++++++++++++++++++++ 13 files changed, 375 insertions(+), 114 deletions(-) create mode 100644 internal/backoff/delay.go create mode 100644 internal/backoff/delay_test.go delete mode 100644 internal/wait/wait.go create mode 100644 tests/integration/retry_limiter_test.go diff --git a/internal/backoff/delay.go b/internal/backoff/delay.go new file mode 100644 index 000000000..e0d89d46e --- /dev/null +++ b/internal/backoff/delay.go @@ -0,0 +1,43 @@ +package backoff + +import ( + "time" +) + +type ( + delayOptions struct { + fast Backoff + slow Backoff + } + delayOption func(o *delayOptions) +) + +func WithFastBackof(fast Backoff) delayOption { + return func(o *delayOptions) { + o.fast = fast + } +} + +func WithSlowBackof(slow Backoff) delayOption { + return func(o *delayOptions) { + o.slow = slow + } +} + +func Delay(t Type, i int, opts ...delayOption) time.Duration { + optionsHolder := delayOptions{ + fast: Fast, + slow: Slow, + } + for _, opt := range opts { + opt(&optionsHolder) + } + switch t { + case TypeFast: + return optionsHolder.fast.Delay(i) + case TypeSlow: + return optionsHolder.slow.Delay(i) + default: + return 0 + } +} diff --git a/internal/backoff/delay_test.go b/internal/backoff/delay_test.go new file mode 100644 index 000000000..1a238aa99 --- /dev/null +++ b/internal/backoff/delay_test.go @@ -0,0 +1,92 @@ +package backoff + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/ydb-platform/ydb-go-sdk/v3/internal/xtest" +) + +func TestDelay(t *testing.T) { + for _, tt := range []struct { + name string + act time.Duration + exp time.Duration + }{ + { + name: xtest.CurrentFileLine(), + act: Delay(TypeNoBackoff, 0), + exp: 0, + }, + { + name: xtest.CurrentFileLine(), + act: Delay(TypeNoBackoff, 1), + exp: 0, + }, + { + name: xtest.CurrentFileLine(), + act: Delay(TypeNoBackoff, 2), + exp: 0, + }, + { + name: xtest.CurrentFileLine(), + act: Delay(TypeFast, 0, WithFastBackof(New( + WithSlotDuration(fastSlot), + WithCeiling(6), + WithJitterLimit(1), + ))), + exp: 5 * time.Millisecond, + }, + { + name: xtest.CurrentFileLine(), + act: Delay(TypeFast, 1, WithFastBackof(New( + WithSlotDuration(fastSlot), + WithCeiling(6), + WithJitterLimit(1), + ))), + exp: 10 * time.Millisecond, + }, + { + name: xtest.CurrentFileLine(), + act: Delay(TypeFast, 3, WithFastBackof(New( + WithSlotDuration(fastSlot), + WithCeiling(6), + WithJitterLimit(1), + ))), + exp: 40 * time.Millisecond, + }, + { + name: xtest.CurrentFileLine(), + act: Delay(TypeSlow, 0, WithSlowBackof(New( + WithSlotDuration(slowSlot), + WithCeiling(6), + WithJitterLimit(1), + ))), + exp: time.Second, + }, + { + name: xtest.CurrentFileLine(), + act: Delay(TypeSlow, 1, WithSlowBackof(New( + WithSlotDuration(slowSlot), + WithCeiling(6), + WithJitterLimit(1), + ))), + exp: 2 * time.Second, + }, + { + name: xtest.CurrentFileLine(), + act: Delay(TypeSlow, 3, WithSlowBackof(New( + WithSlotDuration(slowSlot), + WithCeiling(6), + WithJitterLimit(1), + ))), + exp: 8 * time.Second, + }, + } { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.exp, tt.act) + }) + } +} diff --git a/internal/query/options/retry.go b/internal/query/options/retry.go index b604152e3..a2c643842 100644 --- a/internal/query/options/retry.go +++ b/internal/query/options/retry.go @@ -44,6 +44,7 @@ type ( doTxSettingsOption struct { txSettings tx.Settings } + retryOptionsOption []retry.Option ) func (s *doSettings) Trace() *trace.Query { @@ -106,6 +107,18 @@ func WithTrace(t *trace.Query) traceOption { return traceOption{t: t} } +func (opts retryOptionsOption) applyDoOption(s *doSettings) { + s.retryOpts = append(s.retryOpts, opts...) +} + +func (opts retryOptionsOption) applyDoTxOption(s *doTxSettings) { + s.doOpts = append(s.doOpts, opts) +} + +func WithRetryOptions(retryOptions ...retry.Option) retryOptionsOption { + return retryOptions +} + func ParseDoOpts(t *trace.Query, opts ...DoOption) (s *doSettings) { s = &doSettings{ trace: t, diff --git a/internal/wait/wait.go b/internal/wait/wait.go deleted file mode 100644 index 11a01dee6..000000000 --- a/internal/wait/wait.go +++ /dev/null @@ -1,52 +0,0 @@ -package wait - -import ( - "context" - "time" - - "github.com/ydb-platform/ydb-go-sdk/v3/internal/backoff" - "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" -) - -// waitBackoff is a helper function that waits for i-th Backoff b or ctx -// expiration. -// It returns non-nil error if and only if deadline expiration branch wins. -func waitBackoff(ctx context.Context, b backoff.Backoff, i int) error { - t := time.NewTimer(b.Delay(i)) - defer t.Stop() - - select { - case <-t.C: - return nil - case <-ctx.Done(): - if err := ctx.Err(); err != nil { - return xerrors.WithStackTrace(err) - } - - return nil - } -} - -func Wait(ctx context.Context, fastBackoff, slowBackoff backoff.Backoff, t backoff.Type, i int) error { - var b backoff.Backoff - switch t { - case backoff.TypeNoBackoff: - if err := ctx.Err(); err != nil { - return xerrors.WithStackTrace(err) - } - - return nil - case backoff.TypeFast: - if fastBackoff == nil { - fastBackoff = backoff.Fast - } - b = fastBackoff - case backoff.TypeSlow: - if slowBackoff == nil { - slowBackoff = backoff.Slow - } - b = slowBackoff - } - - return waitBackoff(ctx, b, i) -} diff --git a/internal/xerrors/join.go b/internal/xerrors/join.go index 80ec421c1..a1e33a054 100644 --- a/internal/xerrors/join.go +++ b/internal/xerrors/join.go @@ -6,8 +6,8 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/xstring" ) -func Join(errs ...error) joinError { - return joinError{ +func Join(errs ...error) *joinError { + return &joinError{ errs: errs, } } @@ -16,7 +16,7 @@ type joinError struct { errs []error } -func (e joinError) Error() string { +func (e *joinError) Error() string { b := xstring.Buffer() defer b.Free() b.WriteByte('[') @@ -31,7 +31,7 @@ func (e joinError) Error() string { return b.String() } -func (e joinError) As(target interface{}) bool { +func (e *joinError) As(target interface{}) bool { for _, err := range e.errs { if As(err, target) { return true @@ -41,7 +41,7 @@ func (e joinError) As(target interface{}) bool { return false } -func (e joinError) Is(target error) bool { +func (e *joinError) Is(target error) bool { for _, err := range e.errs { if Is(err, target) { return true @@ -51,6 +51,6 @@ func (e joinError) Is(target error) bool { return false } -func (e joinError) Unwrap() []error { +func (e *joinError) Unwrap() []error { return e.errs } diff --git a/internal/xerrors/join_test.go b/internal/xerrors/join_test.go index a9ff7e297..9af1d4dfd 100644 --- a/internal/xerrors/join_test.go +++ b/internal/xerrors/join_test.go @@ -10,7 +10,7 @@ import ( func TestJoin(t *testing.T) { for _, tt := range []struct { - err joinError + err *joinError iss []error ass []interface{} s string diff --git a/query/client.go b/query/client.go index 87fedc52b..71f8e2f3e 100644 --- a/query/client.go +++ b/query/client.go @@ -5,6 +5,7 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/closer" "github.com/ydb-platform/ydb-go-sdk/v3/internal/query/options" + "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -66,3 +67,7 @@ func WithTrace(t *trace.Query) bothDoAndDoTxOption { func WithLabel(lbl string) bothDoAndDoTxOption { return options.WithLabel(lbl) } + +func WithRetryOptions(retryOptions ...retry.Option) bothDoAndDoTxOption { + return options.WithRetryOptions(retryOptions...) +} diff --git a/retry/quoter.go b/retry/quoter.go index dd186b89d..ad297936b 100644 --- a/retry/quoter.go +++ b/retry/quoter.go @@ -2,9 +2,18 @@ package retry import ( "context" + "errors" "time" "github.com/jonboulle/clockwork" + + "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" +) + +var ( + ErrNoQuota = xerrors.Wrap(errors.New("no retry quota")) + + _ Limiter = (*rateLimiter)(nil) ) type ( @@ -17,57 +26,54 @@ type ( quota chan struct{} } rateLimiterOption func(q *rateLimiter) - unlimitedLimiter struct{} -) - -func (unlimitedLimiter) Acquire(ctx context.Context) error { - select { - case <-ctx.Done(): - return ctx.Err() - default: - return nil - } -} - -var ( - _ Limiter = (*rateLimiter)(nil) - _ Limiter = (*unlimitedLimiter)(nil) ) -func Quoter(rate uint, opts ...rateLimiterOption) *rateLimiter { +func Quoter(attemptsPerSecond int, opts ...rateLimiterOption) *rateLimiter { q := &rateLimiter{ clock: clockwork.NewRealClock(), - quota: make(chan struct{}, rate), - } - for range make([]struct{}, rate) { - q.quota <- struct{}{} } for _, opt := range opts { opt(q) } - q.ticker = q.clock.NewTicker(time.Second / time.Duration(rate)) - go func() { - defer close(q.quota) - for range q.ticker.Chan() { - select { - case q.quota <- struct{}{}: - default: - } + if attemptsPerSecond <= 0 { + q.quota = make(chan struct{}) + close(q.quota) + } else { + q.quota = make(chan struct{}, attemptsPerSecond) + for range make([]struct{}, attemptsPerSecond) { + q.quota <- struct{}{} } - }() + q.ticker = q.clock.NewTicker(time.Second / time.Duration(attemptsPerSecond)) + go func() { + defer close(q.quota) + for range q.ticker.Chan() { + select { + case q.quota <- struct{}{}: + default: + } + } + }() + } return q } func (q *rateLimiter) Stop() { - q.ticker.Stop() + if q.ticker != nil { + q.ticker.Stop() + } } func (q *rateLimiter) Acquire(ctx context.Context) error { select { case <-ctx.Done(): return ctx.Err() - case <-q.quota: - return nil + default: + select { + case <-q.quota: + return nil + case <-ctx.Done(): + return xerrors.WithStackTrace(ctx.Err()) + } } } diff --git a/retry/quoter_test.go b/retry/quoter_test.go index cb9f5f23b..a25aaf9f6 100644 --- a/retry/quoter_test.go +++ b/retry/quoter_test.go @@ -13,11 +13,13 @@ import ( ) func TestUnlimitedLimiter(t *testing.T) { - ctx, cancel := xcontext.WithCancel(xtest.Context(t)) - q := unlimitedLimiter{} - require.NoError(t, q.Acquire(ctx)) - cancel() - require.ErrorIs(t, q.Acquire(ctx), context.Canceled) + xtest.TestManyTimes(t, func(t testing.TB) { + ctx, cancel := xcontext.WithCancel(xtest.Context(t)) + q := Quoter(-1) + require.NoError(t, q.Acquire(ctx)) + cancel() + require.ErrorIs(t, q.Acquire(ctx), context.Canceled) + }) } func TestQuoter(t *testing.T) { diff --git a/retry/retry.go b/retry/retry.go index 65e88ce91..228a84083 100644 --- a/retry/retry.go +++ b/retry/retry.go @@ -3,10 +3,10 @@ package retry import ( "context" "fmt" + "time" "github.com/ydb-platform/ydb-go-sdk/v3/internal/backoff" "github.com/ydb-platform/ydb-go-sdk/v3/internal/stack" - "github.com/ydb-platform/ydb-go-sdk/v3/internal/wait" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" "github.com/ydb-platform/ydb-go-sdk/v3/trace" @@ -25,6 +25,7 @@ type retryOptions struct { stackTrace bool fastBackoff backoff.Backoff slowBackoff backoff.Backoff + limiter Limiter panicCallback func(e interface{}) } @@ -124,6 +125,29 @@ func WithTrace(t *trace.Retry) traceOption { return traceOption{t: t} } +var _ Option = limiterOption{} + +type limiterOption struct { + l Limiter +} + +func (l limiterOption) ApplyRetryOption(opts *retryOptions) { + opts.limiter = l.l +} + +func (l limiterOption) ApplyDoOption(opts *doOptions) { + opts.retryOptions = append(opts.retryOptions, WithLimiter(l.l)) +} + +func (l limiterOption) ApplyDoTxOption(opts *doTxOptions) { + opts.retryOptions = append(opts.retryOptions, WithLimiter(l.l)) +} + +// WithLimiter returns limiter option +func WithLimiter(l Limiter) limiterOption { + return limiterOption{l: l} +} + var _ Option = idempotentOption(false) type idempotentOption bool @@ -234,6 +258,7 @@ func Retry(ctx context.Context, op retryOperation, opts ...Option) (finalErr err options := &retryOptions{ call: stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/retry.Retry"), trace: &trace.Retry{}, + limiter: Quoter(-1), fastBackoff: backoff.Fast, slowBackoff: backoff.Slow, } @@ -297,21 +322,14 @@ func Retry(ctx context.Context, op retryOperation, opts ...Option) (finalErr err return nil } - if ctxErr := ctx.Err(); ctxErr != nil { - return xerrors.WithStackTrace( - xerrors.Join( - fmt.Errorf("context error occurred on attempt No.%d", attempts), - ctxErr, err, - ), - ) - } - m := Check(err) if m.StatusCode() != code { i = 0 } + code = m.StatusCode() + if !m.MustRetry(options.idempotent) { return xerrors.WithStackTrace( fmt.Errorf("non-retryable error occurred on attempt No.%d (idempotent=%v): %w", @@ -320,17 +338,33 @@ func Retry(ctx context.Context, op retryOperation, opts ...Option) (finalErr err ) } - if e := wait.Wait(ctx, options.fastBackoff, options.slowBackoff, m.BackoffType(), i); e != nil { + t := time.NewTimer(backoff.Delay(m.BackoffType(), i, + backoff.WithFastBackof(options.fastBackoff), + backoff.WithSlowBackof(options.slowBackoff), + )) + + select { + case <-ctx.Done(): + t.Stop() + return xerrors.WithStackTrace( xerrors.Join( - fmt.Errorf("wait exit on attempt No.%d", - attempts, - ), e, err, + fmt.Errorf("attempt No.%d: %w", attempts, ctx.Err()), + err, ), ) + case <-t.C: + t.Stop() + + if e := options.limiter.Acquire(ctx); e != nil { + return xerrors.WithStackTrace( + xerrors.Join( + fmt.Errorf("attempt No.%d: %w", attempts, ErrNoQuota), + err, + ), + ) + } } - - code = m.StatusCode() } } } diff --git a/retry/retry_test.go b/retry/retry_test.go index 770f2ea65..e603a237a 100644 --- a/retry/retry_test.go +++ b/retry/retry_test.go @@ -2,6 +2,7 @@ package retry import ( "context" + "errors" "fmt" "testing" "time" @@ -13,6 +14,7 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/xtest" ) func TestRetryModes(t *testing.T) { @@ -187,3 +189,16 @@ func TestRetryTransportCancelled(t *testing.T) { }) } } + +func TestRetryWithLimiter(t *testing.T) { + xtest.TestManyTimes(t, func(t testing.TB) { + l := Quoter(10) + defer l.Stop() + ctx, cancel := context.WithTimeout(xtest.Context(t), 5*time.Millisecond) + defer cancel() + err := Retry(ctx, func(ctx context.Context) (err error) { + return RetryableError(errors.New("custom error")) + }, WithLimiter(l)) + require.ErrorIs(t, err, ErrNoQuota) + }) +} diff --git a/table/table.go b/table/table.go index 1cb1ed8e0..920a9d7d0 100644 --- a/table/table.go +++ b/table/table.go @@ -520,10 +520,10 @@ var _ Option = retryOptionsOption{} type retryOptionsOption []retry.Option func (retryOptions retryOptionsOption) ApplyTableOption(opts *Options) { - opts.RetryOptions = append(opts.RetryOptions, retry.WithIdempotent(true)) + opts.RetryOptions = append(opts.RetryOptions, retryOptions...) } -func WithRetryOptions(retryOptions []retry.Option) retryOptionsOption { +func WithRetryOptions(retryOptions ...retry.Option) retryOptionsOption { return retryOptions } diff --git a/tests/integration/retry_limiter_test.go b/tests/integration/retry_limiter_test.go new file mode 100644 index 000000000..2873a8d42 --- /dev/null +++ b/tests/integration/retry_limiter_test.go @@ -0,0 +1,103 @@ +//go:build integration +// +build integration + +package integration + +import ( + "context" + "database/sql" + "errors" + "os" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/ydb-platform/ydb-go-sdk/v3" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/version" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/xtest" + "github.com/ydb-platform/ydb-go-sdk/v3/query" + "github.com/ydb-platform/ydb-go-sdk/v3/retry" + "github.com/ydb-platform/ydb-go-sdk/v3/table" +) + +type noQuota struct{} + +func (n noQuota) Acquire(ctx context.Context) error { + return retry.ErrNoQuota +} + +func TestRetryLimiter(t *testing.T) { + ctx := xtest.Context(t) + + nativeDriver, err := ydb.Open(ctx, os.Getenv("YDB_CONNECTION_STRING"), + ydb.WithDiscoveryInterval(time.Second), + ) + require.NoError(t, err) + + defer func() { + // cleanup + _ = nativeDriver.Close(ctx) + }() + + c, err := ydb.Connector(nativeDriver) + require.NoError(t, err) + + defer func() { + // cleanup + _ = c.Close() + }() + + db := sql.OpenDB(c) + defer func() { + // cleanup + _ = db.Close() + }() + + l := noQuota{} + + t.Run("retry.Retry", func(t *testing.T) { + err := retry.Retry(ctx, func(ctx context.Context) (err error) { + return retry.RetryableError(errors.New("custom error")) + }, retry.WithLimiter(l)) + require.ErrorIs(t, err, retry.ErrNoQuota) + }) + t.Run("retry.Do", func(t *testing.T) { + err := retry.Do(ctx, db, func(ctx context.Context, cc *sql.Conn) (err error) { + return retry.RetryableError(errors.New("custom error")) + }, retry.WithLimiter(l)) + require.ErrorIs(t, err, retry.ErrNoQuota) + }) + t.Run("retry.DoTx", func(t *testing.T) { + err := retry.DoTx(ctx, db, func(ctx context.Context, tx *sql.Tx) (err error) { + return retry.RetryableError(errors.New("custom error")) + }, retry.WithLimiter(l)) + require.ErrorIs(t, err, retry.ErrNoQuota) + }) + t.Run("db.Table().Do", func(t *testing.T) { + err := nativeDriver.Table().Do(ctx, func(ctx context.Context, s table.Session) error { + return retry.RetryableError(errors.New("custom error")) + }, table.WithRetryOptions(retry.WithLimiter(l))) + require.ErrorIs(t, err, retry.ErrNoQuota) + }) + t.Run("db.Table().DoTx", func(t *testing.T) { + err := nativeDriver.Table().DoTx(ctx, func(ctx context.Context, tx table.TransactionActor) error { + return retry.RetryableError(errors.New("custom error")) + }, table.WithRetryOptions(retry.WithLimiter(l))) + require.ErrorIs(t, err, retry.ErrNoQuota) + }) + if version.Gte(os.Getenv("YDB_VERSION"), "24.1") { + t.Run("db.Query().Do", func(t *testing.T) { + err := nativeDriver.Query().Do(ctx, func(ctx context.Context, s query.Session) error { + return retry.RetryableError(errors.New("custom error")) + }, query.WithRetryOptions(retry.WithLimiter(l))) + require.ErrorIs(t, err, retry.ErrNoQuota) + }) + t.Run("db.Query().DoTx", func(t *testing.T) { + err := nativeDriver.Query().DoTx(ctx, func(ctx context.Context, tx query.TxActor) error { + return retry.RetryableError(errors.New("custom error")) + }, query.WithRetryOptions(retry.WithLimiter(l))) + require.ErrorIs(t, err, retry.ErrNoQuota) + }) + } +} From 5e2387235e2b96051a56957e0040f775991da414 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 8 Apr 2024 14:07:29 +0300 Subject: [PATCH 14/33] fixed regeneration of quota --- retry/quoter.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/retry/quoter.go b/retry/quoter.go index ad297936b..16407967f 100644 --- a/retry/quoter.go +++ b/retry/quoter.go @@ -24,6 +24,7 @@ type ( clock clockwork.Clock ticker clockwork.Ticker quota chan struct{} + done chan struct{} } rateLimiterOption func(q *rateLimiter) ) @@ -31,6 +32,7 @@ type ( func Quoter(attemptsPerSecond int, opts ...rateLimiterOption) *rateLimiter { q := &rateLimiter{ clock: clockwork.NewRealClock(), + done: make(chan struct{}), } for _, opt := range opts { opt(q) @@ -46,10 +48,16 @@ func Quoter(attemptsPerSecond int, opts ...rateLimiterOption) *rateLimiter { q.ticker = q.clock.NewTicker(time.Second / time.Duration(attemptsPerSecond)) go func() { defer close(q.quota) - for range q.ticker.Chan() { + for { select { - case q.quota <- struct{}{}: - default: + case <-q.ticker.Chan(): + select { + case q.quota <- struct{}{}: + case <-q.done: + return + } + case <-q.done: + return } } }() @@ -62,6 +70,7 @@ func (q *rateLimiter) Stop() { if q.ticker != nil { q.ticker.Stop() } + close(q.done) } func (q *rateLimiter) Acquire(ctx context.Context) error { From 24f494b5eac0d55d5947b9d7d5d00a3c06217432 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 8 Apr 2024 14:08:53 +0300 Subject: [PATCH 15/33] stop quoter --- retry/quoter_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/retry/quoter_test.go b/retry/quoter_test.go index a25aaf9f6..2318af439 100644 --- a/retry/quoter_test.go +++ b/retry/quoter_test.go @@ -29,6 +29,7 @@ func TestQuoter(t *testing.T) { q := Quoter(1, func(q *rateLimiter) { q.clock = clock }) + defer q.Stop() require.NoError(t, q.Acquire(ctx)) acquireCh := make(chan struct{}) go func() { From 4a7491ee9b1534d53839e14402cedd0e8222b79c Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 8 Apr 2024 14:12:24 +0300 Subject: [PATCH 16/33] fixes --- retry/retry.go | 3 ++- tests/integration/retry_limiter_test.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/retry/retry.go b/retry/retry.go index 228a84083..813b06f38 100644 --- a/retry/retry.go +++ b/retry/retry.go @@ -356,10 +356,11 @@ func Retry(ctx context.Context, op retryOperation, opts ...Option) (finalErr err case <-t.C: t.Stop() - if e := options.limiter.Acquire(ctx); e != nil { + if acquireErr := options.limiter.Acquire(ctx); acquireErr != nil { return xerrors.WithStackTrace( xerrors.Join( fmt.Errorf("attempt No.%d: %w", attempts, ErrNoQuota), + acquireErr, err, ), ) diff --git a/tests/integration/retry_limiter_test.go b/tests/integration/retry_limiter_test.go index 2873a8d42..3e03cba24 100644 --- a/tests/integration/retry_limiter_test.go +++ b/tests/integration/retry_limiter_test.go @@ -24,7 +24,7 @@ import ( type noQuota struct{} func (n noQuota) Acquire(ctx context.Context) error { - return retry.ErrNoQuota + return errors.New("no quota") } func TestRetryLimiter(t *testing.T) { From a940528feb38409f780307bdcbb2c4e94c1d1134 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 8 Apr 2024 16:39:36 +0300 Subject: [PATCH 17/33] global retry limiter --- config/config.go | 7 +++ internal/balancer/balancer.go | 1 + internal/config/config.go | 16 ++++++ internal/coordination/client.go | 56 ++++++++++++++------ internal/ratelimiter/client.go | 6 +++ internal/scheme/client.go | 9 +++- internal/scheme/config/config.go | 10 ++-- internal/scripting/client.go | 3 ++ internal/table/retry.go | 1 + internal/topic/topicclientinternal/client.go | 4 ++ internal/xsql/conn.go | 45 ++++++++++------ internal/xsql/connector.go | 24 ++++++++- options.go | 10 ++++ retry/sql.go | 10 ++-- sql.go | 1 + tests/integration/retry_limiter_test.go | 20 ++++--- 16 files changed, 170 insertions(+), 53 deletions(-) diff --git a/config/config.go b/config/config.go index 28ffd0f90..29d8d434d 100644 --- a/config/config.go +++ b/config/config.go @@ -12,6 +12,7 @@ import ( balancerConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer/config" "github.com/ydb-platform/ydb-go-sdk/v3/internal/config" "github.com/ydb-platform/ydb-go-sdk/v3/internal/meta" + "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -157,6 +158,12 @@ func WithTrace(t trace.Driver, opts ...trace.DriverComposeOption) Option { //nol } } +func WithRetryLimiter(l retry.Limiter) Option { + return func(c *Config) { + config.SetRetryLimiter(&c.Common, l) + } +} + func WithTraceRetry(t *trace.Retry, opts ...trace.RetryComposeOption) Option { return func(c *Config) { config.SetTraceRetry(&c.Common, t, opts...) diff --git a/internal/balancer/balancer.go b/internal/balancer/balancer.go index f69ec11e2..147b7bd3d 100644 --- a/internal/balancer/balancer.go +++ b/internal/balancer/balancer.go @@ -89,6 +89,7 @@ func (b *Balancer) clusterDiscovery(ctx context.Context) (err error) { }, retry.WithIdempotent(true), retry.WithTrace(b.driverConfig.TraceRetry()), + retry.WithLimiter(b.driverConfig.RetryLimiter()), ) } diff --git a/internal/config/config.go b/internal/config/config.go index dcf89c4b7..d18e980ab 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,14 +3,18 @@ package config import ( "time" + "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) +var defaultRetryLimiter = retry.Quoter(-1) + type Common struct { operationTimeout time.Duration operationCancelAfter time.Duration disableAutoRetry bool traceRetry trace.Retry + retryLimiter retry.Limiter panicCallback func(e interface{}) } @@ -48,6 +52,14 @@ func (c *Common) TraceRetry() *trace.Retry { return &c.traceRetry } +func (c *Common) RetryLimiter() retry.Limiter { + if c.retryLimiter == nil { + return defaultRetryLimiter + } + + return c.retryLimiter +} + // SetOperationTimeout define the maximum amount of time a YDB server will process // an operation. After timeout exceeds YDB will try to cancel operation and // regardless of the cancellation appropriate error will be returned to @@ -81,3 +93,7 @@ func SetAutoRetry(c *Common, autoRetry bool) { func SetTraceRetry(c *Common, t *trace.Retry, opts ...trace.RetryComposeOption) { c.traceRetry = *c.traceRetry.Compose(t, opts...) } + +func SetRetryLimiter(c *Common, l retry.Limiter) { + c.retryLimiter = l +} diff --git a/internal/coordination/client.go b/internal/coordination/client.go index e7c194049..2a3670b4d 100644 --- a/internal/coordination/client.go +++ b/internal/coordination/client.go @@ -110,9 +110,15 @@ func (c *Client) CreateNode(ctx context.Context, path string, config coordinatio return createNode(ctx, c.client, request) } - return retry.Retry(ctx, func(ctx context.Context) error { - return createNode(ctx, c.client, request) - }, retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry())) + return retry.Retry(ctx, + func(ctx context.Context) error { + return createNode(ctx, c.client, request) + }, + retry.WithStackTrace(), + retry.WithIdempotent(true), + retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), + ) } func (c *Client) AlterNode(ctx context.Context, path string, config coordination.NodeConfig) (finalErr error) { @@ -137,9 +143,15 @@ func (c *Client) AlterNode(ctx context.Context, path string, config coordination return xerrors.WithStackTrace(call(ctx)) } - return retry.Retry(ctx, func(ctx context.Context) (err error) { - return alterNode(ctx, c.client, request) - }, retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry())) + return retry.Retry(ctx, + func(ctx context.Context) (err error) { + return alterNode(ctx, c.client, request) + }, + retry.WithStackTrace(), + retry.WithIdempotent(true), + retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), + ) } func alterNodeRequest( @@ -192,9 +204,15 @@ func (c *Client) DropNode(ctx context.Context, path string) (finalErr error) { return xerrors.WithStackTrace(call(ctx)) } - return retry.Retry(ctx, func(ctx context.Context) (err error) { - return dropNode(ctx, c.client, request) - }, retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry())) + return retry.Retry(ctx, + func(ctx context.Context) (err error) { + return dropNode(ctx, c.client, request) + }, + retry.WithStackTrace(), + retry.WithIdempotent(true), + retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), + ) } func dropNodeRequest(path string, operationParams *Ydb_Operations.OperationParams) *Ydb_Coordination.DropNodeRequest { @@ -241,14 +259,20 @@ func (c *Client) DescribeNode( return describeNode(ctx, c.client, request) } - err := retry.Retry(ctx, func(ctx context.Context) (err error) { - entry, config, err = describeNode(ctx, c.client, request) - if err != nil { - return xerrors.WithStackTrace(err) - } + err := retry.Retry(ctx, + func(ctx context.Context) (err error) { + entry, config, err = describeNode(ctx, c.client, request) + if err != nil { + return xerrors.WithStackTrace(err) + } - return nil - }, retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry())) + return nil + }, + retry.WithStackTrace(), + retry.WithIdempotent(true), + retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), + ) if err != nil { return nil, nil, xerrors.WithStackTrace(err) } diff --git a/internal/ratelimiter/client.go b/internal/ratelimiter/client.go index eec07b118..5fc580cd3 100644 --- a/internal/ratelimiter/client.go +++ b/internal/ratelimiter/client.go @@ -63,6 +63,7 @@ func (c *Client) CreateResource( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) } @@ -112,6 +113,7 @@ func (c *Client) AlterResource( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) } @@ -161,6 +163,7 @@ func (c *Client) DropResource( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) } @@ -206,6 +209,7 @@ func (c *Client) ListResource( retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) return list, err @@ -265,6 +269,7 @@ func (c *Client) DescribeResource( retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) return @@ -333,6 +338,7 @@ func (c *Client) AcquireResource( return retry.Retry(ctx, call, retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) } diff --git a/internal/scheme/client.go b/internal/scheme/client.go index 6f46ed736..f2c61dd4f 100644 --- a/internal/scheme/client.go +++ b/internal/scheme/client.go @@ -22,7 +22,7 @@ import ( var errNilClient = xerrors.Wrap(errors.New("scheme client is not initialized")) type Client struct { - config config.Config + config *config.Config service Ydb_Scheme_V1.SchemeServiceClient } @@ -38,7 +38,7 @@ func (c *Client) Close(_ context.Context) error { return nil } -func New(ctx context.Context, cc grpc.ClientConnInterface, config config.Config) *Client { +func New(ctx context.Context, cc grpc.ClientConnInterface, config *config.Config) *Client { return &Client{ config: config, service: Ydb_Scheme_V1.NewSchemeServiceClient(cc), @@ -64,6 +64,7 @@ func (c *Client) MakeDirectory(ctx context.Context, path string) (finalErr error retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) } @@ -103,6 +104,7 @@ func (c *Client) RemoveDirectory(ctx context.Context, path string) (finalErr err retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) } @@ -144,6 +146,7 @@ func (c *Client) ListDirectory(ctx context.Context, path string) (d scheme.Direc retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) return d, xerrors.WithStackTrace(err) @@ -207,6 +210,7 @@ func (c *Client) DescribePath(ctx context.Context, path string) (e scheme.Entry, retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) return e, xerrors.WithStackTrace(err) @@ -268,6 +272,7 @@ func (c *Client) ModifyPermissions( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) } diff --git a/internal/scheme/config/config.go b/internal/scheme/config/config.go index a684fc83e..13865ffba 100644 --- a/internal/scheme/config/config.go +++ b/internal/scheme/config/config.go @@ -14,12 +14,12 @@ type Config struct { } // Trace returns trace over scheme client calls -func (c Config) Trace() *trace.Scheme { +func (c *Config) Trace() *trace.Scheme { return c.trace } // Database returns database name -func (c Config) Database() string { +func (c *Config) Database() string { return c.databaseName } @@ -46,13 +46,13 @@ func With(config config.Common) Option { } } -func New(opts ...Option) Config { - c := Config{ +func New(opts ...Option) *Config { + c := &Config{ trace: &trace.Scheme{}, } for _, opt := range opts { if opt != nil { - opt(&c) + opt(c) } } diff --git a/internal/scripting/client.go b/internal/scripting/client.go index a8ecc18bb..473997ce9 100644 --- a/internal/scripting/client.go +++ b/internal/scripting/client.go @@ -60,6 +60,7 @@ func (c *Client) Execute( err = retry.Retry(ctx, call, retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) return r, xerrors.WithStackTrace(err) @@ -139,6 +140,7 @@ func (c *Client) Explain( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) return e, xerrors.WithStackTrace(err) @@ -213,6 +215,7 @@ func (c *Client) StreamExecute( err = retry.Retry(ctx, call, retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), ) return r, xerrors.WithStackTrace(err) diff --git a/internal/table/retry.go b/internal/table/retry.go index d6577d7ad..f9ca31e4b 100644 --- a/internal/table/retry.go +++ b/internal/table/retry.go @@ -99,6 +99,7 @@ func (c *Client) retryOptions(opts ...table.Option) *table.Options { ), RetryOptions: []retry.Option{ retry.WithTrace(c.config.TraceRetry()), + retry.WithLimiter(c.config.RetryLimiter()), }, } for _, opt := range opts { diff --git a/internal/topic/topicclientinternal/client.go b/internal/topic/topicclientinternal/client.go index 2318d97b1..7d228baf6 100644 --- a/internal/topic/topicclientinternal/client.go +++ b/internal/topic/topicclientinternal/client.go @@ -87,6 +87,7 @@ func (c *Client) Alter(ctx context.Context, path string, opts ...topicoptions.Al return retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), + retry.WithLimiter(c.cfg.RetryLimiter()), ) } @@ -119,6 +120,7 @@ func (c *Client) Create( return retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), + retry.WithLimiter(c.cfg.RetryLimiter()), ) } @@ -156,6 +158,7 @@ func (c *Client) Describe( err = retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), + retry.WithLimiter(c.cfg.RetryLimiter()), ) } else { err = call(ctx) @@ -192,6 +195,7 @@ func (c *Client) Drop(ctx context.Context, path string, opts ...topicoptions.Dro return retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), + retry.WithLimiter(c.cfg.RetryLimiter()), ) } diff --git a/internal/xsql/conn.go b/internal/xsql/conn.go index 71404066a..1fe636a94 100644 --- a/internal/xsql/conn.go +++ b/internal/xsql/conn.go @@ -543,7 +543,7 @@ func (c *conn) IsColumnExists(ctx context.Context, tableName, columnName string) return false, xerrors.WithStackTrace(fmt.Errorf("table '%s' not exist", tableName)) } - err = retry.Retry(ctx, func(ctx context.Context) (err error) { + err = c.retryIdempotent(ctx, func(ctx context.Context) (err error) { desc, err := c.session.DescribeTable(ctx, tableName) if err != nil { return err @@ -557,7 +557,7 @@ func (c *conn) IsColumnExists(ctx context.Context, tableName, columnName string) } return nil - }, retry.WithIdempotent(true)) + }) if err != nil { return false, xerrors.WithStackTrace(err) } @@ -578,7 +578,7 @@ func (c *conn) GetColumns(ctx context.Context, tableName string) (columns []stri return nil, xerrors.WithStackTrace(fmt.Errorf("table '%s' not exist", tableName)) } - err = retry.Retry(ctx, func(ctx context.Context) (err error) { + err = c.retryIdempotent(ctx, func(ctx context.Context) (err error) { desc, err := c.session.DescribeTable(ctx, tableName) if err != nil { return err @@ -588,7 +588,7 @@ func (c *conn) GetColumns(ctx context.Context, tableName string) (columns []stri } return nil - }, retry.WithIdempotent(true)) + }) if err != nil { return nil, xerrors.WithStackTrace(err) } @@ -617,7 +617,7 @@ func (c *conn) GetColumnType(ctx context.Context, tableName, columnName string) return "", xerrors.WithStackTrace(fmt.Errorf("column '%s' not exist in table '%s'", columnName, tableName)) } - err = retry.Retry(ctx, func(ctx context.Context) (err error) { + err = c.retryIdempotent(ctx, func(ctx context.Context) (err error) { desc, err := c.session.DescribeTable(ctx, tableName) if err != nil { return err @@ -631,7 +631,7 @@ func (c *conn) GetColumnType(ctx context.Context, tableName, columnName string) } return nil - }, retry.WithIdempotent(true)) + }) if err != nil { return "", xerrors.WithStackTrace(err) } @@ -652,7 +652,7 @@ func (c *conn) GetPrimaryKeys(ctx context.Context, tableName string) (pkCols []s return nil, xerrors.WithStackTrace(fmt.Errorf("table '%s' not exist", tableName)) } - err = retry.Retry(ctx, func(ctx context.Context) (err error) { + err = c.retryIdempotent(ctx, func(ctx context.Context) (err error) { desc, err := c.session.DescribeTable(ctx, tableName) if err != nil { return err @@ -660,7 +660,7 @@ func (c *conn) GetPrimaryKeys(ctx context.Context, tableName string) (pkCols []s pkCols = append(pkCols, desc.PrimaryKey...) return nil - }, retry.WithIdempotent(true)) + }) if err != nil { return nil, xerrors.WithStackTrace(err) } @@ -729,11 +729,11 @@ func (c *conn) getTables(ctx context.Context, absPath string, recursive, exclude } var d scheme.Directory - err := retry.Retry(ctx, func(ctx context.Context) (err error) { + err := c.retryIdempotent(ctx, func(ctx context.Context) (err error) { d, err = c.connector.parent.Scheme().ListDirectory(ctx, absPath) return err - }, retry.WithIdempotent(true)) + }) if err != nil { return nil, xerrors.WithStackTrace(err) } @@ -766,11 +766,11 @@ func (c *conn) GetTables(ctx context.Context, folder string, recursive, excludeS absPath := c.normalizePath(folder) var e scheme.Entry - err := retry.Retry(ctx, func(ctx context.Context) (err error) { + err := c.retryIdempotent(ctx, func(ctx context.Context) (err error) { e, err = c.connector.parent.Scheme().DescribePath(ctx, absPath) return err - }, retry.WithIdempotent(true)) + }) if err != nil { return nil, xerrors.WithStackTrace(err) } @@ -809,7 +809,7 @@ func (c *conn) GetIndexes(ctx context.Context, tableName string) (indexes []stri return nil, xerrors.WithStackTrace(fmt.Errorf("table '%s' not exist", tableName)) } - err = retry.Retry(ctx, func(ctx context.Context) (err error) { + err = c.retryIdempotent(ctx, func(ctx context.Context) (err error) { desc, err := c.session.DescribeTable(ctx, tableName) if err != nil { return err @@ -819,7 +819,7 @@ func (c *conn) GetIndexes(ctx context.Context, tableName string) (indexes []stri } return nil - }, retry.WithIdempotent(true)) + }) if err != nil { return nil, xerrors.WithStackTrace(err) } @@ -827,6 +827,19 @@ func (c *conn) GetIndexes(ctx context.Context, tableName string) (indexes []stri return indexes, nil } +func (c *conn) retryIdempotent(ctx context.Context, f func(ctx context.Context) error) error { + err := retry.Retry(ctx, f, + retry.WithIdempotent(true), + retry.WithTrace(c.connector.traceRetry), + retry.WithLimiter(c.connector.retryLimiter), + ) + if err != nil { + return xerrors.WithStackTrace(err) + } + + return nil +} + func (c *conn) GetIndexColumns(ctx context.Context, tableName, indexName string) (columns []string, _ error) { tableName = c.normalizePath(tableName) tableExists, err := helpers.IsEntryExists(ctx, @@ -840,7 +853,7 @@ func (c *conn) GetIndexColumns(ctx context.Context, tableName, indexName string) return nil, xerrors.WithStackTrace(fmt.Errorf("table '%s' not exist", tableName)) } - err = retry.Retry(ctx, func(ctx context.Context) (err error) { + err = c.retryIdempotent(ctx, func(ctx context.Context) (err error) { desc, err := c.session.DescribeTable(ctx, tableName) if err != nil { return err @@ -854,7 +867,7 @@ func (c *conn) GetIndexColumns(ctx context.Context, tableName, indexName string) } return xerrors.WithStackTrace(fmt.Errorf("index '%s' not found in table '%s'", indexName, tableName)) - }, retry.WithIdempotent(true)) + }) if err != nil { return nil, xerrors.WithStackTrace(err) } diff --git a/internal/xsql/connector.go b/internal/xsql/connector.go index 8009f66d1..abd9e5673 100644 --- a/internal/xsql/connector.go +++ b/internal/xsql/connector.go @@ -15,6 +15,7 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" "github.com/ydb-platform/ydb-go-sdk/v3/meta" + "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/scheme" "github.com/ydb-platform/ydb-go-sdk/v3/scripting" "github.com/ydb-platform/ydb-go-sdk/v3/table" @@ -175,6 +176,20 @@ func WithTraceRetry(t *trace.Retry) ConnectorOption { return traceRetryConnectorOption{t: t} } +type retryLimiterConnectorOption struct { + l retry.Limiter +} + +func (l retryLimiterConnectorOption) Apply(c *Connector) error { + c.retryLimiter = l.l + + return nil +} + +func WithRetryLimiter(l retry.Limiter) ConnectorOption { + return retryLimiterConnectorOption{l: l} +} + type fakeTxConnectorOption QueryMode func (m fakeTxConnectorOption) Apply(c *Connector) error { @@ -248,8 +263,9 @@ type Connector struct { disableServerBalancer bool idleThreshold time.Duration - trace *trace.DatabaseSQL - traceRetry *trace.Retry + trace *trace.DatabaseSQL + traceRetry *trace.Retry + retryLimiter retry.Limiter } var ( @@ -353,6 +369,10 @@ func (d *driverWrapper) TraceRetry() *trace.Retry { return d.c.traceRetry } +func (d *driverWrapper) RetryLimiter() retry.Limiter { + return d.c.retryLimiter +} + func (d *driverWrapper) Open(_ string) (driver.Conn, error) { return nil, ErrUnsupported } diff --git a/options.go b/options.go index 305f700fc..4628bee41 100644 --- a/options.go +++ b/options.go @@ -25,6 +25,7 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xsql" "github.com/ydb-platform/ydb-go-sdk/v3/log" + "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/topic/topicoptions" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -294,6 +295,15 @@ func WithDiscoveryInterval(discoveryInterval time.Duration) Option { } } +// WithRetryLimiter sets retry limiter for all calls of all retryers. +func WithRetryLimiter(l retry.Limiter) Option { + return func(ctx context.Context, c *Driver) error { + c.options = append(c.options, config.WithRetryLimiter(l)) + + return nil + } +} + // WithTraceDriver appends trace.Driver into driver traces func WithTraceDriver(t trace.Driver, opts ...trace.DriverComposeOption) Option { //nolint:gocritic return func(ctx context.Context, c *Driver) error { diff --git a/retry/sql.go b/retry/sql.go index 9b826a1ca..c15ae0290 100644 --- a/retry/sql.go +++ b/retry/sql.go @@ -142,12 +142,14 @@ func DoTx(ctx context.Context, db *sql.DB, op func(context.Context, *sql.Tx) err } attempts = 0 ) - if tracer, has := db.Driver().(interface { + if d, has := db.Driver().(interface { TraceRetry() *trace.Retry + RetryLimiter() Limiter }); has { - options.retryOptions = append(options.retryOptions, nil) - copy(options.retryOptions[1:], options.retryOptions) - options.retryOptions[0] = WithTrace(tracer.TraceRetry()) + options.retryOptions = append(options.retryOptions, nil, nil) + copy(options.retryOptions[2:], options.retryOptions) + options.retryOptions[0] = WithTrace(d.TraceRetry()) + options.retryOptions[1] = WithLimiter(d.RetryLimiter()) } for _, opt := range opts { if opt != nil { diff --git a/sql.go b/sql.go index 0db01dd72..8a5d8ed47 100644 --- a/sql.go +++ b/sql.go @@ -167,6 +167,7 @@ func Connector(parent *Driver, opts ...ConnectorOption) (SQLConnector, error) { ), xsql.WithOnClose(d.detach), xsql.WithTraceRetry(parent.config.TraceRetry()), + xsql.WithRetryLimiter(parent.config.RetryLimiter()), )..., ) if err != nil { diff --git a/tests/integration/retry_limiter_test.go b/tests/integration/retry_limiter_test.go index 3e03cba24..5c367d03b 100644 --- a/tests/integration/retry_limiter_test.go +++ b/tests/integration/retry_limiter_test.go @@ -30,8 +30,12 @@ func (n noQuota) Acquire(ctx context.Context) error { func TestRetryLimiter(t *testing.T) { ctx := xtest.Context(t) + defaultLimiter := retry.Quoter(1) + defer defaultLimiter.Stop() + nativeDriver, err := ydb.Open(ctx, os.Getenv("YDB_CONNECTION_STRING"), ydb.WithDiscoveryInterval(time.Second), + ydb.WithRetryLimiter(defaultLimiter), ) require.NoError(t, err) @@ -54,49 +58,49 @@ func TestRetryLimiter(t *testing.T) { _ = db.Close() }() - l := noQuota{} + retryLimiter := noQuota{} t.Run("retry.Retry", func(t *testing.T) { err := retry.Retry(ctx, func(ctx context.Context) (err error) { return retry.RetryableError(errors.New("custom error")) - }, retry.WithLimiter(l)) + }, retry.WithLimiter(retryLimiter)) require.ErrorIs(t, err, retry.ErrNoQuota) }) t.Run("retry.Do", func(t *testing.T) { err := retry.Do(ctx, db, func(ctx context.Context, cc *sql.Conn) (err error) { return retry.RetryableError(errors.New("custom error")) - }, retry.WithLimiter(l)) + }, retry.WithLimiter(retryLimiter)) require.ErrorIs(t, err, retry.ErrNoQuota) }) t.Run("retry.DoTx", func(t *testing.T) { err := retry.DoTx(ctx, db, func(ctx context.Context, tx *sql.Tx) (err error) { return retry.RetryableError(errors.New("custom error")) - }, retry.WithLimiter(l)) + }, retry.WithLimiter(retryLimiter)) require.ErrorIs(t, err, retry.ErrNoQuota) }) t.Run("db.Table().Do", func(t *testing.T) { err := nativeDriver.Table().Do(ctx, func(ctx context.Context, s table.Session) error { return retry.RetryableError(errors.New("custom error")) - }, table.WithRetryOptions(retry.WithLimiter(l))) + }, table.WithRetryOptions(retry.WithLimiter(retryLimiter))) require.ErrorIs(t, err, retry.ErrNoQuota) }) t.Run("db.Table().DoTx", func(t *testing.T) { err := nativeDriver.Table().DoTx(ctx, func(ctx context.Context, tx table.TransactionActor) error { return retry.RetryableError(errors.New("custom error")) - }, table.WithRetryOptions(retry.WithLimiter(l))) + }, table.WithRetryOptions(retry.WithLimiter(retryLimiter))) require.ErrorIs(t, err, retry.ErrNoQuota) }) if version.Gte(os.Getenv("YDB_VERSION"), "24.1") { t.Run("db.Query().Do", func(t *testing.T) { err := nativeDriver.Query().Do(ctx, func(ctx context.Context, s query.Session) error { return retry.RetryableError(errors.New("custom error")) - }, query.WithRetryOptions(retry.WithLimiter(l))) + }, query.WithRetryOptions(retry.WithLimiter(retryLimiter))) require.ErrorIs(t, err, retry.ErrNoQuota) }) t.Run("db.Query().DoTx", func(t *testing.T) { err := nativeDriver.Query().DoTx(ctx, func(ctx context.Context, tx query.TxActor) error { return retry.RetryableError(errors.New("custom error")) - }, query.WithRetryOptions(retry.WithLimiter(l))) + }, query.WithRetryOptions(retry.WithLimiter(retryLimiter))) require.ErrorIs(t, err, retry.ErrNoQuota) }) } From d5c6c4a193c81ea48f328635dc4209c56bf06316 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 8 Apr 2024 16:43:46 +0300 Subject: [PATCH 18/33] added experimental comment --- config/config.go | 1 + internal/query/options/retry.go | 48 ++++++++----------------- options.go | 2 ++ query/client.go | 5 +-- retry/quoter.go | 5 +++ retry/retry.go | 2 ++ table/table.go | 21 ++++++----- tests/integration/retry_limiter_test.go | 8 ++--- 8 files changed, 41 insertions(+), 51 deletions(-) diff --git a/config/config.go b/config/config.go index 29d8d434d..99e482258 100644 --- a/config/config.go +++ b/config/config.go @@ -158,6 +158,7 @@ func WithTrace(t trace.Driver, opts ...trace.DriverComposeOption) Option { //nol } } +// Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental func WithRetryLimiter(l retry.Limiter) Option { return func(c *Config) { config.SetRetryLimiter(&c.Common, l) diff --git a/internal/query/options/retry.go b/internal/query/options/retry.go index a2c643842..9726a29d5 100644 --- a/internal/query/options/retry.go +++ b/internal/query/options/retry.go @@ -7,12 +7,10 @@ import ( ) var ( - _ DoOption = idempotentOption{} - _ DoOption = labelOption("") + _ DoOption = retryOptionsOption(nil) _ DoOption = traceOption{} - _ DoTxOption = idempotentOption{} - _ DoTxOption = labelOption("") + _ DoTxOption = retryOptionsOption(nil) _ DoTxOption = traceOption{} _ DoTxOption = doTxSettingsOption{} ) @@ -36,15 +34,13 @@ type ( txSettings tx.Settings } - idempotentOption struct{} - labelOption string - traceOption struct { + retryOptionsOption []retry.Option + traceOption struct { t *trace.Query } doTxSettingsOption struct { txSettings tx.Settings } - retryOptionsOption []retry.Option ) func (s *doSettings) Trace() *trace.Query { @@ -63,14 +59,6 @@ func (s *doTxSettings) TxSettings() tx.Settings { return s.txSettings } -func (opt idempotentOption) applyDoTxOption(s *doTxSettings) { - s.doOpts = append(s.doOpts, opt) -} - -func (idempotentOption) applyDoOption(s *doSettings) { - s.retryOpts = append(s.retryOpts, retry.WithIdempotent(true)) -} - func (opt traceOption) applyDoOption(s *doSettings) { s.trace = s.trace.Compose(opt.t) } @@ -79,12 +67,12 @@ func (opt traceOption) applyDoTxOption(s *doTxSettings) { s.doOpts = append(s.doOpts, opt) } -func (opt labelOption) applyDoOption(s *doSettings) { - s.retryOpts = append(s.retryOpts, retry.WithLabel(string(opt))) +func (opts retryOptionsOption) applyDoOption(s *doSettings) { + s.retryOpts = append(s.retryOpts, opts...) } -func (opt labelOption) applyDoTxOption(s *doTxSettings) { - s.doOpts = append(s.doOpts, opt) +func (opts retryOptionsOption) applyDoTxOption(s *doTxSettings) { + s.doOpts = append(s.doOpts, opts) } func (opt doTxSettingsOption) applyDoTxOption(opts *doTxSettings) { @@ -95,28 +83,20 @@ func WithTxSettings(txSettings tx.Settings) doTxSettingsOption { return doTxSettingsOption{txSettings: txSettings} } -func WithIdempotent() idempotentOption { - return idempotentOption{} +func WithIdempotent() retryOptionsOption { + return []retry.Option{retry.WithIdempotent(true)} } -func WithLabel(lbl string) labelOption { - return labelOption(lbl) +func WithLabel(lbl string) retryOptionsOption { + return []retry.Option{retry.WithLabel(lbl)} } func WithTrace(t *trace.Query) traceOption { return traceOption{t: t} } -func (opts retryOptionsOption) applyDoOption(s *doSettings) { - s.retryOpts = append(s.retryOpts, opts...) -} - -func (opts retryOptionsOption) applyDoTxOption(s *doTxSettings) { - s.doOpts = append(s.doOpts, opts) -} - -func WithRetryOptions(retryOptions ...retry.Option) retryOptionsOption { - return retryOptions +func WithLimiter(l retry.Limiter) retryOptionsOption { + return []retry.Option{retry.WithLimiter(l)} } func ParseDoOpts(t *trace.Query, opts ...DoOption) (s *doSettings) { diff --git a/options.go b/options.go index 4628bee41..0a6460720 100644 --- a/options.go +++ b/options.go @@ -296,6 +296,8 @@ func WithDiscoveryInterval(discoveryInterval time.Duration) Option { } // WithRetryLimiter sets retry limiter for all calls of all retryers. +// +// Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental func WithRetryLimiter(l retry.Limiter) Option { return func(ctx context.Context, c *Driver) error { c.options = append(c.options, config.WithRetryLimiter(l)) diff --git a/query/client.go b/query/client.go index 71f8e2f3e..26dada609 100644 --- a/query/client.go +++ b/query/client.go @@ -68,6 +68,7 @@ func WithLabel(lbl string) bothDoAndDoTxOption { return options.WithLabel(lbl) } -func WithRetryOptions(retryOptions ...retry.Option) bothDoAndDoTxOption { - return options.WithRetryOptions(retryOptions...) +// Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental +func WithLimiter(l retry.Limiter) bothDoAndDoTxOption { + return options.WithLimiter(l) } diff --git a/retry/quoter.go b/retry/quoter.go index 16407967f..fd8e5abbf 100644 --- a/retry/quoter.go +++ b/retry/quoter.go @@ -11,12 +11,14 @@ import ( ) var ( + // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental ErrNoQuota = xerrors.Wrap(errors.New("no retry quota")) _ Limiter = (*rateLimiter)(nil) ) type ( + // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental Limiter interface { Acquire(ctx context.Context) error } @@ -29,6 +31,7 @@ type ( rateLimiterOption func(q *rateLimiter) ) +// Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental func Quoter(attemptsPerSecond int, opts ...rateLimiterOption) *rateLimiter { q := &rateLimiter{ clock: clockwork.NewRealClock(), @@ -66,6 +69,7 @@ func Quoter(attemptsPerSecond int, opts ...rateLimiterOption) *rateLimiter { return q } +// Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental func (q *rateLimiter) Stop() { if q.ticker != nil { q.ticker.Stop() @@ -73,6 +77,7 @@ func (q *rateLimiter) Stop() { close(q.done) } +// Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental func (q *rateLimiter) Acquire(ctx context.Context) error { select { case <-ctx.Done(): diff --git a/retry/retry.go b/retry/retry.go index 813b06f38..c2e139d1f 100644 --- a/retry/retry.go +++ b/retry/retry.go @@ -144,6 +144,8 @@ func (l limiterOption) ApplyDoTxOption(opts *doTxOptions) { } // WithLimiter returns limiter option +// +// Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental func WithLimiter(l Limiter) limiterOption { return limiterOption{l: l} } diff --git a/table/table.go b/table/table.go index 920a9d7d0..6435e9289 100644 --- a/table/table.go +++ b/table/table.go @@ -523,21 +523,20 @@ func (retryOptions retryOptionsOption) ApplyTableOption(opts *Options) { opts.RetryOptions = append(opts.RetryOptions, retryOptions...) } -func WithRetryOptions(retryOptions ...retry.Option) retryOptionsOption { - return retryOptions +// Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental +func WithLimiter(l retry.Limiter) retryOptionsOption { + return []retry.Option{retry.WithLimiter(l)} } -var _ Option = idempotentOption{} - -type idempotentOption struct{} - -func (idempotentOption) ApplyTableOption(opts *Options) { - opts.Idempotent = true - opts.RetryOptions = append(opts.RetryOptions, retry.WithIdempotent(true)) +// Deprecated: redundant option +// Will be removed after Oct 2024. +// Read about versioning policy: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#deprecated +func WithRetryOptions(retryOptions []retry.Option) retryOptionsOption { + return retryOptions } -func WithIdempotent() idempotentOption { - return idempotentOption{} +func WithIdempotent() retryOptionsOption { + return []retry.Option{retry.WithIdempotent(true)} } var _ Option = txSettingsOption{} diff --git a/tests/integration/retry_limiter_test.go b/tests/integration/retry_limiter_test.go index 5c367d03b..77d93f49d 100644 --- a/tests/integration/retry_limiter_test.go +++ b/tests/integration/retry_limiter_test.go @@ -81,26 +81,26 @@ func TestRetryLimiter(t *testing.T) { t.Run("db.Table().Do", func(t *testing.T) { err := nativeDriver.Table().Do(ctx, func(ctx context.Context, s table.Session) error { return retry.RetryableError(errors.New("custom error")) - }, table.WithRetryOptions(retry.WithLimiter(retryLimiter))) + }, table.WithLimiter(retryLimiter)) require.ErrorIs(t, err, retry.ErrNoQuota) }) t.Run("db.Table().DoTx", func(t *testing.T) { err := nativeDriver.Table().DoTx(ctx, func(ctx context.Context, tx table.TransactionActor) error { return retry.RetryableError(errors.New("custom error")) - }, table.WithRetryOptions(retry.WithLimiter(retryLimiter))) + }, table.WithLimiter(retryLimiter)) require.ErrorIs(t, err, retry.ErrNoQuota) }) if version.Gte(os.Getenv("YDB_VERSION"), "24.1") { t.Run("db.Query().Do", func(t *testing.T) { err := nativeDriver.Query().Do(ctx, func(ctx context.Context, s query.Session) error { return retry.RetryableError(errors.New("custom error")) - }, query.WithRetryOptions(retry.WithLimiter(retryLimiter))) + }, query.WithLimiter(retryLimiter)) require.ErrorIs(t, err, retry.ErrNoQuota) }) t.Run("db.Query().DoTx", func(t *testing.T) { err := nativeDriver.Query().DoTx(ctx, func(ctx context.Context, tx query.TxActor) error { return retry.RetryableError(errors.New("custom error")) - }, query.WithRetryOptions(retry.WithLimiter(retryLimiter))) + }, query.WithLimiter(retryLimiter)) require.ErrorIs(t, err, retry.ErrNoQuota) }) } From e7e892871c18ce28b03758884047af7934b8708e Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 8 Apr 2024 21:46:14 +0300 Subject: [PATCH 19/33] added retry limiter to SLO --- tests/slo/database/sql/storage.go | 32 ++++++++++++++++++++----------- tests/slo/native/query/storage.go | 26 ++++++++++++++++--------- tests/slo/native/table/storage.go | 31 ++++++++++++++++++++---------- 3 files changed, 59 insertions(+), 30 deletions(-) diff --git a/tests/slo/database/sql/storage.go b/tests/slo/database/sql/storage.go index 8c740803c..65b897ddd 100755 --- a/tests/slo/database/sql/storage.go +++ b/tests/slo/database/sql/storage.go @@ -66,32 +66,40 @@ var ( ) type Storage struct { - cc *ydb.Driver - c ydb.SQLConnector - db *sql.DB - cfg *config.Config - createQuery string - dropQuery string - upsertQuery string - selectQuery string + cc *ydb.Driver + c ydb.SQLConnector + db *sql.DB + cfg *config.Config + createQuery string + dropQuery string + upsertQuery string + selectQuery string + retryLimiter interface { + retry.Limiter + Stop() + } } func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (s *Storage, err error) { ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() + retryLimiter := retry.Quoter(int(float64(poolSize) * (1.0 + 0.20))) + s = &Storage{ cfg: cfg, createQuery: fmt.Sprintf(createTemplate, cfg.Table, cfg.PartitionSize, cfg.MinPartitionsCount, cfg.MaxPartitionsCount, cfg.MinPartitionsCount), - dropQuery: fmt.Sprintf(dropTemplate, cfg.Table), - upsertQuery: fmt.Sprintf(upsertTemplate, cfg.Table), - selectQuery: fmt.Sprintf(selectTemplate, cfg.Table), + dropQuery: fmt.Sprintf(dropTemplate, cfg.Table), + upsertQuery: fmt.Sprintf(upsertTemplate, cfg.Table), + selectQuery: fmt.Sprintf(selectTemplate, cfg.Table), + retryLimiter: retryLimiter, } s.cc, err = ydb.Open( ctx, s.cfg.Endpoint+s.cfg.DB, + ydb.WithRetryLimiter(retryLimiter), ) if err != nil { return nil, fmt.Errorf("ydb.Open error: %w", err) @@ -224,6 +232,8 @@ func (s *Storage) dropTable(ctx context.Context) error { } func (s *Storage) close(ctx context.Context) error { + s.retryLimiter.Stop() + if err := ctx.Err(); err != nil { return err } diff --git a/tests/slo/native/query/storage.go b/tests/slo/native/query/storage.go index 4c0132adc..3017eec81 100755 --- a/tests/slo/native/query/storage.go +++ b/tests/slo/native/query/storage.go @@ -5,13 +5,12 @@ import ( "errors" "fmt" "io" - "os" "path" "time" ydb "github.com/ydb-platform/ydb-go-sdk/v3" - "github.com/ydb-platform/ydb-go-sdk/v3/log" "github.com/ydb-platform/ydb-go-sdk/v3/query" + "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/trace" "slo/internal/config" @@ -19,9 +18,13 @@ import ( ) type Storage struct { - db *ydb.Driver - cfg *config.Config - tablePath string + db *ydb.Driver + cfg *config.Config + tablePath string + retryLimiter interface { + retry.Limiter + Stop() + } } const writeQuery = ` @@ -69,10 +72,12 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() + retryLimiter := retry.Quoter(int(float64(poolSize) * (1.0 + 0.20))) + db, err := ydb.Open(ctx, cfg.Endpoint+cfg.DB, ydb.WithSessionPoolSizeLimit(poolSize), - ydb.WithLogger(log.Default(os.Stderr, log.WithMinLevel(log.ERROR)), trace.DetailsAll), + ydb.WithRetryLimiter(retryLimiter), ) if err != nil { return nil, err @@ -81,9 +86,10 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage prefix := path.Join(db.Name(), label) s := &Storage{ - db: db, - cfg: cfg, - tablePath: "`" + path.Join(prefix, cfg.Table) + "`", + db: db, + cfg: cfg, + tablePath: "`" + path.Join(prefix, cfg.Table) + "`", + retryLimiter: retryLimiter, } return s, nil @@ -254,6 +260,8 @@ func (s *Storage) dropTable(ctx context.Context) error { } func (s *Storage) close(ctx context.Context) error { + s.retryLimiter.Stop() + var ( shutdownCtx context.Context shutdownCancel context.CancelFunc diff --git a/tests/slo/native/table/storage.go b/tests/slo/native/table/storage.go index 220054aa0..dd5a17350 100755 --- a/tests/slo/native/table/storage.go +++ b/tests/slo/native/table/storage.go @@ -7,6 +7,7 @@ import ( "time" "github.com/ydb-platform/ydb-go-sdk/v3" + "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/table" "github.com/ydb-platform/ydb-go-sdk/v3/table/options" "github.com/ydb-platform/ydb-go-sdk/v3/table/result" @@ -57,21 +58,28 @@ var ( ) type Storage struct { - db *ydb.Driver - cfg *config.Config - prefix string - upsertQuery string - selectQuery string + db *ydb.Driver + cfg *config.Config + prefix string + upsertQuery string + selectQuery string + retryLimiter interface { + retry.Limiter + Stop() + } } func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage, error) { ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() + retryLimiter := retry.Quoter(int(float64(poolSize) * (1.0 + 0.20))) + db, err := ydb.Open( ctx, cfg.Endpoint+cfg.DB, ydb.WithSessionPoolSizeLimit(poolSize), + ydb.WithRetryLimiter(retryLimiter), ) if err != nil { return nil, err @@ -80,11 +88,12 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage prefix := path.Join(db.Name(), label) s := &Storage{ - db: db, - cfg: cfg, - prefix: prefix, - upsertQuery: fmt.Sprintf(upsertTemplate, prefix, cfg.Table), - selectQuery: fmt.Sprintf(selectTemplate, prefix, cfg.Table), + db: db, + cfg: cfg, + prefix: prefix, + upsertQuery: fmt.Sprintf(upsertTemplate, prefix, cfg.Table), + selectQuery: fmt.Sprintf(selectTemplate, prefix, cfg.Table), + retryLimiter: retryLimiter, } return s, nil @@ -245,6 +254,8 @@ func (s *Storage) dropTable(ctx context.Context) error { } func (s *Storage) close(ctx context.Context) error { + s.retryLimiter.Stop() + ctx, cancel := context.WithTimeout(ctx, time.Duration(s.cfg.ShutdownTime)*time.Second) defer cancel() From 33c7e29ec02983663dbbe89f74a5357667337a0b Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 8 Apr 2024 22:18:43 +0300 Subject: [PATCH 20/33] fix test --- retry/retry_test.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/retry/retry_test.go b/retry/retry_test.go index e603a237a..0d43f7155 100644 --- a/retry/retry_test.go +++ b/retry/retry_test.go @@ -190,11 +190,16 @@ func TestRetryTransportCancelled(t *testing.T) { } } +type noQuota struct{} + +func (noQuota) Acquire(ctx context.Context) error { + return errors.New("no quota") +} + func TestRetryWithLimiter(t *testing.T) { xtest.TestManyTimes(t, func(t testing.TB) { - l := Quoter(10) - defer l.Stop() - ctx, cancel := context.WithTimeout(xtest.Context(t), 5*time.Millisecond) + l := noQuota{} + ctx, cancel := context.WithCancel(xtest.Context(t)) defer cancel() err := Retry(ctx, func(ctx context.Context) (err error) { return RetryableError(errors.New("custom error")) From 8f7a1d3c51c5d5c250704166be33cbf7710ebfc7 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Tue, 9 Apr 2024 10:28:16 +0300 Subject: [PATCH 21/33] fix retry budget in SLO --- tests/slo/database/sql/storage.go | 2 +- tests/slo/native/query/storage.go | 2 +- tests/slo/native/table/storage.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/slo/database/sql/storage.go b/tests/slo/database/sql/storage.go index 65b897ddd..ba51214a1 100755 --- a/tests/slo/database/sql/storage.go +++ b/tests/slo/database/sql/storage.go @@ -84,7 +84,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (s *Stora ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryLimiter := retry.Quoter(int(float64(poolSize) * (1.0 + 0.20))) + retryLimiter := retry.Quoter(int(float64(poolSize) * 0.1)) s = &Storage{ cfg: cfg, diff --git a/tests/slo/native/query/storage.go b/tests/slo/native/query/storage.go index 3017eec81..9d4aa463c 100755 --- a/tests/slo/native/query/storage.go +++ b/tests/slo/native/query/storage.go @@ -72,7 +72,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryLimiter := retry.Quoter(int(float64(poolSize) * (1.0 + 0.20))) + retryLimiter := retry.Quoter(int(float64(poolSize) * 0.1)) db, err := ydb.Open(ctx, cfg.Endpoint+cfg.DB, diff --git a/tests/slo/native/table/storage.go b/tests/slo/native/table/storage.go index dd5a17350..7e06d0995 100755 --- a/tests/slo/native/table/storage.go +++ b/tests/slo/native/table/storage.go @@ -73,7 +73,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryLimiter := retry.Quoter(int(float64(poolSize) * (1.0 + 0.20))) + retryLimiter := retry.Quoter(int(float64(poolSize) * 0.1)) db, err := ydb.Open( ctx, From cc0f31788239c3e59b1cf3a61179476a2ab73725 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Tue, 9 Apr 2024 10:39:39 +0300 Subject: [PATCH 22/33] LimiterStopper interface --- retry/quoter.go | 5 +++++ tests/slo/database/sql/storage.go | 5 +---- tests/slo/native/query/storage.go | 5 +---- tests/slo/native/table/storage.go | 5 +---- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/retry/quoter.go b/retry/quoter.go index fd8e5abbf..9e8949c2b 100644 --- a/retry/quoter.go +++ b/retry/quoter.go @@ -22,6 +22,11 @@ type ( Limiter interface { Acquire(ctx context.Context) error } + // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental + LimiterStoper interface { + Limiter + Stop() + } rateLimiter struct { clock clockwork.Clock ticker clockwork.Ticker diff --git a/tests/slo/database/sql/storage.go b/tests/slo/database/sql/storage.go index ba51214a1..9c0cbc0e7 100755 --- a/tests/slo/database/sql/storage.go +++ b/tests/slo/database/sql/storage.go @@ -74,10 +74,7 @@ type Storage struct { dropQuery string upsertQuery string selectQuery string - retryLimiter interface { - retry.Limiter - Stop() - } + retryLimiter retry.LimiterStoper } func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (s *Storage, err error) { diff --git a/tests/slo/native/query/storage.go b/tests/slo/native/query/storage.go index 9d4aa463c..3760d6608 100755 --- a/tests/slo/native/query/storage.go +++ b/tests/slo/native/query/storage.go @@ -21,10 +21,7 @@ type Storage struct { db *ydb.Driver cfg *config.Config tablePath string - retryLimiter interface { - retry.Limiter - Stop() - } + retryLimiter retry.LimiterStoper } const writeQuery = ` diff --git a/tests/slo/native/table/storage.go b/tests/slo/native/table/storage.go index 7e06d0995..c8777e729 100755 --- a/tests/slo/native/table/storage.go +++ b/tests/slo/native/table/storage.go @@ -63,10 +63,7 @@ type Storage struct { prefix string upsertQuery string selectQuery string - retryLimiter interface { - retry.Limiter - Stop() - } + retryLimiter retry.LimiterStoper } func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage, error) { From 8a96d36ae738edab3a59d1dcb1cbe2be65a24964 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Sat, 23 Mar 2024 14:29:57 +0300 Subject: [PATCH 23/33] fix mistake --- internal/backoff/delay.go | 4 ++-- internal/backoff/delay_test.go | 12 ++++++------ retry/retry.go | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/backoff/delay.go b/internal/backoff/delay.go index e0d89d46e..21b07522d 100644 --- a/internal/backoff/delay.go +++ b/internal/backoff/delay.go @@ -12,13 +12,13 @@ type ( delayOption func(o *delayOptions) ) -func WithFastBackof(fast Backoff) delayOption { +func WithFastBackoff(fast Backoff) delayOption { return func(o *delayOptions) { o.fast = fast } } -func WithSlowBackof(slow Backoff) delayOption { +func WithSlowBackoff(slow Backoff) delayOption { return func(o *delayOptions) { o.slow = slow } diff --git a/internal/backoff/delay_test.go b/internal/backoff/delay_test.go index 1a238aa99..7c813b89a 100644 --- a/internal/backoff/delay_test.go +++ b/internal/backoff/delay_test.go @@ -32,7 +32,7 @@ func TestDelay(t *testing.T) { }, { name: xtest.CurrentFileLine(), - act: Delay(TypeFast, 0, WithFastBackof(New( + act: Delay(TypeFast, 0, WithFastBackoff(New( WithSlotDuration(fastSlot), WithCeiling(6), WithJitterLimit(1), @@ -41,7 +41,7 @@ func TestDelay(t *testing.T) { }, { name: xtest.CurrentFileLine(), - act: Delay(TypeFast, 1, WithFastBackof(New( + act: Delay(TypeFast, 1, WithFastBackoff(New( WithSlotDuration(fastSlot), WithCeiling(6), WithJitterLimit(1), @@ -50,7 +50,7 @@ func TestDelay(t *testing.T) { }, { name: xtest.CurrentFileLine(), - act: Delay(TypeFast, 3, WithFastBackof(New( + act: Delay(TypeFast, 3, WithFastBackoff(New( WithSlotDuration(fastSlot), WithCeiling(6), WithJitterLimit(1), @@ -59,7 +59,7 @@ func TestDelay(t *testing.T) { }, { name: xtest.CurrentFileLine(), - act: Delay(TypeSlow, 0, WithSlowBackof(New( + act: Delay(TypeSlow, 0, WithSlowBackoff(New( WithSlotDuration(slowSlot), WithCeiling(6), WithJitterLimit(1), @@ -68,7 +68,7 @@ func TestDelay(t *testing.T) { }, { name: xtest.CurrentFileLine(), - act: Delay(TypeSlow, 1, WithSlowBackof(New( + act: Delay(TypeSlow, 1, WithSlowBackoff(New( WithSlotDuration(slowSlot), WithCeiling(6), WithJitterLimit(1), @@ -77,7 +77,7 @@ func TestDelay(t *testing.T) { }, { name: xtest.CurrentFileLine(), - act: Delay(TypeSlow, 3, WithSlowBackof(New( + act: Delay(TypeSlow, 3, WithSlowBackoff(New( WithSlotDuration(slowSlot), WithCeiling(6), WithJitterLimit(1), diff --git a/retry/retry.go b/retry/retry.go index c2e139d1f..12a782a3e 100644 --- a/retry/retry.go +++ b/retry/retry.go @@ -341,8 +341,8 @@ func Retry(ctx context.Context, op retryOperation, opts ...Option) (finalErr err } t := time.NewTimer(backoff.Delay(m.BackoffType(), i, - backoff.WithFastBackof(options.fastBackoff), - backoff.WithSlowBackof(options.slowBackoff), + backoff.WithFastBackoff(options.fastBackoff), + backoff.WithSlowBackoff(options.slowBackoff), )) select { From 2a635d19fc14c8e12a8868723c7ad5421c6ca0e8 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 22 Apr 2024 11:58:37 +0300 Subject: [PATCH 24/33] refactored retry budget sources --- config/config.go | 4 +- internal/balancer/balancer.go | 2 +- internal/config/config.go | 9 +++-- internal/coordination/client.go | 8 ++-- internal/query/options/retry.go | 5 ++- internal/ratelimiter/client.go | 12 +++--- retry/quoter.go => internal/retry/budget.go | 33 +++++++---------- .../retry/budget_test.go | 10 ++--- internal/scheme/client.go | 10 ++--- internal/scripting/client.go | 6 +-- internal/table/retry.go | 2 +- internal/topic/topicclientinternal/client.go | 8 ++-- internal/xsql/conn.go | 2 +- internal/xsql/connector.go | 10 ++--- options.go | 6 +-- query/client.go | 6 +-- retry/budget.go | 22 +++++++++++ retry/retry.go | 19 +++++----- retry/retry_test.go | 12 +++--- retry/sql.go | 5 ++- table/table.go | 5 ++- ...y_limiter_test.go => retry_budget_test.go} | 35 +++++++++--------- tests/slo/database/sql/storage.go | 37 +++++++++++-------- tests/slo/native/query/storage.go | 27 ++++++++------ tests/slo/native/table/storage.go | 35 ++++++++++-------- 25 files changed, 183 insertions(+), 147 deletions(-) rename retry/quoter.go => internal/retry/budget.go (71%) rename retry/quoter_test.go => internal/retry/budget_test.go (87%) create mode 100644 retry/budget.go rename tests/integration/{retry_limiter_test.go => retry_budget_test.go} (76%) diff --git a/config/config.go b/config/config.go index 99e482258..ac25e4537 100644 --- a/config/config.go +++ b/config/config.go @@ -12,7 +12,7 @@ import ( balancerConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer/config" "github.com/ydb-platform/ydb-go-sdk/v3/internal/config" "github.com/ydb-platform/ydb-go-sdk/v3/internal/meta" - "github.com/ydb-platform/ydb-go-sdk/v3/retry" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -159,7 +159,7 @@ func WithTrace(t trace.Driver, opts ...trace.DriverComposeOption) Option { //nol } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func WithRetryLimiter(l retry.Limiter) Option { +func WithRetryLimiter(l retry.Budget) Option { return func(c *Config) { config.SetRetryLimiter(&c.Common, l) } diff --git a/internal/balancer/balancer.go b/internal/balancer/balancer.go index 147b7bd3d..96cd20db5 100644 --- a/internal/balancer/balancer.go +++ b/internal/balancer/balancer.go @@ -89,7 +89,7 @@ func (b *Balancer) clusterDiscovery(ctx context.Context) (err error) { }, retry.WithIdempotent(true), retry.WithTrace(b.driverConfig.TraceRetry()), - retry.WithLimiter(b.driverConfig.RetryLimiter()), + retry.WithBudget(b.driverConfig.RetryLimiter()), ) } diff --git a/internal/config/config.go b/internal/config/config.go index d18e980ab..842239f09 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,18 +3,19 @@ package config import ( "time" + retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) -var defaultRetryLimiter = retry.Quoter(-1) +var defaultRetryLimiter = retry.Budget(-1) type Common struct { operationTimeout time.Duration operationCancelAfter time.Duration disableAutoRetry bool traceRetry trace.Retry - retryLimiter retry.Limiter + retryLimiter retry2.Budget panicCallback func(e interface{}) } @@ -52,7 +53,7 @@ func (c *Common) TraceRetry() *trace.Retry { return &c.traceRetry } -func (c *Common) RetryLimiter() retry.Limiter { +func (c *Common) RetryLimiter() retry2.Budget { if c.retryLimiter == nil { return defaultRetryLimiter } @@ -94,6 +95,6 @@ func SetTraceRetry(c *Common, t *trace.Retry, opts ...trace.RetryComposeOption) c.traceRetry = *c.traceRetry.Compose(t, opts...) } -func SetRetryLimiter(c *Common, l retry.Limiter) { +func SetRetryLimiter(c *Common, l retry2.Budget) { c.retryLimiter = l } diff --git a/internal/coordination/client.go b/internal/coordination/client.go index 2a3670b4d..23e5fa7d3 100644 --- a/internal/coordination/client.go +++ b/internal/coordination/client.go @@ -117,7 +117,7 @@ func (c *Client) CreateNode(ctx context.Context, path string, config coordinatio retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) } @@ -150,7 +150,7 @@ func (c *Client) AlterNode(ctx context.Context, path string, config coordination retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) } @@ -211,7 +211,7 @@ func (c *Client) DropNode(ctx context.Context, path string) (finalErr error) { retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) } @@ -271,7 +271,7 @@ func (c *Client) DescribeNode( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) if err != nil { return nil, nil, xerrors.WithStackTrace(err) diff --git a/internal/query/options/retry.go b/internal/query/options/retry.go index 9726a29d5..870ab442a 100644 --- a/internal/query/options/retry.go +++ b/internal/query/options/retry.go @@ -2,6 +2,7 @@ package options import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/query/tx" + retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -95,8 +96,8 @@ func WithTrace(t *trace.Query) traceOption { return traceOption{t: t} } -func WithLimiter(l retry.Limiter) retryOptionsOption { - return []retry.Option{retry.WithLimiter(l)} +func WithBudget(l retry2.Budget) retryOptionsOption { + return []retry.Option{retry.WithBudget(l)} } func ParseDoOpts(t *trace.Query, opts ...DoOption) (s *doSettings) { diff --git a/internal/ratelimiter/client.go b/internal/ratelimiter/client.go index 5fc580cd3..19744a949 100644 --- a/internal/ratelimiter/client.go +++ b/internal/ratelimiter/client.go @@ -63,7 +63,7 @@ func (c *Client) CreateResource( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) } @@ -113,7 +113,7 @@ func (c *Client) AlterResource( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) } @@ -163,7 +163,7 @@ func (c *Client) DropResource( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) } @@ -209,7 +209,7 @@ func (c *Client) ListResource( retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) return list, err @@ -269,7 +269,7 @@ func (c *Client) DescribeResource( retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) return @@ -338,7 +338,7 @@ func (c *Client) AcquireResource( return retry.Retry(ctx, call, retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) } diff --git a/retry/quoter.go b/internal/retry/budget.go similarity index 71% rename from retry/quoter.go rename to internal/retry/budget.go index 9e8949c2b..e2a954d50 100644 --- a/retry/quoter.go +++ b/internal/retry/budget.go @@ -2,7 +2,6 @@ package retry import ( "context" - "errors" "time" "github.com/jonboulle/clockwork" @@ -10,35 +9,29 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" ) -var ( - // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental - ErrNoQuota = xerrors.Wrap(errors.New("no retry quota")) - - _ Limiter = (*rateLimiter)(nil) -) - type ( // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental - Limiter interface { + Budget interface { Acquire(ctx context.Context) error } - // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental - LimiterStoper interface { - Limiter - Stop() - } - rateLimiter struct { + budget struct { clock clockwork.Clock ticker clockwork.Ticker quota chan struct{} done chan struct{} } - rateLimiterOption func(q *rateLimiter) + BudgetOption func(q *budget) ) +func WithBudgetClock(clock clockwork.Clock) BudgetOption { + return func(q *budget) { + q.clock = clock + } +} + // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func Quoter(attemptsPerSecond int, opts ...rateLimiterOption) *rateLimiter { - q := &rateLimiter{ +func NewBudget(attemptsPerSecond int, opts ...BudgetOption) *budget { + q := &budget{ clock: clockwork.NewRealClock(), done: make(chan struct{}), } @@ -75,7 +68,7 @@ func Quoter(attemptsPerSecond int, opts ...rateLimiterOption) *rateLimiter { } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func (q *rateLimiter) Stop() { +func (q *budget) Stop() { if q.ticker != nil { q.ticker.Stop() } @@ -83,7 +76,7 @@ func (q *rateLimiter) Stop() { } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func (q *rateLimiter) Acquire(ctx context.Context) error { +func (q *budget) Acquire(ctx context.Context) error { select { case <-ctx.Done(): return ctx.Err() diff --git a/retry/quoter_test.go b/internal/retry/budget_test.go similarity index 87% rename from retry/quoter_test.go rename to internal/retry/budget_test.go index 2318af439..714f8e807 100644 --- a/retry/quoter_test.go +++ b/internal/retry/budget_test.go @@ -12,23 +12,21 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/xtest" ) -func TestUnlimitedLimiter(t *testing.T) { +func TestUnlimitedBudget(t *testing.T) { xtest.TestManyTimes(t, func(t testing.TB) { ctx, cancel := xcontext.WithCancel(xtest.Context(t)) - q := Quoter(-1) + q := NewBudget(-1) require.NoError(t, q.Acquire(ctx)) cancel() require.ErrorIs(t, q.Acquire(ctx), context.Canceled) }) } -func TestQuoter(t *testing.T) { +func TestBudget(t *testing.T) { xtest.TestManyTimes(t, func(t testing.TB) { ctx, cancel := xcontext.WithCancel(xtest.Context(t)) clock := clockwork.NewFakeClock() - q := Quoter(1, func(q *rateLimiter) { - q.clock = clock - }) + q := NewBudget(1, WithBudgetClock(clock)) defer q.Stop() require.NoError(t, q.Acquire(ctx)) acquireCh := make(chan struct{}) diff --git a/internal/scheme/client.go b/internal/scheme/client.go index f2c61dd4f..8b93ea0d7 100644 --- a/internal/scheme/client.go +++ b/internal/scheme/client.go @@ -64,7 +64,7 @@ func (c *Client) MakeDirectory(ctx context.Context, path string) (finalErr error retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) } @@ -104,7 +104,7 @@ func (c *Client) RemoveDirectory(ctx context.Context, path string) (finalErr err retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) } @@ -146,7 +146,7 @@ func (c *Client) ListDirectory(ctx context.Context, path string) (d scheme.Direc retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) return d, xerrors.WithStackTrace(err) @@ -210,7 +210,7 @@ func (c *Client) DescribePath(ctx context.Context, path string) (e scheme.Entry, retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) return e, xerrors.WithStackTrace(err) @@ -272,7 +272,7 @@ func (c *Client) ModifyPermissions( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) } diff --git a/internal/scripting/client.go b/internal/scripting/client.go index 473997ce9..ecdd664f7 100644 --- a/internal/scripting/client.go +++ b/internal/scripting/client.go @@ -60,7 +60,7 @@ func (c *Client) Execute( err = retry.Retry(ctx, call, retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) return r, xerrors.WithStackTrace(err) @@ -140,7 +140,7 @@ func (c *Client) Explain( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) return e, xerrors.WithStackTrace(err) @@ -215,7 +215,7 @@ func (c *Client) StreamExecute( err = retry.Retry(ctx, call, retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), ) return r, xerrors.WithStackTrace(err) diff --git a/internal/table/retry.go b/internal/table/retry.go index f9ca31e4b..f385f969f 100644 --- a/internal/table/retry.go +++ b/internal/table/retry.go @@ -99,7 +99,7 @@ func (c *Client) retryOptions(opts ...table.Option) *table.Options { ), RetryOptions: []retry.Option{ retry.WithTrace(c.config.TraceRetry()), - retry.WithLimiter(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryLimiter()), }, } for _, opt := range opts { diff --git a/internal/topic/topicclientinternal/client.go b/internal/topic/topicclientinternal/client.go index 7d228baf6..ae62d43ba 100644 --- a/internal/topic/topicclientinternal/client.go +++ b/internal/topic/topicclientinternal/client.go @@ -87,7 +87,7 @@ func (c *Client) Alter(ctx context.Context, path string, opts ...topicoptions.Al return retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), - retry.WithLimiter(c.cfg.RetryLimiter()), + retry.WithBudget(c.cfg.RetryLimiter()), ) } @@ -120,7 +120,7 @@ func (c *Client) Create( return retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), - retry.WithLimiter(c.cfg.RetryLimiter()), + retry.WithBudget(c.cfg.RetryLimiter()), ) } @@ -158,7 +158,7 @@ func (c *Client) Describe( err = retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), - retry.WithLimiter(c.cfg.RetryLimiter()), + retry.WithBudget(c.cfg.RetryLimiter()), ) } else { err = call(ctx) @@ -195,7 +195,7 @@ func (c *Client) Drop(ctx context.Context, path string, opts ...topicoptions.Dro return retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), - retry.WithLimiter(c.cfg.RetryLimiter()), + retry.WithBudget(c.cfg.RetryLimiter()), ) } diff --git a/internal/xsql/conn.go b/internal/xsql/conn.go index 1fe636a94..211ff3d07 100644 --- a/internal/xsql/conn.go +++ b/internal/xsql/conn.go @@ -831,7 +831,7 @@ func (c *conn) retryIdempotent(ctx context.Context, f func(ctx context.Context) err := retry.Retry(ctx, f, retry.WithIdempotent(true), retry.WithTrace(c.connector.traceRetry), - retry.WithLimiter(c.connector.retryLimiter), + retry.WithBudget(c.connector.retryLimiter), ) if err != nil { return xerrors.WithStackTrace(err) diff --git a/internal/xsql/connector.go b/internal/xsql/connector.go index abd9e5673..a934b376c 100644 --- a/internal/xsql/connector.go +++ b/internal/xsql/connector.go @@ -11,11 +11,11 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/bind" metaHeaders "github.com/ydb-platform/ydb-go-sdk/v3/internal/meta" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/internal/stack" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" "github.com/ydb-platform/ydb-go-sdk/v3/meta" - "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/scheme" "github.com/ydb-platform/ydb-go-sdk/v3/scripting" "github.com/ydb-platform/ydb-go-sdk/v3/table" @@ -177,7 +177,7 @@ func WithTraceRetry(t *trace.Retry) ConnectorOption { } type retryLimiterConnectorOption struct { - l retry.Limiter + l retry.Budget } func (l retryLimiterConnectorOption) Apply(c *Connector) error { @@ -186,7 +186,7 @@ func (l retryLimiterConnectorOption) Apply(c *Connector) error { return nil } -func WithRetryLimiter(l retry.Limiter) ConnectorOption { +func WithRetryLimiter(l retry.Budget) ConnectorOption { return retryLimiterConnectorOption{l: l} } @@ -265,7 +265,7 @@ type Connector struct { trace *trace.DatabaseSQL traceRetry *trace.Retry - retryLimiter retry.Limiter + retryLimiter retry.Budget } var ( @@ -369,7 +369,7 @@ func (d *driverWrapper) TraceRetry() *trace.Retry { return d.c.traceRetry } -func (d *driverWrapper) RetryLimiter() retry.Limiter { +func (d *driverWrapper) RetryLimiter() retry.Budget { return d.c.retryLimiter } diff --git a/options.go b/options.go index 0a6460720..39e863713 100644 --- a/options.go +++ b/options.go @@ -19,13 +19,13 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/dsn" queryConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/query/config" ratelimiterConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/ratelimiter/config" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" schemeConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/scheme/config" scriptingConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/scripting/config" tableConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/table/config" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xsql" "github.com/ydb-platform/ydb-go-sdk/v3/log" - "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/topic/topicoptions" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -295,10 +295,10 @@ func WithDiscoveryInterval(discoveryInterval time.Duration) Option { } } -// WithRetryLimiter sets retry limiter for all calls of all retryers. +// WithRetryBudget sets retry limiter for all calls of all retryers. // // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func WithRetryLimiter(l retry.Limiter) Option { +func WithRetryBudget(l retry.Budget) Option { return func(ctx context.Context, c *Driver) error { c.options = append(c.options, config.WithRetryLimiter(l)) diff --git a/query/client.go b/query/client.go index 26dada609..e3269eb9c 100644 --- a/query/client.go +++ b/query/client.go @@ -5,7 +5,7 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/closer" "github.com/ydb-platform/ydb-go-sdk/v3/internal/query/options" - "github.com/ydb-platform/ydb-go-sdk/v3/retry" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -69,6 +69,6 @@ func WithLabel(lbl string) bothDoAndDoTxOption { } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func WithLimiter(l retry.Limiter) bothDoAndDoTxOption { - return options.WithLimiter(l) +func WithBudget(l retry.Budget) bothDoAndDoTxOption { + return options.WithBudget(l) } diff --git a/retry/budget.go b/retry/budget.go new file mode 100644 index 000000000..fa57b3947 --- /dev/null +++ b/retry/budget.go @@ -0,0 +1,22 @@ +package retry + +import ( + "errors" + + "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" +) + +// ErrNoQuota is a special error for no quota provided by external retry budget +// +// Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental +var ErrNoQuota = xerrors.Wrap(errors.New("no retry quota")) + +type budget interface { + retry.Budget + Stop() +} + +func Budget(attemptsPerSecond int, opts ...retry.BudgetOption) budget { + return retry.NewBudget(attemptsPerSecond, opts...) +} diff --git a/retry/retry.go b/retry/retry.go index 12a782a3e..636c0dc8b 100644 --- a/retry/retry.go +++ b/retry/retry.go @@ -6,6 +6,7 @@ import ( "time" "github.com/ydb-platform/ydb-go-sdk/v3/internal/backoff" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/internal/stack" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" @@ -25,7 +26,7 @@ type retryOptions struct { stackTrace bool fastBackoff backoff.Backoff slowBackoff backoff.Backoff - limiter Limiter + budget retry.Budget panicCallback func(e interface{}) } @@ -128,25 +129,25 @@ func WithTrace(t *trace.Retry) traceOption { var _ Option = limiterOption{} type limiterOption struct { - l Limiter + l retry.Budget } func (l limiterOption) ApplyRetryOption(opts *retryOptions) { - opts.limiter = l.l + opts.budget = l.l } func (l limiterOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, WithLimiter(l.l)) + opts.retryOptions = append(opts.retryOptions, WithBudget(l.l)) } func (l limiterOption) ApplyDoTxOption(opts *doTxOptions) { - opts.retryOptions = append(opts.retryOptions, WithLimiter(l.l)) + opts.retryOptions = append(opts.retryOptions, WithBudget(l.l)) } -// WithLimiter returns limiter option +// WithBudget returns budget option // // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func WithLimiter(l Limiter) limiterOption { +func WithBudget(l retry.Budget) limiterOption { return limiterOption{l: l} } @@ -260,7 +261,7 @@ func Retry(ctx context.Context, op retryOperation, opts ...Option) (finalErr err options := &retryOptions{ call: stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/retry.Retry"), trace: &trace.Retry{}, - limiter: Quoter(-1), + budget: Budget(-1), fastBackoff: backoff.Fast, slowBackoff: backoff.Slow, } @@ -358,7 +359,7 @@ func Retry(ctx context.Context, op retryOperation, opts ...Option) (finalErr err case <-t.C: t.Stop() - if acquireErr := options.limiter.Acquire(ctx); acquireErr != nil { + if acquireErr := options.budget.Acquire(ctx); acquireErr != nil { return xerrors.WithStackTrace( xerrors.Join( fmt.Errorf("attempt No.%d: %w", attempts, ErrNoQuota), diff --git a/retry/retry_test.go b/retry/retry_test.go index 0d43f7155..29afcf6fa 100644 --- a/retry/retry_test.go +++ b/retry/retry_test.go @@ -192,18 +192,20 @@ func TestRetryTransportCancelled(t *testing.T) { type noQuota struct{} +var errNoQuota = errors.New("no quota") + func (noQuota) Acquire(ctx context.Context) error { - return errors.New("no quota") + return errNoQuota } -func TestRetryWithLimiter(t *testing.T) { +func TestRetryWithBudget(t *testing.T) { xtest.TestManyTimes(t, func(t testing.TB) { - l := noQuota{} + quota := noQuota{} ctx, cancel := context.WithCancel(xtest.Context(t)) defer cancel() err := Retry(ctx, func(ctx context.Context) (err error) { return RetryableError(errors.New("custom error")) - }, WithLimiter(l)) - require.ErrorIs(t, err, ErrNoQuota) + }, WithBudget(quota)) + require.ErrorIs(t, err, errNoQuota) }) } diff --git a/retry/sql.go b/retry/sql.go index c15ae0290..7ceeb1a02 100644 --- a/retry/sql.go +++ b/retry/sql.go @@ -5,6 +5,7 @@ import ( "database/sql" "fmt" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/internal/stack" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" @@ -144,12 +145,12 @@ func DoTx(ctx context.Context, db *sql.DB, op func(context.Context, *sql.Tx) err ) if d, has := db.Driver().(interface { TraceRetry() *trace.Retry - RetryLimiter() Limiter + RetryLimiter() retry.Budget }); has { options.retryOptions = append(options.retryOptions, nil, nil) copy(options.retryOptions[2:], options.retryOptions) options.retryOptions[0] = WithTrace(d.TraceRetry()) - options.retryOptions[1] = WithLimiter(d.RetryLimiter()) + options.retryOptions[1] = WithBudget(d.RetryLimiter()) } for _, opt := range opts { if opt != nil { diff --git a/table/table.go b/table/table.go index 6435e9289..b19c63ce0 100644 --- a/table/table.go +++ b/table/table.go @@ -8,6 +8,7 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/closer" "github.com/ydb-platform/ydb-go-sdk/v3/internal/params" + retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/internal/types" "github.com/ydb-platform/ydb-go-sdk/v3/internal/value" "github.com/ydb-platform/ydb-go-sdk/v3/retry" @@ -524,8 +525,8 @@ func (retryOptions retryOptionsOption) ApplyTableOption(opts *Options) { } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func WithLimiter(l retry.Limiter) retryOptionsOption { - return []retry.Option{retry.WithLimiter(l)} +func WithBudget(l retry2.Budget) retryOptionsOption { + return []retry.Option{retry.WithBudget(l)} } // Deprecated: redundant option diff --git a/tests/integration/retry_limiter_test.go b/tests/integration/retry_budget_test.go similarity index 76% rename from tests/integration/retry_limiter_test.go rename to tests/integration/retry_budget_test.go index 77d93f49d..b959ff9e0 100644 --- a/tests/integration/retry_limiter_test.go +++ b/tests/integration/retry_budget_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/require" "github.com/ydb-platform/ydb-go-sdk/v3" + retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/internal/version" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xtest" "github.com/ydb-platform/ydb-go-sdk/v3/query" @@ -27,15 +28,15 @@ func (n noQuota) Acquire(ctx context.Context) error { return errors.New("no quota") } -func TestRetryLimiter(t *testing.T) { +func TestRetryBudget(t *testing.T) { ctx := xtest.Context(t) - defaultLimiter := retry.Quoter(1) + defaultLimiter := retry.Budget(1) defer defaultLimiter.Stop() nativeDriver, err := ydb.Open(ctx, os.Getenv("YDB_CONNECTION_STRING"), ydb.WithDiscoveryInterval(time.Second), - ydb.WithRetryLimiter(defaultLimiter), + ydb.WithRetryBudget(defaultLimiter), ) require.NoError(t, err) @@ -63,45 +64,45 @@ func TestRetryLimiter(t *testing.T) { t.Run("retry.Retry", func(t *testing.T) { err := retry.Retry(ctx, func(ctx context.Context) (err error) { return retry.RetryableError(errors.New("custom error")) - }, retry.WithLimiter(retryLimiter)) - require.ErrorIs(t, err, retry.ErrNoQuota) + }, retry.WithBudget(retryLimiter)) + require.ErrorIs(t, err, retry2.ErrNoQuota) }) t.Run("retry.Do", func(t *testing.T) { err := retry.Do(ctx, db, func(ctx context.Context, cc *sql.Conn) (err error) { return retry.RetryableError(errors.New("custom error")) - }, retry.WithLimiter(retryLimiter)) - require.ErrorIs(t, err, retry.ErrNoQuota) + }, retry.WithBudget(retryLimiter)) + require.ErrorIs(t, err, retry2.ErrNoQuota) }) t.Run("retry.DoTx", func(t *testing.T) { err := retry.DoTx(ctx, db, func(ctx context.Context, tx *sql.Tx) (err error) { return retry.RetryableError(errors.New("custom error")) - }, retry.WithLimiter(retryLimiter)) - require.ErrorIs(t, err, retry.ErrNoQuota) + }, retry.WithBudget(retryLimiter)) + require.ErrorIs(t, err, retry2.ErrNoQuota) }) t.Run("db.Table().Do", func(t *testing.T) { err := nativeDriver.Table().Do(ctx, func(ctx context.Context, s table.Session) error { return retry.RetryableError(errors.New("custom error")) - }, table.WithLimiter(retryLimiter)) - require.ErrorIs(t, err, retry.ErrNoQuota) + }, table.WithBudget(retryLimiter)) + require.ErrorIs(t, err, retry2.ErrNoQuota) }) t.Run("db.Table().DoTx", func(t *testing.T) { err := nativeDriver.Table().DoTx(ctx, func(ctx context.Context, tx table.TransactionActor) error { return retry.RetryableError(errors.New("custom error")) - }, table.WithLimiter(retryLimiter)) - require.ErrorIs(t, err, retry.ErrNoQuota) + }, table.WithBudget(retryLimiter)) + require.ErrorIs(t, err, retry2.ErrNoQuota) }) if version.Gte(os.Getenv("YDB_VERSION"), "24.1") { t.Run("db.Query().Do", func(t *testing.T) { err := nativeDriver.Query().Do(ctx, func(ctx context.Context, s query.Session) error { return retry.RetryableError(errors.New("custom error")) - }, query.WithLimiter(retryLimiter)) - require.ErrorIs(t, err, retry.ErrNoQuota) + }, query.WithBudget(retryLimiter)) + require.ErrorIs(t, err, retry2.ErrNoQuota) }) t.Run("db.Query().DoTx", func(t *testing.T) { err := nativeDriver.Query().DoTx(ctx, func(ctx context.Context, tx query.TxActor) error { return retry.RetryableError(errors.New("custom error")) - }, query.WithLimiter(retryLimiter)) - require.ErrorIs(t, err, retry.ErrNoQuota) + }, query.WithBudget(retryLimiter)) + require.ErrorIs(t, err, retry2.ErrNoQuota) }) } } diff --git a/tests/slo/database/sql/storage.go b/tests/slo/database/sql/storage.go index 9c0cbc0e7..7e0de6736 100755 --- a/tests/slo/database/sql/storage.go +++ b/tests/slo/database/sql/storage.go @@ -8,6 +8,7 @@ import ( "time" ydb "github.com/ydb-platform/ydb-go-sdk/v3" + retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/table" "github.com/ydb-platform/ydb-go-sdk/v3/trace" @@ -66,37 +67,41 @@ var ( ) type Storage struct { - cc *ydb.Driver - c ydb.SQLConnector - db *sql.DB - cfg *config.Config - createQuery string - dropQuery string - upsertQuery string - selectQuery string - retryLimiter retry.LimiterStoper + cc *ydb.Driver + c ydb.SQLConnector + db *sql.DB + cfg *config.Config + createQuery string + dropQuery string + upsertQuery string + selectQuery string + retryBudget interface { + retry2.Budget + + Stop() + } } func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (s *Storage, err error) { ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryLimiter := retry.Quoter(int(float64(poolSize) * 0.1)) + retryBudget := retry.Budget(int(float64(poolSize) * 0.1)) s = &Storage{ cfg: cfg, createQuery: fmt.Sprintf(createTemplate, cfg.Table, cfg.PartitionSize, cfg.MinPartitionsCount, cfg.MaxPartitionsCount, cfg.MinPartitionsCount), - dropQuery: fmt.Sprintf(dropTemplate, cfg.Table), - upsertQuery: fmt.Sprintf(upsertTemplate, cfg.Table), - selectQuery: fmt.Sprintf(selectTemplate, cfg.Table), - retryLimiter: retryLimiter, + dropQuery: fmt.Sprintf(dropTemplate, cfg.Table), + upsertQuery: fmt.Sprintf(upsertTemplate, cfg.Table), + selectQuery: fmt.Sprintf(selectTemplate, cfg.Table), + retryBudget: retryBudget, } s.cc, err = ydb.Open( ctx, s.cfg.Endpoint+s.cfg.DB, - ydb.WithRetryLimiter(retryLimiter), + ydb.WithRetryBudget(retryBudget), ) if err != nil { return nil, fmt.Errorf("ydb.Open error: %w", err) @@ -229,7 +234,7 @@ func (s *Storage) dropTable(ctx context.Context) error { } func (s *Storage) close(ctx context.Context) error { - s.retryLimiter.Stop() + s.retryBudget.Stop() if err := ctx.Err(); err != nil { return err diff --git a/tests/slo/native/query/storage.go b/tests/slo/native/query/storage.go index 3760d6608..21d5bafaa 100755 --- a/tests/slo/native/query/storage.go +++ b/tests/slo/native/query/storage.go @@ -9,6 +9,7 @@ import ( "time" ydb "github.com/ydb-platform/ydb-go-sdk/v3" + retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/query" "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/trace" @@ -18,10 +19,14 @@ import ( ) type Storage struct { - db *ydb.Driver - cfg *config.Config - tablePath string - retryLimiter retry.LimiterStoper + db *ydb.Driver + cfg *config.Config + tablePath string + retryBudget interface { + retry2.Budget + + Stop() + } } const writeQuery = ` @@ -69,12 +74,12 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryLimiter := retry.Quoter(int(float64(poolSize) * 0.1)) + retryBudget := retry.Budget(int(float64(poolSize) * 0.1)) db, err := ydb.Open(ctx, cfg.Endpoint+cfg.DB, ydb.WithSessionPoolSizeLimit(poolSize), - ydb.WithRetryLimiter(retryLimiter), + ydb.WithRetryBudget(retryBudget), ) if err != nil { return nil, err @@ -83,10 +88,10 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage prefix := path.Join(db.Name(), label) s := &Storage{ - db: db, - cfg: cfg, - tablePath: "`" + path.Join(prefix, cfg.Table) + "`", - retryLimiter: retryLimiter, + db: db, + cfg: cfg, + tablePath: "`" + path.Join(prefix, cfg.Table) + "`", + retryBudget: retryBudget, } return s, nil @@ -257,7 +262,7 @@ func (s *Storage) dropTable(ctx context.Context) error { } func (s *Storage) close(ctx context.Context) error { - s.retryLimiter.Stop() + s.retryBudget.Stop() var ( shutdownCtx context.Context diff --git a/tests/slo/native/table/storage.go b/tests/slo/native/table/storage.go index c8777e729..4e91947e5 100755 --- a/tests/slo/native/table/storage.go +++ b/tests/slo/native/table/storage.go @@ -7,6 +7,7 @@ import ( "time" "github.com/ydb-platform/ydb-go-sdk/v3" + retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/table" "github.com/ydb-platform/ydb-go-sdk/v3/table/options" @@ -58,25 +59,29 @@ var ( ) type Storage struct { - db *ydb.Driver - cfg *config.Config - prefix string - upsertQuery string - selectQuery string - retryLimiter retry.LimiterStoper + db *ydb.Driver + cfg *config.Config + prefix string + upsertQuery string + selectQuery string + retryBudget interface { + retry2.Budget + + Stop() + } } func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage, error) { ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryLimiter := retry.Quoter(int(float64(poolSize) * 0.1)) + retryBudget := retry.Budget(int(float64(poolSize) * 0.1)) db, err := ydb.Open( ctx, cfg.Endpoint+cfg.DB, ydb.WithSessionPoolSizeLimit(poolSize), - ydb.WithRetryLimiter(retryLimiter), + ydb.WithRetryBudget(retryBudget), ) if err != nil { return nil, err @@ -85,12 +90,12 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage prefix := path.Join(db.Name(), label) s := &Storage{ - db: db, - cfg: cfg, - prefix: prefix, - upsertQuery: fmt.Sprintf(upsertTemplate, prefix, cfg.Table), - selectQuery: fmt.Sprintf(selectTemplate, prefix, cfg.Table), - retryLimiter: retryLimiter, + db: db, + cfg: cfg, + prefix: prefix, + upsertQuery: fmt.Sprintf(upsertTemplate, prefix, cfg.Table), + selectQuery: fmt.Sprintf(selectTemplate, prefix, cfg.Table), + retryBudget: retryBudget, } return s, nil @@ -251,7 +256,7 @@ func (s *Storage) dropTable(ctx context.Context) error { } func (s *Storage) close(ctx context.Context) error { - s.retryLimiter.Stop() + s.retryBudget.Stop() ctx, cancel := context.WithTimeout(ctx, time.Duration(s.cfg.ShutdownTime)*time.Second) defer cancel() From 862d8f4c15edc8324429c2c211cfbc931c08af26 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 22 Apr 2024 15:59:52 +0300 Subject: [PATCH 25/33] refactoring of retry budget sources --- config/config.go | 6 ++-- internal/balancer/balancer.go | 2 +- internal/config/config.go | 19 +++++------ internal/coordination/client.go | 8 ++--- internal/query/options/retry.go | 6 ++-- internal/ratelimiter/client.go | 12 +++---- internal/scheme/client.go | 10 +++--- internal/scripting/client.go | 6 ++-- internal/table/retry.go | 2 +- internal/topic/topicclientinternal/client.go | 8 ++--- internal/xsql/conn.go | 2 +- internal/xsql/connector.go | 24 ++++++------- options.go | 8 ++--- query/client.go | 6 ++-- {internal/retry => retry/budget}/budget.go | 8 ++--- .../retry => retry/budget}/budget_test.go | 6 ++-- retry/{budget.go => budget/errors.go} | 12 +------ retry/retry.go | 30 ++++++++-------- retry/sql.go | 6 ++-- sql.go | 2 +- table/table.go | 6 ++-- tests/integration/retry_budget_test.go | 34 +++++++++---------- tests/slo/database/sql/storage.go | 5 ++- tests/slo/native/query/storage.go | 6 ++-- tests/slo/native/table/storage.go | 6 ++-- 25 files changed, 112 insertions(+), 128 deletions(-) rename {internal/retry => retry/budget}/budget.go (90%) rename {internal/retry => retry/budget}/budget_test.go (93%) rename retry/{budget.go => budget/errors.go} (56%) diff --git a/config/config.go b/config/config.go index ac25e4537..370dd283d 100644 --- a/config/config.go +++ b/config/config.go @@ -12,7 +12,7 @@ import ( balancerConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/balancer/config" "github.com/ydb-platform/ydb-go-sdk/v3/internal/config" "github.com/ydb-platform/ydb-go-sdk/v3/internal/meta" - "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -159,9 +159,9 @@ func WithTrace(t trace.Driver, opts ...trace.DriverComposeOption) Option { //nol } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func WithRetryLimiter(l retry.Budget) Option { +func WithRetryBudget(b budget.Budget) Option { return func(c *Config) { - config.SetRetryLimiter(&c.Common, l) + config.SetRetryBudget(&c.Common, b) } } diff --git a/internal/balancer/balancer.go b/internal/balancer/balancer.go index 96cd20db5..b33f7266a 100644 --- a/internal/balancer/balancer.go +++ b/internal/balancer/balancer.go @@ -89,7 +89,7 @@ func (b *Balancer) clusterDiscovery(ctx context.Context) (err error) { }, retry.WithIdempotent(true), retry.WithTrace(b.driverConfig.TraceRetry()), - retry.WithBudget(b.driverConfig.RetryLimiter()), + retry.WithBudget(b.driverConfig.RetryBudget()), ) } diff --git a/internal/config/config.go b/internal/config/config.go index 842239f09..90c71b481 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,19 +3,18 @@ package config import ( "time" - retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" - "github.com/ydb-platform/ydb-go-sdk/v3/retry" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) -var defaultRetryLimiter = retry.Budget(-1) +var defaultRetryBudget = budget.New(-1) type Common struct { operationTimeout time.Duration operationCancelAfter time.Duration disableAutoRetry bool traceRetry trace.Retry - retryLimiter retry2.Budget + retryBudget budget.Budget panicCallback func(e interface{}) } @@ -53,12 +52,12 @@ func (c *Common) TraceRetry() *trace.Retry { return &c.traceRetry } -func (c *Common) RetryLimiter() retry2.Budget { - if c.retryLimiter == nil { - return defaultRetryLimiter +func (c *Common) RetryBudget() budget.Budget { + if c.retryBudget == nil { + return defaultRetryBudget } - return c.retryLimiter + return c.retryBudget } // SetOperationTimeout define the maximum amount of time a YDB server will process @@ -95,6 +94,6 @@ func SetTraceRetry(c *Common, t *trace.Retry, opts ...trace.RetryComposeOption) c.traceRetry = *c.traceRetry.Compose(t, opts...) } -func SetRetryLimiter(c *Common, l retry2.Budget) { - c.retryLimiter = l +func SetRetryBudget(c *Common, b budget.Budget) { + c.retryBudget = b } diff --git a/internal/coordination/client.go b/internal/coordination/client.go index 23e5fa7d3..c3685cc64 100644 --- a/internal/coordination/client.go +++ b/internal/coordination/client.go @@ -117,7 +117,7 @@ func (c *Client) CreateNode(ctx context.Context, path string, config coordinatio retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) } @@ -150,7 +150,7 @@ func (c *Client) AlterNode(ctx context.Context, path string, config coordination retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) } @@ -211,7 +211,7 @@ func (c *Client) DropNode(ctx context.Context, path string) (finalErr error) { retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) } @@ -271,7 +271,7 @@ func (c *Client) DescribeNode( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) if err != nil { return nil, nil, xerrors.WithStackTrace(err) diff --git a/internal/query/options/retry.go b/internal/query/options/retry.go index 870ab442a..a5fe4f8d2 100644 --- a/internal/query/options/retry.go +++ b/internal/query/options/retry.go @@ -2,8 +2,8 @@ package options import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/query/tx" - retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/retry" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -96,8 +96,8 @@ func WithTrace(t *trace.Query) traceOption { return traceOption{t: t} } -func WithBudget(l retry2.Budget) retryOptionsOption { - return []retry.Option{retry.WithBudget(l)} +func WithRetryBudget(b budget.Budget) retryOptionsOption { + return []retry.Option{retry.WithBudget(b)} } func ParseDoOpts(t *trace.Query, opts ...DoOption) (s *doSettings) { diff --git a/internal/ratelimiter/client.go b/internal/ratelimiter/client.go index 19744a949..c1db4c0ce 100644 --- a/internal/ratelimiter/client.go +++ b/internal/ratelimiter/client.go @@ -63,7 +63,7 @@ func (c *Client) CreateResource( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) } @@ -113,7 +113,7 @@ func (c *Client) AlterResource( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) } @@ -163,7 +163,7 @@ func (c *Client) DropResource( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) } @@ -209,7 +209,7 @@ func (c *Client) ListResource( retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) return list, err @@ -269,7 +269,7 @@ func (c *Client) DescribeResource( retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) return @@ -338,7 +338,7 @@ func (c *Client) AcquireResource( return retry.Retry(ctx, call, retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) } diff --git a/internal/scheme/client.go b/internal/scheme/client.go index 8b93ea0d7..154c44eed 100644 --- a/internal/scheme/client.go +++ b/internal/scheme/client.go @@ -64,7 +64,7 @@ func (c *Client) MakeDirectory(ctx context.Context, path string) (finalErr error retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) } @@ -104,7 +104,7 @@ func (c *Client) RemoveDirectory(ctx context.Context, path string) (finalErr err retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) } @@ -146,7 +146,7 @@ func (c *Client) ListDirectory(ctx context.Context, path string) (d scheme.Direc retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) return d, xerrors.WithStackTrace(err) @@ -210,7 +210,7 @@ func (c *Client) DescribePath(ctx context.Context, path string) (e scheme.Entry, retry.WithIdempotent(true), retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) return e, xerrors.WithStackTrace(err) @@ -272,7 +272,7 @@ func (c *Client) ModifyPermissions( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) } diff --git a/internal/scripting/client.go b/internal/scripting/client.go index ecdd664f7..92b88b895 100644 --- a/internal/scripting/client.go +++ b/internal/scripting/client.go @@ -60,7 +60,7 @@ func (c *Client) Execute( err = retry.Retry(ctx, call, retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) return r, xerrors.WithStackTrace(err) @@ -140,7 +140,7 @@ func (c *Client) Explain( retry.WithStackTrace(), retry.WithIdempotent(true), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) return e, xerrors.WithStackTrace(err) @@ -215,7 +215,7 @@ func (c *Client) StreamExecute( err = retry.Retry(ctx, call, retry.WithStackTrace(), retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), ) return r, xerrors.WithStackTrace(err) diff --git a/internal/table/retry.go b/internal/table/retry.go index f385f969f..e2b522b45 100644 --- a/internal/table/retry.go +++ b/internal/table/retry.go @@ -99,7 +99,7 @@ func (c *Client) retryOptions(opts ...table.Option) *table.Options { ), RetryOptions: []retry.Option{ retry.WithTrace(c.config.TraceRetry()), - retry.WithBudget(c.config.RetryLimiter()), + retry.WithBudget(c.config.RetryBudget()), }, } for _, opt := range opts { diff --git a/internal/topic/topicclientinternal/client.go b/internal/topic/topicclientinternal/client.go index ae62d43ba..4609df17a 100644 --- a/internal/topic/topicclientinternal/client.go +++ b/internal/topic/topicclientinternal/client.go @@ -87,7 +87,7 @@ func (c *Client) Alter(ctx context.Context, path string, opts ...topicoptions.Al return retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), - retry.WithBudget(c.cfg.RetryLimiter()), + retry.WithBudget(c.cfg.RetryBudget()), ) } @@ -120,7 +120,7 @@ func (c *Client) Create( return retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), - retry.WithBudget(c.cfg.RetryLimiter()), + retry.WithBudget(c.cfg.RetryBudget()), ) } @@ -158,7 +158,7 @@ func (c *Client) Describe( err = retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), - retry.WithBudget(c.cfg.RetryLimiter()), + retry.WithBudget(c.cfg.RetryBudget()), ) } else { err = call(ctx) @@ -195,7 +195,7 @@ func (c *Client) Drop(ctx context.Context, path string, opts ...topicoptions.Dro return retry.Retry(ctx, call, retry.WithIdempotent(true), retry.WithTrace(c.cfg.TraceRetry()), - retry.WithBudget(c.cfg.RetryLimiter()), + retry.WithBudget(c.cfg.RetryBudget()), ) } diff --git a/internal/xsql/conn.go b/internal/xsql/conn.go index 211ff3d07..42d5517e1 100644 --- a/internal/xsql/conn.go +++ b/internal/xsql/conn.go @@ -831,7 +831,7 @@ func (c *conn) retryIdempotent(ctx context.Context, f func(ctx context.Context) err := retry.Retry(ctx, f, retry.WithIdempotent(true), retry.WithTrace(c.connector.traceRetry), - retry.WithBudget(c.connector.retryLimiter), + retry.WithBudget(c.connector.retryBudget), ) if err != nil { return xerrors.WithStackTrace(err) diff --git a/internal/xsql/connector.go b/internal/xsql/connector.go index a934b376c..f097c6954 100644 --- a/internal/xsql/connector.go +++ b/internal/xsql/connector.go @@ -11,11 +11,11 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/bind" metaHeaders "github.com/ydb-platform/ydb-go-sdk/v3/internal/meta" - "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/internal/stack" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" "github.com/ydb-platform/ydb-go-sdk/v3/meta" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/scheme" "github.com/ydb-platform/ydb-go-sdk/v3/scripting" "github.com/ydb-platform/ydb-go-sdk/v3/table" @@ -176,18 +176,18 @@ func WithTraceRetry(t *trace.Retry) ConnectorOption { return traceRetryConnectorOption{t: t} } -type retryLimiterConnectorOption struct { - l retry.Budget +type retryBudgetConnectorOption struct { + b budget.Budget } -func (l retryLimiterConnectorOption) Apply(c *Connector) error { - c.retryLimiter = l.l +func (l retryBudgetConnectorOption) Apply(c *Connector) error { + c.retryBudget = l.b return nil } -func WithRetryLimiter(l retry.Budget) ConnectorOption { - return retryLimiterConnectorOption{l: l} +func WithretryBudget(b budget.Budget) ConnectorOption { + return retryBudgetConnectorOption{b: b} } type fakeTxConnectorOption QueryMode @@ -263,9 +263,9 @@ type Connector struct { disableServerBalancer bool idleThreshold time.Duration - trace *trace.DatabaseSQL - traceRetry *trace.Retry - retryLimiter retry.Budget + trace *trace.DatabaseSQL + traceRetry *trace.Retry + retryBudget budget.Budget } var ( @@ -369,8 +369,8 @@ func (d *driverWrapper) TraceRetry() *trace.Retry { return d.c.traceRetry } -func (d *driverWrapper) RetryLimiter() retry.Budget { - return d.c.retryLimiter +func (d *driverWrapper) RetryBudget() budget.Budget { + return d.c.retryBudget } func (d *driverWrapper) Open(_ string) (driver.Conn, error) { diff --git a/options.go b/options.go index 39e863713..8dcce2969 100644 --- a/options.go +++ b/options.go @@ -19,13 +19,13 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/dsn" queryConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/query/config" ratelimiterConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/ratelimiter/config" - "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" schemeConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/scheme/config" scriptingConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/scripting/config" tableConfig "github.com/ydb-platform/ydb-go-sdk/v3/internal/table/config" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xsql" "github.com/ydb-platform/ydb-go-sdk/v3/log" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/topic/topicoptions" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -295,12 +295,12 @@ func WithDiscoveryInterval(discoveryInterval time.Duration) Option { } } -// WithRetryBudget sets retry limiter for all calls of all retryers. +// WithRetryBudget sets retry budget for all calls of all retryers. // // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func WithRetryBudget(l retry.Budget) Option { +func WithRetryBudget(b budget.Budget) Option { return func(ctx context.Context, c *Driver) error { - c.options = append(c.options, config.WithRetryLimiter(l)) + c.options = append(c.options, config.WithRetryBudget(b)) return nil } diff --git a/query/client.go b/query/client.go index e3269eb9c..e53bda415 100644 --- a/query/client.go +++ b/query/client.go @@ -5,7 +5,7 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/closer" "github.com/ydb-platform/ydb-go-sdk/v3/internal/query/options" - "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -69,6 +69,6 @@ func WithLabel(lbl string) bothDoAndDoTxOption { } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func WithBudget(l retry.Budget) bothDoAndDoTxOption { - return options.WithBudget(l) +func WithRetryBudget(b budget.Budget) bothDoAndDoTxOption { + return options.WithRetryBudget(b) } diff --git a/internal/retry/budget.go b/retry/budget/budget.go similarity index 90% rename from internal/retry/budget.go rename to retry/budget/budget.go index e2a954d50..c0fa1eb4e 100644 --- a/internal/retry/budget.go +++ b/retry/budget/budget.go @@ -1,4 +1,4 @@ -package retry +package budget import ( "context" @@ -20,17 +20,17 @@ type ( quota chan struct{} done chan struct{} } - BudgetOption func(q *budget) + option func(q *budget) ) -func WithBudgetClock(clock clockwork.Clock) BudgetOption { +func withBudgetClock(clock clockwork.Clock) option { return func(q *budget) { q.clock = clock } } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func NewBudget(attemptsPerSecond int, opts ...BudgetOption) *budget { +func New(attemptsPerSecond int, opts ...option) *budget { q := &budget{ clock: clockwork.NewRealClock(), done: make(chan struct{}), diff --git a/internal/retry/budget_test.go b/retry/budget/budget_test.go similarity index 93% rename from internal/retry/budget_test.go rename to retry/budget/budget_test.go index 714f8e807..534f1912c 100644 --- a/internal/retry/budget_test.go +++ b/retry/budget/budget_test.go @@ -1,4 +1,4 @@ -package retry +package budget import ( "context" @@ -15,7 +15,7 @@ import ( func TestUnlimitedBudget(t *testing.T) { xtest.TestManyTimes(t, func(t testing.TB) { ctx, cancel := xcontext.WithCancel(xtest.Context(t)) - q := NewBudget(-1) + q := New(-1) require.NoError(t, q.Acquire(ctx)) cancel() require.ErrorIs(t, q.Acquire(ctx), context.Canceled) @@ -26,7 +26,7 @@ func TestBudget(t *testing.T) { xtest.TestManyTimes(t, func(t testing.TB) { ctx, cancel := xcontext.WithCancel(xtest.Context(t)) clock := clockwork.NewFakeClock() - q := NewBudget(1, WithBudgetClock(clock)) + q := New(1, withBudgetClock(clock)) defer q.Stop() require.NoError(t, q.Acquire(ctx)) acquireCh := make(chan struct{}) diff --git a/retry/budget.go b/retry/budget/errors.go similarity index 56% rename from retry/budget.go rename to retry/budget/errors.go index fa57b3947..d0f2ace13 100644 --- a/retry/budget.go +++ b/retry/budget/errors.go @@ -1,9 +1,8 @@ -package retry +package budget import ( "errors" - "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" ) @@ -11,12 +10,3 @@ import ( // // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental var ErrNoQuota = xerrors.Wrap(errors.New("no retry quota")) - -type budget interface { - retry.Budget - Stop() -} - -func Budget(attemptsPerSecond int, opts ...retry.BudgetOption) budget { - return retry.NewBudget(attemptsPerSecond, opts...) -} diff --git a/retry/retry.go b/retry/retry.go index 636c0dc8b..e4be3d4a7 100644 --- a/retry/retry.go +++ b/retry/retry.go @@ -6,10 +6,10 @@ import ( "time" "github.com/ydb-platform/ydb-go-sdk/v3/internal/backoff" - "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/internal/stack" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -26,7 +26,7 @@ type retryOptions struct { stackTrace bool fastBackoff backoff.Backoff slowBackoff backoff.Backoff - budget retry.Budget + budget budget.Budget panicCallback func(e interface{}) } @@ -126,29 +126,29 @@ func WithTrace(t *trace.Retry) traceOption { return traceOption{t: t} } -var _ Option = limiterOption{} +var _ Option = budgetOption{} -type limiterOption struct { - l retry.Budget +type budgetOption struct { + b budget.Budget } -func (l limiterOption) ApplyRetryOption(opts *retryOptions) { - opts.budget = l.l +func (b budgetOption) ApplyRetryOption(opts *retryOptions) { + opts.budget = b.b } -func (l limiterOption) ApplyDoOption(opts *doOptions) { - opts.retryOptions = append(opts.retryOptions, WithBudget(l.l)) +func (b budgetOption) ApplyDoOption(opts *doOptions) { + opts.retryOptions = append(opts.retryOptions, WithBudget(b.b)) } -func (l limiterOption) ApplyDoTxOption(opts *doTxOptions) { - opts.retryOptions = append(opts.retryOptions, WithBudget(l.l)) +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 -func WithBudget(l retry.Budget) limiterOption { - return limiterOption{l: l} +func WithBudget(b budget.Budget) budgetOption { + return budgetOption{b: b} } var _ Option = idempotentOption(false) @@ -261,7 +261,7 @@ func Retry(ctx context.Context, op retryOperation, opts ...Option) (finalErr err options := &retryOptions{ call: stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/retry.Retry"), trace: &trace.Retry{}, - budget: Budget(-1), + budget: budget.New(-1), fastBackoff: backoff.Fast, slowBackoff: backoff.Slow, } @@ -362,7 +362,7 @@ func Retry(ctx context.Context, op retryOperation, opts ...Option) (finalErr err if acquireErr := options.budget.Acquire(ctx); acquireErr != nil { return xerrors.WithStackTrace( xerrors.Join( - fmt.Errorf("attempt No.%d: %w", attempts, ErrNoQuota), + fmt.Errorf("attempt No.%d: %w", attempts, budget.ErrNoQuota), acquireErr, err, ), diff --git a/retry/sql.go b/retry/sql.go index 7ceeb1a02..23706971e 100644 --- a/retry/sql.go +++ b/retry/sql.go @@ -5,10 +5,10 @@ import ( "database/sql" "fmt" - "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/internal/stack" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xcontext" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" + budget "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) @@ -145,12 +145,12 @@ func DoTx(ctx context.Context, db *sql.DB, op func(context.Context, *sql.Tx) err ) if d, has := db.Driver().(interface { TraceRetry() *trace.Retry - RetryLimiter() retry.Budget + 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.RetryLimiter()) + options.retryOptions[1] = WithBudget(d.RetryBudget()) } for _, opt := range opts { if opt != nil { diff --git a/sql.go b/sql.go index 8a5d8ed47..efe7cf92e 100644 --- a/sql.go +++ b/sql.go @@ -167,7 +167,7 @@ func Connector(parent *Driver, opts ...ConnectorOption) (SQLConnector, error) { ), xsql.WithOnClose(d.detach), xsql.WithTraceRetry(parent.config.TraceRetry()), - xsql.WithRetryLimiter(parent.config.RetryLimiter()), + xsql.WithretryBudget(parent.config.RetryBudget()), )..., ) if err != nil { diff --git a/table/table.go b/table/table.go index b19c63ce0..86e110040 100644 --- a/table/table.go +++ b/table/table.go @@ -8,10 +8,10 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/closer" "github.com/ydb-platform/ydb-go-sdk/v3/internal/params" - retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/internal/types" "github.com/ydb-platform/ydb-go-sdk/v3/internal/value" "github.com/ydb-platform/ydb-go-sdk/v3/retry" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/table/options" "github.com/ydb-platform/ydb-go-sdk/v3/table/result" "github.com/ydb-platform/ydb-go-sdk/v3/trace" @@ -525,8 +525,8 @@ func (retryOptions retryOptionsOption) ApplyTableOption(opts *Options) { } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func WithBudget(l retry2.Budget) retryOptionsOption { - return []retry.Option{retry.WithBudget(l)} +func WithRetryBudget(b budget.Budget) retryOptionsOption { + return []retry.Option{retry.WithBudget(b)} } // Deprecated: redundant option diff --git a/tests/integration/retry_budget_test.go b/tests/integration/retry_budget_test.go index b959ff9e0..0f733095d 100644 --- a/tests/integration/retry_budget_test.go +++ b/tests/integration/retry_budget_test.go @@ -14,11 +14,11 @@ import ( "github.com/stretchr/testify/require" "github.com/ydb-platform/ydb-go-sdk/v3" - retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/internal/version" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xtest" "github.com/ydb-platform/ydb-go-sdk/v3/query" "github.com/ydb-platform/ydb-go-sdk/v3/retry" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/table" ) @@ -31,7 +31,7 @@ func (n noQuota) Acquire(ctx context.Context) error { func TestRetryBudget(t *testing.T) { ctx := xtest.Context(t) - defaultLimiter := retry.Budget(1) + defaultLimiter := budget.New(1) defer defaultLimiter.Stop() nativeDriver, err := ydb.Open(ctx, os.Getenv("YDB_CONNECTION_STRING"), @@ -59,50 +59,50 @@ func TestRetryBudget(t *testing.T) { _ = db.Close() }() - retryLimiter := noQuota{} + retryBudget := noQuota{} t.Run("retry.Retry", func(t *testing.T) { err := retry.Retry(ctx, func(ctx context.Context) (err error) { return retry.RetryableError(errors.New("custom error")) - }, retry.WithBudget(retryLimiter)) - require.ErrorIs(t, err, retry2.ErrNoQuota) + }, retry.WithBudget(retryBudget)) + require.ErrorIs(t, err, budget.ErrNoQuota) }) t.Run("retry.Do", func(t *testing.T) { err := retry.Do(ctx, db, func(ctx context.Context, cc *sql.Conn) (err error) { return retry.RetryableError(errors.New("custom error")) - }, retry.WithBudget(retryLimiter)) - require.ErrorIs(t, err, retry2.ErrNoQuota) + }, retry.WithBudget(retryBudget)) + require.ErrorIs(t, err, budget.ErrNoQuota) }) t.Run("retry.DoTx", func(t *testing.T) { err := retry.DoTx(ctx, db, func(ctx context.Context, tx *sql.Tx) (err error) { return retry.RetryableError(errors.New("custom error")) - }, retry.WithBudget(retryLimiter)) - require.ErrorIs(t, err, retry2.ErrNoQuota) + }, retry.WithBudget(retryBudget)) + require.ErrorIs(t, err, budget.ErrNoQuota) }) t.Run("db.Table().Do", func(t *testing.T) { err := nativeDriver.Table().Do(ctx, func(ctx context.Context, s table.Session) error { return retry.RetryableError(errors.New("custom error")) - }, table.WithBudget(retryLimiter)) - require.ErrorIs(t, err, retry2.ErrNoQuota) + }, table.WithRetryBudget(retryBudget)) + require.ErrorIs(t, err, budget.ErrNoQuota) }) t.Run("db.Table().DoTx", func(t *testing.T) { err := nativeDriver.Table().DoTx(ctx, func(ctx context.Context, tx table.TransactionActor) error { return retry.RetryableError(errors.New("custom error")) - }, table.WithBudget(retryLimiter)) - require.ErrorIs(t, err, retry2.ErrNoQuota) + }, table.WithRetryBudget(retryBudget)) + require.ErrorIs(t, err, budget.ErrNoQuota) }) if version.Gte(os.Getenv("YDB_VERSION"), "24.1") { t.Run("db.Query().Do", func(t *testing.T) { err := nativeDriver.Query().Do(ctx, func(ctx context.Context, s query.Session) error { return retry.RetryableError(errors.New("custom error")) - }, query.WithBudget(retryLimiter)) - require.ErrorIs(t, err, retry2.ErrNoQuota) + }, query.WithRetryBudget(retryBudget)) + require.ErrorIs(t, err, budget.ErrNoQuota) }) t.Run("db.Query().DoTx", func(t *testing.T) { err := nativeDriver.Query().DoTx(ctx, func(ctx context.Context, tx query.TxActor) error { return retry.RetryableError(errors.New("custom error")) - }, query.WithBudget(retryLimiter)) - require.ErrorIs(t, err, retry2.ErrNoQuota) + }, query.WithRetryBudget(retryBudget)) + require.ErrorIs(t, err, budget.ErrNoQuota) }) } } diff --git a/tests/slo/database/sql/storage.go b/tests/slo/database/sql/storage.go index 7e0de6736..410841f2f 100755 --- a/tests/slo/database/sql/storage.go +++ b/tests/slo/database/sql/storage.go @@ -8,7 +8,6 @@ import ( "time" ydb "github.com/ydb-platform/ydb-go-sdk/v3" - retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/table" "github.com/ydb-platform/ydb-go-sdk/v3/trace" @@ -76,7 +75,7 @@ type Storage struct { upsertQuery string selectQuery string retryBudget interface { - retry2.Budget + budget.Budget Stop() } @@ -86,7 +85,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (s *Stora ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryBudget := retry.Budget(int(float64(poolSize) * 0.1)) + retryBudget := budget.New(int(float64(poolSize) * 0.1)) s = &Storage{ cfg: cfg, diff --git a/tests/slo/native/query/storage.go b/tests/slo/native/query/storage.go index 21d5bafaa..c502c5f9a 100755 --- a/tests/slo/native/query/storage.go +++ b/tests/slo/native/query/storage.go @@ -9,9 +9,7 @@ import ( "time" ydb "github.com/ydb-platform/ydb-go-sdk/v3" - retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" "github.com/ydb-platform/ydb-go-sdk/v3/query" - "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/trace" "slo/internal/config" @@ -23,7 +21,7 @@ type Storage struct { cfg *config.Config tablePath string retryBudget interface { - retry2.Budget + budget.Budget Stop() } @@ -74,7 +72,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryBudget := retry.Budget(int(float64(poolSize) * 0.1)) + retryBudget := budget.New(int(float64(poolSize) * 0.1)) db, err := ydb.Open(ctx, cfg.Endpoint+cfg.DB, diff --git a/tests/slo/native/table/storage.go b/tests/slo/native/table/storage.go index 4e91947e5..c51eade67 100755 --- a/tests/slo/native/table/storage.go +++ b/tests/slo/native/table/storage.go @@ -7,8 +7,6 @@ import ( "time" "github.com/ydb-platform/ydb-go-sdk/v3" - retry2 "github.com/ydb-platform/ydb-go-sdk/v3/internal/retry" - "github.com/ydb-platform/ydb-go-sdk/v3/retry" "github.com/ydb-platform/ydb-go-sdk/v3/table" "github.com/ydb-platform/ydb-go-sdk/v3/table/options" "github.com/ydb-platform/ydb-go-sdk/v3/table/result" @@ -65,7 +63,7 @@ type Storage struct { upsertQuery string selectQuery string retryBudget interface { - retry2.Budget + budget.Budget Stop() } @@ -75,7 +73,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryBudget := retry.Budget(int(float64(poolSize) * 0.1)) + retryBudget := budget.New(int(float64(poolSize) * 0.1)) db, err := ydb.Open( ctx, From df47a308dadac2fed70c931b0ec7f20f337218de Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 22 Apr 2024 16:06:01 +0300 Subject: [PATCH 26/33] fix slo --- tests/slo/database/sql/storage.go | 1 + tests/slo/native/query/storage.go | 1 + tests/slo/native/table/storage.go | 1 + 3 files changed, 3 insertions(+) diff --git a/tests/slo/database/sql/storage.go b/tests/slo/database/sql/storage.go index 410841f2f..73fb78c11 100755 --- a/tests/slo/database/sql/storage.go +++ b/tests/slo/database/sql/storage.go @@ -9,6 +9,7 @@ import ( ydb "github.com/ydb-platform/ydb-go-sdk/v3" "github.com/ydb-platform/ydb-go-sdk/v3/retry" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/table" "github.com/ydb-platform/ydb-go-sdk/v3/trace" diff --git a/tests/slo/native/query/storage.go b/tests/slo/native/query/storage.go index c502c5f9a..aa409c7c3 100755 --- a/tests/slo/native/query/storage.go +++ b/tests/slo/native/query/storage.go @@ -10,6 +10,7 @@ import ( ydb "github.com/ydb-platform/ydb-go-sdk/v3" "github.com/ydb-platform/ydb-go-sdk/v3/query" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/trace" "slo/internal/config" diff --git a/tests/slo/native/table/storage.go b/tests/slo/native/table/storage.go index c51eade67..3ba410372 100755 --- a/tests/slo/native/table/storage.go +++ b/tests/slo/native/table/storage.go @@ -7,6 +7,7 @@ import ( "time" "github.com/ydb-platform/ydb-go-sdk/v3" + "github.com/ydb-platform/ydb-go-sdk/v3/retry/budget" "github.com/ydb-platform/ydb-go-sdk/v3/table" "github.com/ydb-platform/ydb-go-sdk/v3/table/options" "github.com/ydb-platform/ydb-go-sdk/v3/table/result" From 294e9e24a06efa261756a15fcbc1113b8788d656 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Tue, 23 Apr 2024 12:51:03 +0300 Subject: [PATCH 27/33] CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3545dff8..ad7b3f038 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,4 @@ +* Added experimental package `retry/budget` for limit second and subsequent retry attempts * Refactored internals for enabling `containedctx` linter * Fixed the hanging semaphore issue on coordination session reconnect From 17ef84e50badc482d58cb45170a23a0d474c7531 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Mon, 22 Apr 2024 16:11:53 +0300 Subject: [PATCH 28/33] fixed gomnd linter issues in SLO tests --- tests/slo/database/sql/storage.go | 2 +- tests/slo/native/query/storage.go | 2 +- tests/slo/native/table/storage.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/slo/database/sql/storage.go b/tests/slo/database/sql/storage.go index 73fb78c11..633580bfd 100755 --- a/tests/slo/database/sql/storage.go +++ b/tests/slo/database/sql/storage.go @@ -86,7 +86,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (s *Stora ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryBudget := budget.New(int(float64(poolSize) * 0.1)) + retryBudget := budget.New(int(float64(poolSize) * 0.1)) //nolint:gomnd s = &Storage{ cfg: cfg, diff --git a/tests/slo/native/query/storage.go b/tests/slo/native/query/storage.go index aa409c7c3..1eaa00006 100755 --- a/tests/slo/native/query/storage.go +++ b/tests/slo/native/query/storage.go @@ -73,7 +73,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryBudget := budget.New(int(float64(poolSize) * 0.1)) + retryBudget := budget.New(int(float64(poolSize) * 0.1)) //nolint:gomnd db, err := ydb.Open(ctx, cfg.Endpoint+cfg.DB, diff --git a/tests/slo/native/table/storage.go b/tests/slo/native/table/storage.go index 3ba410372..df4af1f44 100755 --- a/tests/slo/native/table/storage.go +++ b/tests/slo/native/table/storage.go @@ -74,7 +74,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryBudget := budget.New(int(float64(poolSize) * 0.1)) + retryBudget := budget.New(int(float64(poolSize) * 0.1)) //nolint:gomnd db, err := ydb.Open( ctx, From 1484dbb5febcdbd4700fdcda5846d1cc0a7c6884 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Tue, 23 Apr 2024 12:14:49 +0300 Subject: [PATCH 29/33] fixes from PR review --- retry/budget/budget.go | 16 ++++++++-------- retry/budget/errors.go | 12 ++++++++---- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/retry/budget/budget.go b/retry/budget/budget.go index c0fa1eb4e..53fbf43b0 100644 --- a/retry/budget/budget.go +++ b/retry/budget/budget.go @@ -77,15 +77,15 @@ func (q *budget) Stop() { // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental func (q *budget) Acquire(ctx context.Context) error { + if err := ctx.Err(); err != nil { + return xerrors.WithStackTrace(err) + } select { + case <-q.done: + return xerrors.WithStackTrace(errClosedBudget) + case <-q.quota: + return nil case <-ctx.Done(): - return ctx.Err() - default: - select { - case <-q.quota: - return nil - case <-ctx.Done(): - return xerrors.WithStackTrace(ctx.Err()) - } + return xerrors.WithStackTrace(ctx.Err()) } } diff --git a/retry/budget/errors.go b/retry/budget/errors.go index d0f2ace13..0eeb206f5 100644 --- a/retry/budget/errors.go +++ b/retry/budget/errors.go @@ -6,7 +6,11 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" ) -// ErrNoQuota is a special error for no quota provided by external retry budget -// -// Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -var ErrNoQuota = xerrors.Wrap(errors.New("no retry quota")) +var ( + // ErrNoQuota is a special error for no quota provided by external retry budget + // + // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental + ErrNoQuota = xerrors.Wrap(errors.New("no retry quota")) + + errClosedBudget = xerrors.Wrap(errors.New("retry budget closed")) +) From f5814b5c93a7856e5cdc50ef677c92eee50da3c7 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Tue, 23 Apr 2024 12:42:29 +0300 Subject: [PATCH 30/33] renamed `budget.New()` => `budget.Limited()` added `budget.Percent()` --- internal/config/config.go | 2 +- retry/budget/budget.go | 42 +++++++++++++++++++++----- retry/budget/budget_test.go | 25 +++++++++++++-- retry/retry.go | 2 +- tests/integration/retry_budget_test.go | 2 +- tests/slo/database/sql/storage.go | 2 +- tests/slo/native/query/storage.go | 2 +- tests/slo/native/table/storage.go | 2 +- 8 files changed, 62 insertions(+), 17 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 90c71b481..051571bbd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -7,7 +7,7 @@ import ( "github.com/ydb-platform/ydb-go-sdk/v3/trace" ) -var defaultRetryBudget = budget.New(-1) +var defaultRetryBudget = budget.Limited(-1) type Common struct { operationTimeout time.Duration diff --git a/retry/budget/budget.go b/retry/budget/budget.go index 53fbf43b0..dfe84fde9 100644 --- a/retry/budget/budget.go +++ b/retry/budget/budget.go @@ -2,36 +2,43 @@ package budget import ( "context" + "fmt" "time" "github.com/jonboulle/clockwork" "github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors" + "github.com/ydb-platform/ydb-go-sdk/v3/internal/xrand" ) type ( // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental Budget interface { + // Acquire called from retryer on second and subsequence attempts Acquire(ctx context.Context) error } - budget struct { + fixedBudget struct { clock clockwork.Clock ticker clockwork.Ticker quota chan struct{} done chan struct{} } - option func(q *budget) + fixedBudgetOption func(q *fixedBudget) + percentBudget struct { + percent int + rand xrand.Rand + } ) -func withBudgetClock(clock clockwork.Clock) option { - return func(q *budget) { +func withFixedBudgetClock(clock clockwork.Clock) fixedBudgetOption { + return func(q *fixedBudget) { q.clock = clock } } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func New(attemptsPerSecond int, opts ...option) *budget { - q := &budget{ +func Limited(attemptsPerSecond int, opts ...fixedBudgetOption) *fixedBudget { + q := &fixedBudget{ clock: clockwork.NewRealClock(), done: make(chan struct{}), } @@ -68,7 +75,7 @@ func New(attemptsPerSecond int, opts ...option) *budget { } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func (q *budget) Stop() { +func (q *fixedBudget) Stop() { if q.ticker != nil { q.ticker.Stop() } @@ -76,7 +83,7 @@ func (q *budget) Stop() { } // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental -func (q *budget) Acquire(ctx context.Context) error { +func (q *fixedBudget) Acquire(ctx context.Context) error { if err := ctx.Err(); err != nil { return xerrors.WithStackTrace(err) } @@ -89,3 +96,22 @@ func (q *budget) Acquire(ctx context.Context) error { return xerrors.WithStackTrace(ctx.Err()) } } + +// Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental +func Percent(percent int) *percentBudget { + if percent > 100 || percent < 0 { + panic(fmt.Sprintf("wrong percent value: %d", percent)) + } + return &percentBudget{ + percent: percent, + rand: xrand.New(xrand.WithLock()), + } +} + +func (b *percentBudget) Acquire(ctx context.Context) error { + if b.rand.Int(100) < b.percent { + return nil + } + + return ErrNoQuota +} diff --git a/retry/budget/budget_test.go b/retry/budget/budget_test.go index 534f1912c..5850a3ee7 100644 --- a/retry/budget/budget_test.go +++ b/retry/budget/budget_test.go @@ -15,18 +15,18 @@ import ( func TestUnlimitedBudget(t *testing.T) { xtest.TestManyTimes(t, func(t testing.TB) { ctx, cancel := xcontext.WithCancel(xtest.Context(t)) - q := New(-1) + q := Limited(-1) require.NoError(t, q.Acquire(ctx)) cancel() require.ErrorIs(t, q.Acquire(ctx), context.Canceled) }) } -func TestBudget(t *testing.T) { +func TestLimited(t *testing.T) { xtest.TestManyTimes(t, func(t testing.TB) { ctx, cancel := xcontext.WithCancel(xtest.Context(t)) clock := clockwork.NewFakeClock() - q := New(1, withBudgetClock(clock)) + q := Limited(1, withFixedBudgetClock(clock)) defer q.Stop() require.NoError(t, q.Acquire(ctx)) acquireCh := make(chan struct{}) @@ -51,3 +51,22 @@ func TestBudget(t *testing.T) { require.ErrorIs(t, q.Acquire(ctx), context.Canceled) }) } + +func TestPercent(t *testing.T) { + xtest.TestManyTimes(t, func(t testing.TB) { + var ( + total = 1000000 + percent = 0.25 + ctx = xtest.Context(t) + b = Percent(int(percent * 100)) + success int + ) + for i := 0; i < total; i++ { + if b.Acquire(ctx) == nil { + success++ + } + } + require.GreaterOrEqual(t, success, int(float64(total)*(percent-0.1*percent))) + require.LessOrEqual(t, success, int(float64(total)*(percent+0.1*percent))) + }, xtest.StopAfter(5*time.Second)) +} diff --git a/retry/retry.go b/retry/retry.go index e4be3d4a7..8b4e4d05d 100644 --- a/retry/retry.go +++ b/retry/retry.go @@ -261,7 +261,7 @@ func Retry(ctx context.Context, op retryOperation, opts ...Option) (finalErr err options := &retryOptions{ call: stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/retry.Retry"), trace: &trace.Retry{}, - budget: budget.New(-1), + budget: budget.Limited(-1), fastBackoff: backoff.Fast, slowBackoff: backoff.Slow, } diff --git a/tests/integration/retry_budget_test.go b/tests/integration/retry_budget_test.go index 0f733095d..36e847401 100644 --- a/tests/integration/retry_budget_test.go +++ b/tests/integration/retry_budget_test.go @@ -31,7 +31,7 @@ func (n noQuota) Acquire(ctx context.Context) error { func TestRetryBudget(t *testing.T) { ctx := xtest.Context(t) - defaultLimiter := budget.New(1) + defaultLimiter := budget.Limited(1) defer defaultLimiter.Stop() nativeDriver, err := ydb.Open(ctx, os.Getenv("YDB_CONNECTION_STRING"), diff --git a/tests/slo/database/sql/storage.go b/tests/slo/database/sql/storage.go index 633580bfd..0fb188e54 100755 --- a/tests/slo/database/sql/storage.go +++ b/tests/slo/database/sql/storage.go @@ -86,7 +86,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (s *Stora ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryBudget := budget.New(int(float64(poolSize) * 0.1)) //nolint:gomnd + retryBudget := budget.Limited(int(float64(poolSize) * 0.1)) //nolint:gomnd s = &Storage{ cfg: cfg, diff --git a/tests/slo/native/query/storage.go b/tests/slo/native/query/storage.go index 1eaa00006..a806775fd 100755 --- a/tests/slo/native/query/storage.go +++ b/tests/slo/native/query/storage.go @@ -73,7 +73,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryBudget := budget.New(int(float64(poolSize) * 0.1)) //nolint:gomnd + retryBudget := budget.Limited(int(float64(poolSize) * 0.1)) //nolint:gomnd db, err := ydb.Open(ctx, cfg.Endpoint+cfg.DB, diff --git a/tests/slo/native/table/storage.go b/tests/slo/native/table/storage.go index df4af1f44..affd5f60b 100755 --- a/tests/slo/native/table/storage.go +++ b/tests/slo/native/table/storage.go @@ -74,7 +74,7 @@ func NewStorage(ctx context.Context, cfg *config.Config, poolSize int) (*Storage ctx, cancel := context.WithTimeout(ctx, time.Minute*5) //nolint:gomnd defer cancel() - retryBudget := budget.New(int(float64(poolSize) * 0.1)) //nolint:gomnd + retryBudget := budget.Limited(int(float64(poolSize) * 0.1)) //nolint:gomnd db, err := ydb.Open( ctx, From fd6225d1f2d49fecf1b0242c8a21d9452ee7a053 Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Tue, 23 Apr 2024 12:52:50 +0300 Subject: [PATCH 31/33] fix comment --- retry/budget/budget.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/retry/budget/budget.go b/retry/budget/budget.go index dfe84fde9..617e25e0f 100644 --- a/retry/budget/budget.go +++ b/retry/budget/budget.go @@ -14,7 +14,7 @@ import ( type ( // Experimental: https://github.com/ydb-platform/ydb-go-sdk/blob/master/VERSIONING.md#experimental Budget interface { - // Acquire called from retryer on second and subsequence attempts + // Acquire will called on second and subsequent retry attempts Acquire(ctx context.Context) error } fixedBudget struct { From 7c3c1a746dd319338c4a213e1ade826eeca187ad Mon Sep 17 00:00:00 2001 From: Aleksey Myasnikov Date: Tue, 23 Apr 2024 13:14:27 +0300 Subject: [PATCH 32/33] fix linter issue --- retry/budget/budget.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/retry/budget/budget.go b/retry/budget/budget.go index 617e25e0f..2f2625178 100644 --- a/retry/budget/budget.go +++ b/retry/budget/budget.go @@ -102,6 +102,7 @@ func Percent(percent int) *percentBudget { if percent > 100 || percent < 0 { panic(fmt.Sprintf("wrong percent value: %d", percent)) } + return &percentBudget{ percent: percent, rand: xrand.New(xrand.WithLock()), @@ -109,7 +110,7 @@ func Percent(percent int) *percentBudget { } func (b *percentBudget) Acquire(ctx context.Context) error { - if b.rand.Int(100) < b.percent { + if b.rand.Int(100) < b.percent { //nolint:gomnd return nil } From 506a662ffd2aa3f7db22cead1c6ec0fa50d9337e Mon Sep 17 00:00:00 2001 From: robot Date: Tue, 23 Apr 2024 10:53:17 +0000 Subject: [PATCH 33/33] Release v3.66.0 --- CHANGELOG.md | 1 + internal/version/version.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad7b3f038..a6d38dea6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,4 @@ +## v3.66.0 * Added experimental package `retry/budget` for limit second and subsequent retry attempts * Refactored internals for enabling `containedctx` linter * Fixed the hanging semaphore issue on coordination session reconnect diff --git a/internal/version/version.go b/internal/version/version.go index d89575acc..1a50462e0 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -2,8 +2,8 @@ package version const ( Major = "3" - Minor = "65" - Patch = "3" + Minor = "66" + Patch = "0" Prefix = "ydb-go-sdk" )