Skip to content

Commit c0c3250

Browse files
asmyasnikovasmyasnikov
authored
Merge pull request #198 from ydb-platform/fix-panic-on-lazy-init
* Fixed race condition on lazy clients first call
2 parents c6fa814 + 9e9af75 commit c0c3250

File tree

6 files changed

+72
-86
lines changed

6 files changed

+72
-86
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
* Fixed race condition on lazy clients first call
2+
13
## v3.20.1
24
* Fixed gofumpt linter issue on `credentials/credentials.go`
35

‎internal/lazy/coordiantion.go‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
type lazyCoordination struct {
1717
db database.Connection
1818
options []config.Option
19-
client coordination.Client
19+
c coordination.Client
2020
m sync.Mutex
2121
}
2222

@@ -28,23 +28,20 @@ func Coordination(db database.Connection, options []config.Option) coordination.
2828
}
2929

3030
func (c *lazyCoordination) CreateNode(ctx context.Context, path string, config coordination.NodeConfig) (err error) {
31-
c.init()
3231
return retry.Retry(ctx, func(ctx context.Context) (err error) {
33-
return c.client.CreateNode(ctx, path, config)
32+
return c.client().CreateNode(ctx, path, config)
3433
})
3534
}
3635

3736
func (c *lazyCoordination) AlterNode(ctx context.Context, path string, config coordination.NodeConfig) (err error) {
38-
c.init()
3937
return retry.Retry(ctx, func(ctx context.Context) (err error) {
40-
return c.client.AlterNode(ctx, path, config)
38+
return c.client().AlterNode(ctx, path, config)
4139
})
4240
}
4341

4442
func (c *lazyCoordination) DropNode(ctx context.Context, path string) (err error) {
45-
c.init()
4643
return retry.Retry(ctx, func(ctx context.Context) (err error) {
47-
return c.client.DropNode(ctx, path)
44+
return c.client().DropNode(ctx, path)
4845
})
4946
}
5047

