Skip to content

Commit cb1d926

Browse files
authored
Merge pull request #225 from ydb-platform/remove-lazy
Fix bug with reopening lazy table client on call after Close()
2 parents 33a7ce2 + a7e3c2c commit cb1d926

File tree

15 files changed

+142
-130
lines changed

15 files changed

+142
-130
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
* Fixed re-opening case after close lazy-initialized clients
2+
* Removed dependency of call context for initializing lazy table client
3+
* Added `config.AutoRetry()` flag with `true` value by default. `config.AutoRetry()` affects how to errors handle in sub-clients calls.
4+
* Added `config.WithNoAutoRetry` for disabling auto-retry on errors in sub-clients calls
5+
* Refactored `internal/lazy` package (supported check `config.AutoRetry()`, removed all error wrappings with stacktrace)
6+
17
## v3.23.0
28
* Added `WithTLSConfig` option for redefine TLS config
39
* Added `sugar.LoadCertificatesFromFile` and `sugar.LoadCertificatesFromPem` helpers

config/config.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,13 @@ func WithOperationCancelAfter(operationCancelAfter time.Duration) Option {
208208
}
209209
}
210210

211+
// WithNoAutoRetry disable auto-retry calls from YDB sub-clients
212+
func WithNoAutoRetry() Option {
213+
return func(c *Config) {
214+
config.SetAutoRetry(&c.Common, false)
215+
}
216+
}
217+
211218
// WithPanicCallback applies panic callback to config
212219
func WithPanicCallback(panicCallback func(e interface{})) Option {
213220
return func(c *Config) {

internal/config/config.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,16 @@ import (
77
type Common struct {
88
operationTimeout time.Duration
99
operationCancelAfter time.Duration
10+
disableAutoRetry bool
1011

1112
panicCallback func(e interface{})
1213
}
1314

15+
// AutoRetry defines auto-retry flag
16+
func (c *Common) AutoRetry() bool {
17+
return !c.disableAutoRetry
18+
}
19+
1420
// PanicCallback returns user-defined panic callback
1521
// If nil - panic callback not defined
1622
func (c *Common) PanicCallback() func(e interface{}) {
@@ -59,3 +65,8 @@ func SetOperationCancelAfter(c *Common, operationCancelAfter time.Duration) {
5965
func SetPanicCallback(c *Common, panicCallback func(e interface{})) {
6066
c.panicCallback = panicCallback
6167
}
68+
69+
// SetAutoRetry affects on AutoRetry() flag
70+
func SetAutoRetry(c *Common, autoRetry bool) {
71+
c.disableAutoRetry = !autoRetry
72+
}

internal/coordination/client.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ type client struct {
2121
service Ydb_Coordination_V1.CoordinationServiceClient
2222
}
2323

24-
func New(cc grpc.ClientConnInterface, options []config.Option) coordination.Client {
24+
func New(cc grpc.ClientConnInterface, config config.Config) coordination.Client {
2525
return &client{
26-
config: config.New(options...),
26+
config: config,
2727
service: Ydb_Coordination_V1.NewCoordinationServiceClient(cc),
2828
}
2929
}

internal/lazy/coordiantion.go

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,38 +8,46 @@ import (
88
builder "github.com/ydb-platform/ydb-go-sdk/v3/internal/coordination"
99
"github.com/ydb-platform/ydb-go-sdk/v3/internal/coordination/config"
1010
"github.com/ydb-platform/ydb-go-sdk/v3/internal/database"
11-
"github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors"
1211
"github.com/ydb-platform/ydb-go-sdk/v3/retry"
1312
"github.com/ydb-platform/ydb-go-sdk/v3/scheme"
1413
)
1514

1615
type lazyCoordination struct {
17-
db database.Connection
18-
options []config.Option
19-
c coordination.Client
20-
m sync.Mutex
16+
db database.Connection
17+
config config.Config
18+
c coordination.Client
19+
m sync.Mutex
2120
}
2221

2322
func Coordination(db database.Connection, options []config.Option) coordination.Client {
2423
return &lazyCoordination{
25-
db: db,
26-
options: options,
24+
db: db,
25+
config: config.New(options...),
2726
}
2827
}
2928

3029
func (c *lazyCoordination) CreateNode(ctx context.Context, path string, config coordination.NodeConfig) (err error) {
30+
if !c.config.AutoRetry() {
31+
return c.client().CreateNode(ctx, path, config)
32+
}
3133
return retry.Retry(ctx, func(ctx context.Context) (err error) {
3234
return c.client().CreateNode(ctx, path, config)
3335
})
3436
}
3537

3638
func (c *lazyCoordination) AlterNode(ctx context.Context, path string, config coordination.NodeConfig) (err error) {
39+
if !c.config.AutoRetry() {
40+
return c.client().AlterNode(ctx, path, config)
41+
}
3742
return retry.Retry(ctx, func(ctx context.Context) (err error) {
3843
return c.client().AlterNode(ctx, path, config)
3944
})
4045
}
4146

4247
func (c *lazyCoordination) DropNode(ctx context.Context, path string) (err error) {
48+
if !c.config.AutoRetry() {
49+
return c.client().DropNode(ctx, path)
50+
}
4351
return retry.Retry(ctx, func(ctx context.Context) (err error) {
4452
return c.client().DropNode(ctx, path)
4553
})
@@ -53,11 +61,14 @@ func (c *lazyCoordination) DescribeNode(
5361
config *coordination.NodeConfig,
5462
err error,
5563
) {
64+
if !c.config.AutoRetry() {
65+
return c.client().DescribeNode(ctx, path)
66+
}
5667
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
5768
entry, config, err = c.client().DescribeNode(ctx, path)
58-
return xerrors.WithStackTrace(err)
69+
return err
5970
})
60-
return entry, config, xerrors.WithStackTrace(err)
71+
return entry, config, err
6172
}
6273

6374
func (c *lazyCoordination) Close(ctx context.Context) (err error) {
@@ -66,21 +77,14 @@ func (c *lazyCoordination) Close(ctx context.Context) (err error) {
6677
if c.c == nil {
6778
return nil
6879
}
69-
defer func() {
70-
c.c = nil
71-
}()
72-
err = c.c.Close(ctx)
73-
if err != nil {
74-
return xerrors.WithStackTrace(err)
75-
}
76-
return nil
80+
return c.c.Close(ctx)
7781
}
7882

7983
func (c *lazyCoordination) client() coordination.Client {
8084
c.m.Lock()
8185
defer c.m.Unlock()
8286
if c.c == nil {
83-
c.c = builder.New(c.db, c.options)
87+
c.c = builder.New(c.db, c.config)
8488
}
8589
return c.c
8690
}

internal/lazy/ratelimiter.go

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,21 @@ import (
88
builder "github.com/ydb-platform/ydb-go-sdk/v3/internal/ratelimiter"
99
"github.com/ydb-platform/ydb-go-sdk/v3/internal/ratelimiter/config"
1010
"github.com/ydb-platform/ydb-go-sdk/v3/internal/ratelimiter/options"
11-
"github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors"
1211
"github.com/ydb-platform/ydb-go-sdk/v3/ratelimiter"
1312
"github.com/ydb-platform/ydb-go-sdk/v3/retry"
1413
)
1514

1615
type lazyRatelimiter struct {
17-
db database.Connection
18-
options []config.Option
19-
c ratelimiter.Client
20-
m sync.Mutex
16+
db database.Connection
17+
config config.Config
18+
c ratelimiter.Client
19+
m sync.Mutex
2120
}
2221

2322
func Ratelimiter(db database.Connection, options []config.Option) ratelimiter.Client {
2423
return &lazyRatelimiter{
25-
db: db,
26-
options: options,
24+
db: db,
25+
config: config.New(options...),
2726
}
2827
}
2928

@@ -33,21 +32,17 @@ func (r *lazyRatelimiter) Close(ctx context.Context) (err error) {
3332
if r.c == nil {
3433
return nil
3534
}
36-
defer func() {
37-
r.c = nil
38-
}()
39-
err = r.c.Close(ctx)
40-
if err != nil {
41-
return xerrors.WithStackTrace(err)
42-
}
43-
return nil
35+
return r.c.Close(ctx)
4436
}
4537

4638
func (r *lazyRatelimiter) CreateResource(
4739
ctx context.Context,
4840
coordinationNodePath string,
4941
resource ratelimiter.Resource,
5042
) (err error) {
43+
if !r.config.AutoRetry() {
44+
return r.client().CreateResource(ctx, coordinationNodePath, resource)
45+
}
5146
return retry.Retry(ctx, func(ctx context.Context) (err error) {
5247
return r.client().CreateResource(ctx, coordinationNodePath, resource)
5348
})
@@ -58,6 +53,9 @@ func (r *lazyRatelimiter) AlterResource(
5853
coordinationNodePath string,
5954
resource ratelimiter.Resource,
6055
) (err error) {
56+
if !r.config.AutoRetry() {
57+
return r.client().AlterResource(ctx, coordinationNodePath, resource)
58+
}
6159
return retry.Retry(ctx, func(ctx context.Context) (err error) {
6260
return r.client().AlterResource(ctx, coordinationNodePath, resource)
6361
})
@@ -68,6 +66,9 @@ func (r *lazyRatelimiter) DropResource(
6866
coordinationNodePath string,
6967
resourcePath string,
7068
) (err error) {
69+
if !r.config.AutoRetry() {
70+
return r.client().DropResource(ctx, coordinationNodePath, resourcePath)
71+
}
7172
return retry.Retry(ctx, func(ctx context.Context) (err error) {
7273
return r.client().DropResource(ctx, coordinationNodePath, resourcePath)
7374
})
@@ -79,23 +80,29 @@ func (r *lazyRatelimiter) ListResource(
7980
resourcePath string,
8081
recursive bool,
8182
) (paths []string, err error) {
83+
if !r.config.AutoRetry() {
84+
return r.client().ListResource(ctx, coordinationNodePath, resourcePath, recursive)
85+
}
8286
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
8387
paths, err = r.client().ListResource(ctx, coordinationNodePath, resourcePath, recursive)
84-
return xerrors.WithStackTrace(err)
88+
return err
8589
})
86-
return paths, xerrors.WithStackTrace(err)
90+
return paths, err
8791
}
8892

8993
func (r *lazyRatelimiter) DescribeResource(
9094
ctx context.Context,
9195
coordinationNodePath string,
9296
resourcePath string,
9397
) (resource *ratelimiter.Resource, err error) {
98+
if !r.config.AutoRetry() {
99+
return r.client().DescribeResource(ctx, coordinationNodePath, resourcePath)
100+
}
94101
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
95102
resource, err = r.client().DescribeResource(ctx, coordinationNodePath, resourcePath)
96-
return xerrors.WithStackTrace(err)
103+
return err
97104
})
98-
return resource, xerrors.WithStackTrace(err)
105+
return resource, err
99106
}
100107

101108
func (r *lazyRatelimiter) AcquireResource(
@@ -105,6 +112,9 @@ func (r *lazyRatelimiter) AcquireResource(
105112
amount uint64,
106113
opts ...options.AcquireOption,
107114
) (err error) {
115+
if !r.config.AutoRetry() {
116+
return r.client().AcquireResource(ctx, coordinationNodePath, resourcePath, amount, opts...)
117+
}
108118
return retry.Retry(ctx, func(ctx context.Context) (err error) {
109119
return r.client().AcquireResource(ctx, coordinationNodePath, resourcePath, amount, opts...)
110120
})
@@ -114,7 +124,7 @@ func (r *lazyRatelimiter) client() ratelimiter.Client {
114124
r.m.Lock()
115125
defer r.m.Unlock()
116126
if r.c == nil {
117-
r.c = builder.New(r.db, r.options)
127+
r.c = builder.New(r.db, r.config)
118128
}
119129
return r.c
120130
}

internal/lazy/scheme.go

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,21 @@ import (
77
"github.com/ydb-platform/ydb-go-sdk/v3/internal/database"
88
builder "github.com/ydb-platform/ydb-go-sdk/v3/internal/scheme"
99
"github.com/ydb-platform/ydb-go-sdk/v3/internal/scheme/config"
10-
"github.com/ydb-platform/ydb-go-sdk/v3/internal/xerrors"
1110
"github.com/ydb-platform/ydb-go-sdk/v3/retry"
1211
"github.com/ydb-platform/ydb-go-sdk/v3/scheme"
1312
)
1413

1514
type lazyScheme struct {
16-
db database.Connection
17-
options []config.Option
18-
c scheme.Client
19-
m sync.Mutex
15+
db database.Connection
16+
config config.Config
17+
c scheme.Client
18+
m sync.Mutex
2019
}
2120

2221
func Scheme(db database.Connection, options []config.Option) scheme.Client {
2322
return &lazyScheme{
24-
db: db,
25-
options: options,
23+
db: db,
24+
config: config.New(options...),
2625
}
2726
}
2827

@@ -42,22 +41,15 @@ func (s *lazyScheme) Close(ctx context.Context) (err error) {
4241
if s.c == nil {
4342
return nil
4443
}
45-
defer func() {
46-
s.c = nil
47-
}()
48-
err = s.c.Close(ctx)
49-
if err != nil {
50-
return xerrors.WithStackTrace(err)
51-
}
52-
return nil
44+
return s.c.Close(ctx)
5345
}
5446

5547
func (s *lazyScheme) DescribePath(ctx context.Context, path string) (e scheme.Entry, err error) {
5648
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
5749
e, err = s.client().DescribePath(ctx, path)
58-
return xerrors.WithStackTrace(err)
50+
return err
5951
}, retry.WithIdempotent(true))
60-
return e, xerrors.WithStackTrace(err)
52+
return e, err
6153
}
6254

6355
func (s *lazyScheme) MakeDirectory(ctx context.Context, path string) (err error) {
@@ -69,9 +61,9 @@ func (s *lazyScheme) MakeDirectory(ctx context.Context, path string) (err error)
6961
func (s *lazyScheme) ListDirectory(ctx context.Context, path string) (d scheme.Directory, err error) {
7062
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
7163
d, err = s.client().ListDirectory(ctx, path)
72-
return xerrors.WithStackTrace(err)
64+
return err
7365
}, retry.WithIdempotent(true))
74-
return d, xerrors.WithStackTrace(err)
66+
return d, err
7567
}
7668

7769
func (s *lazyScheme) RemoveDirectory(ctx context.Context, path string) (err error) {
@@ -84,7 +76,7 @@ func (s *lazyScheme) client() scheme.Client {
8476
s.m.Lock()
8577
defer s.m.Unlock()
8678
if s.c == nil {
87-
s.c = builder.New(s.db, s.options)
79+
s.c = builder.New(s.db, s.config)
8880
}
8981
return s.c
9082
}

0 commit comments

Comments
 (0)