@@ -56,9 +53,8 @@ func (c *lazyCoordination) DescribeNode(
5653
config *coordination.NodeConfig,
5754
err error,
5855
) {
59-
c.init()
6056
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
61-
entry, config, err = c.client.DescribeNode(ctx, path)
57+
entry, config, err = c.client().DescribeNode(ctx, path)
6258
return xerrors.WithStackTrace(err)
6359
})
6460
return entry, config, xerrors.WithStackTrace(err)
@@ -67,23 +63,24 @@ func (c *lazyCoordination) DescribeNode(
6763
func (c *lazyCoordination) Close(ctx context.Context) (err error) {
6864
c.m.Lock()
6965
defer c.m.Unlock()
70-
if c.client == nil {
66+
if c.c == nil {
7167
return nil
7268
}
7369
defer func() {
74-
c.client = nil
70+
c.c = nil
7571
}()
76-
err = c.client.Close(ctx)
72+
err = c.c.Close(ctx)
7773
if err != nil {
7874
return xerrors.WithStackTrace(err)
7975
}
8076
return nil
8177
}
8278

83-
func (c *lazyCoordination) init() {
79+
func (c *lazyCoordination) client() coordination.Client {
8480
c.m.Lock()
85-
if c.client == nil {
86-
c.client = builder.New(c.db, c.options)
81+
defer c.m.Unlock()
82+
if c.c == nil {
83+
c.c = builder.New(c.db, c.options)
8784
}
88-
c.m.Unlock()
85+
return c.c
8986
}

‎internal/lazy/ratelimiter.go‎

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
type lazyRatelimiter struct {
1717
db database.Connection
1818
options []config.Option
19-
client ratelimiter.Client
19+
c ratelimiter.Client
2020
m sync.Mutex
2121
}
2222

@@ -30,13 +30,13 @@ func Ratelimiter(db database.Connection, options []config.Option) ratelimiter.Cl
3030
func (r *lazyRatelimiter) Close(ctx context.Context) (err error) {
3131
r.m.Lock()
3232
defer r.m.Unlock()
33-
if r.client == nil {
33+
if r.c == nil {
3434
return nil
3535
}
3636
defer func() {
37-
r.client = nil
37+
r.c = nil
3838
}()
39-
err = r.client.Close(ctx)
39+
err = r.c.Close(ctx)
4040
if err != nil {
4141
return xerrors.WithStackTrace(err)
4242
}
@@ -48,9 +48,8 @@ func (r *lazyRatelimiter) CreateResource(
4848
coordinationNodePath string,
4949
resource ratelimiter.Resource,
5050
) (err error) {
51-
r.init()
5251
return retry.Retry(ctx, func(ctx context.Context) (err error) {
53-
return r.client.CreateResource(ctx, coordinationNodePath, resource)
52+
return r.client().CreateResource(ctx, coordinationNodePath, resource)
5453
})
5554
}
5655

@@ -59,9 +58,8 @@ func (r *lazyRatelimiter) AlterResource(
5958
coordinationNodePath string,
6059
resource ratelimiter.Resource,
6160
) (err error) {
62-
r.init()
6361
return retry.Retry(ctx, func(ctx context.Context) (err error) {
64-
return r.client.AlterResource(ctx, coordinationNodePath, resource)
62+
return r.client().AlterResource(ctx, coordinationNodePath, resource)
6563
})
6664
}
6765

@@ -70,9 +68,8 @@ func (r *lazyRatelimiter) DropResource(
7068
coordinationNodePath string,
7169
resourcePath string,
7270
) (err error) {
73-
r.init()
7471
return retry.Retry(ctx, func(ctx context.Context) (err error) {
75-
return r.client.DropResource(ctx, coordinationNodePath, resourcePath)
72+
return r.client().DropResource(ctx, coordinationNodePath, resourcePath)
7673
})
7774
}
7875

@@ -82,9 +79,8 @@ func (r *lazyRatelimiter) ListResource(
8279
resourcePath string,
8380
recursive bool,
8481
) (paths []string, err error) {
85-
r.init()
8682
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
87-
paths, err = r.client.ListResource(ctx, coordinationNodePath, resourcePath, recursive)
83+
paths, err = r.client().ListResource(ctx, coordinationNodePath, resourcePath, recursive)
8884
return xerrors.WithStackTrace(err)
8985
})
9086
return paths, xerrors.WithStackTrace(err)
@@ -95,9 +91,8 @@ func (r *lazyRatelimiter) DescribeResource(
9591
coordinationNodePath string,
9692
resourcePath string,
9793
) (resource *ratelimiter.Resource, err error) {
98-
r.init()
9994
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
100-
resource, err = r.client.DescribeResource(ctx, coordinationNodePath, resourcePath)
95+
resource, err = r.client().DescribeResource(ctx, coordinationNodePath, resourcePath)
10196
return xerrors.WithStackTrace(err)
10297
})
10398
return resource, xerrors.WithStackTrace(err)
@@ -110,16 +105,16 @@ func (r *lazyRatelimiter) AcquireResource(
110105
amount uint64,
111106
opts ...options.AcquireOption,
112107
) (err error) {
113-
r.init()
114108
return retry.Retry(ctx, func(ctx context.Context) (err error) {
115-
return r.client.AcquireResource(ctx, coordinationNodePath, resourcePath, amount, opts...)
109+
return r.client().AcquireResource(ctx, coordinationNodePath, resourcePath, amount, opts...)
116110
})
117111
}
118112

119-
func (r *lazyRatelimiter) init() {
113+
func (r *lazyRatelimiter) client() ratelimiter.Client {
120114
r.m.Lock()
121-
if r.client == nil {
122-
r.client = builder.New(r.db, r.options)
115+
defer r.m.Unlock()
116+
if r.c == nil {
117+
r.c = builder.New(r.db, r.options)
123118
}
124-
r.m.Unlock()
119+
return r.c
125120
}

‎internal/lazy/scheme.go‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
type lazyScheme struct {
1616
db database.Connection
1717
options []config.Option
18-
client scheme.Client
18+
c scheme.Client
1919
m sync.Mutex
2020
}
2121

@@ -31,64 +31,60 @@ func (s *lazyScheme) ModifyPermissions(
3131
path string,
3232
opts ...scheme.PermissionsOption,
3333
) (err error) {
34-
s.init()
3534
return retry.Retry(ctx, func(ctx context.Context) (err error) {
36-
return s.client.ModifyPermissions(ctx, path, opts...)
35+
return s.client().ModifyPermissions(ctx, path, opts...)
3736
})
3837
}
3938

4039
func (s *lazyScheme) Close(ctx context.Context) (err error) {
4140
s.m.Lock()
4241
defer s.m.Unlock()
43-
if s.client == nil {
42+
if s.c == nil {
4443
return nil
4544
}
4645
defer func() {
47-
s.client = nil
46+
s.c = nil
4847
}()
49-
err = s.client.Close(ctx)
48+
err = s.c.Close(ctx)
5049
if err != nil {
5150
return xerrors.WithStackTrace(err)
5251
}
5352
return nil
5453
}
5554

56-
func (s *lazyScheme) init() {
57-
s.m.Lock()
58-
if s.client == nil {
59-
s.client = builder.New(s.db, s.options)
60-
}
61-
s.m.Unlock()
62-
}
63-
6455
func (s *lazyScheme) DescribePath(ctx context.Context, path string) (e scheme.Entry, err error) {
65-
s.init()
6656
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
67-
e, err = s.client.DescribePath(ctx, path)
57+
e, err = s.client().DescribePath(ctx, path)
6858
return xerrors.WithStackTrace(err)
6959
}, retry.WithIdempotent())
7060
return e, xerrors.WithStackTrace(err)
7161
}
7262

7363
func (s *lazyScheme) MakeDirectory(ctx context.Context, path string) (err error) {
74-
s.init()
7564
return retry.Retry(ctx, func(ctx context.Context) (err error) {
76-
return s.client.MakeDirectory(ctx, path)
65+
return s.client().MakeDirectory(ctx, path)
7766
})
7867
}
7968

8069
func (s *lazyScheme) ListDirectory(ctx context.Context, path string) (d scheme.Directory, err error) {
81-
s.init()
8270
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
83-
d, err = s.client.ListDirectory(ctx, path)
71+
d, err = s.client().ListDirectory(ctx, path)
8472
return xerrors.WithStackTrace(err)
8573
}, retry.WithIdempotent())
8674
return d, xerrors.WithStackTrace(err)
8775
}
8876

8977
func (s *lazyScheme) RemoveDirectory(ctx context.Context, path string) (err error) {
90-
s.init()
9178
return retry.Retry(ctx, func(ctx context.Context) (err error) {
92-
return s.client.RemoveDirectory(ctx, path)
79+
return s.client().RemoveDirectory(ctx, path)
9380
})
9481
}
82+
83+
func (s *lazyScheme) client() scheme.Client {
84+
s.m.Lock()
85+
defer s.m.Unlock()
86+
if s.c == nil {
87+
s.c = builder.New(s.db, s.options)
88+
}
89+
return s.c
90+
}

‎internal/lazy/scripting.go‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import (
1717
type lazyScripting struct {
1818
db database.Connection
1919
options []config.Option
20-
client scripting.Client
20+
c scripting.Client
2121
m sync.Mutex
2222
}
2323

@@ -26,9 +26,8 @@ func (s *lazyScripting) Execute(
2626
query string,
2727
params *table.QueryParameters,
2828
) (res result.Result, err error) {
29-
s.init()
3029
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
31-
res, err = s.client.Execute(ctx, query, params)
30+
res, err = s.client().Execute(ctx, query, params)
3231
return xerrors.WithStackTrace(err)
3332
})
3433
return res, xerrors.WithStackTrace(err)
@@ -39,9 +38,8 @@ func (s *lazyScripting) Explain(
3938
query string,
4039
mode scripting.ExplainMode,
4140
) (e table.ScriptingYQLExplanation, err error) {
42-
s.init()
4341
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
44-
e, err = s.client.Explain(ctx, query, mode)
42+
e, err = s.client().Explain(ctx, query, mode)
4543
return xerrors.WithStackTrace(err)
4644
})
4745
return e, xerrors.WithStackTrace(err)
@@ -52,9 +50,8 @@ func (s *lazyScripting) StreamExecute(
5250
query string,
5351
params *table.QueryParameters,
5452
) (res result.StreamResult, err error) {
55-
s.init()
5653
err = retry.Retry(ctx, func(ctx context.Context) (err error) {
57-
res, err = s.client.StreamExecute(ctx, query, params)
54+
res, err = s.client().StreamExecute(ctx, query, params)
5855
return xerrors.WithStackTrace(err)
5956
})
6057
return res, xerrors.WithStackTrace(err)
@@ -63,13 +60,13 @@ func (s *lazyScripting) StreamExecute(
6360
func (s *lazyScripting) Close(ctx context.Context) (err error) {
6461
s.m.Lock()
6562
defer s.m.Unlock()
66-
if s.client == nil {
63+
if s.c == nil {
6764
return nil
6865
}
6966
defer func() {
70-
s.client = nil
67+
s.c = nil
7168
}()
72-
err = s.client.Close(ctx)
69+
err = s.c.Close(ctx)
7370
if err != nil {
7471
return xerrors.WithStackTrace(err)
7572
}
@@ -83,10 +80,11 @@ func Scripting(db database.Connection, options []config.Option) scripting.Client
8380
}
8481
}
8582

86-
func (s *lazyScripting) init() {
83+
func (s *lazyScripting) client() scripting.Client {
8784
s.m.Lock()
88-
if s.client == nil {
89-
s.client = builder.New(s.db, s.options)
85+
defer s.m.Unlock()
86+
if s.c == nil {
87+
s.c = builder.New(s.db, s.options)
9088
}
91-
s.m.Unlock()
89+
return s.c
9290
}

0 commit comments

Comments
 (0)