Skip to content

Commit e063da0

Browse files
committed
* Fixed re-opening case after close lazy-initialized clients
* Removed dependency of call context for initializing lazy table client
1 parent 33a7ce2 commit e063da0

File tree

9 files changed

+28
-41
lines changed

9 files changed

+28
-41
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
* Fixed re-opening case after close lazy-initialized clients
2+
* Removed dependency of call context for initializing lazy table client
3+
14
## v3.23.0
25
* Added `WithTLSConfig` option for redefine TLS config
36
* Added `sugar.LoadCertificatesFromFile` and `sugar.LoadCertificatesFromPem` helpers

internal/lazy/coordiantion.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,6 @@ func (c *lazyCoordination) Close(ctx context.Context) (err error) {
6666
if c.c == nil {
6767
return nil
6868
}
69-
defer func() {
70-
c.c = nil
71-
}()
7269
err = c.c.Close(ctx)
7370
if err != nil {
7471
return xerrors.WithStackTrace(err)

internal/lazy/ratelimiter.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@ func (r *lazyRatelimiter) Close(ctx context.Context) (err error) {
3333
if r.c == nil {
3434
return nil
3535
}
36-
defer func() {
37-
r.c = nil
38-
}()
3936
err = r.c.Close(ctx)
4037
if err != nil {
4138
return xerrors.WithStackTrace(err)

internal/lazy/scheme.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,6 @@ func (s *lazyScheme) Close(ctx context.Context) (err error) {
4242
if s.c == nil {
4343
return nil
4444
}
45-
defer func() {
46-
s.c = nil
47-
}()
4845
err = s.c.Close(ctx)
4946
if err != nil {
5047
return xerrors.WithStackTrace(err)

internal/lazy/scripting.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,6 @@ func (s *lazyScripting) Close(ctx context.Context) (err error) {
6363
if s.c == nil {
6464
return nil
6565
}
66-
defer func() {
67-
s.c = nil
68-
}()
6966
err = s.c.Close(ctx)
7067
if err != nil {
7168
return xerrors.WithStackTrace(err)

internal/lazy/table.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,15 @@ func Table(db database.Connection, options []config.Option) table.Client {
2626
}
2727

2828
func (t *lazyTable) CreateSession(ctx context.Context, opts ...table.Option) (s table.ClosableSession, err error) {
29-
return t.client(ctx).CreateSession(ctx, opts...)
29+
return t.client().CreateSession(ctx, opts...)
3030
}
3131

3232
func (t *lazyTable) Do(ctx context.Context, op table.Operation, opts ...table.Option) (err error) {
33-
return t.client(ctx).Do(ctx, op, opts...)
33+
return t.client().Do(ctx, op, opts...)
3434
}
3535

3636
func (t *lazyTable) DoTx(ctx context.Context, op table.TxOperation, opts ...table.Option) (err error) {
37-
return t.client(ctx).DoTx(ctx, op, opts...)
37+
return t.client().DoTx(ctx, op, opts...)
3838
}
3939

4040
func (t *lazyTable) Close(ctx context.Context) (err error) {
@@ -43,21 +43,18 @@ func (t *lazyTable) Close(ctx context.Context) (err error) {
4343
if t.c == nil {
4444
return nil
4545
}
46-
defer func() {
47-
t.c = nil
48-
}()
4946
err = t.c.Close(ctx)
5047
if err != nil {
5148
return xerrors.WithStackTrace(err)
5249
}
5350
return nil
5451
}
5552

56-
func (t *lazyTable) client(ctx context.Context) table.Client {
53+
func (t *lazyTable) client() table.Client {
5754
t.m.Lock()
5855
defer t.m.Unlock()
5956
if t.c == nil {
60-
t.c = builder.New(ctx, t.db, t.options...)
57+
t.c = builder.New(t.db, t.options)
6158
}
6259
return t.c
6360
}

internal/table/client.go

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ import (
2424
)
2525

2626
var (
27-
// errSessionPoolClosed returned by a client instance to indicate
28-
// that client is closed and not able to complete requested operation.
29-
errSessionPoolClosed = xerrors.Wrap(fmt.Errorf("session pool is closed"))
27+
// errAlreadyClosed returned by a client instance to indicate
28+
// that client is closed early and not able to complete requested operation.
29+
errAlreadyClosed = xerrors.Wrap(fmt.Errorf("table client closed early"))
3030

3131
// errSessionPoolOverflow returned by a client instance to indicate
3232
// that the client is full and requested operation is not able to complete.
@@ -52,18 +52,20 @@ type Client interface {
5252
CloseSession(ctx context.Context, s Session) (err error)
5353
}
5454

55-
func New(ctx context.Context, cc grpc.ClientConnInterface, opts ...config.Option) Client {
55+
func New(cc grpc.ClientConnInterface, opts []config.Option) Client {
5656
config := config.New(opts...)
57-
return newClient(ctx, cc, nil, config)
57+
return newClient(cc, nil, config)
5858
}
5959

6060
func newClient(
61-
ctx context.Context,
6261
cc grpc.ClientConnInterface,
6362
builder SessionBuilder,
6463
config config.Config,
6564
) *client {
66-
onDone := trace.TableOnInit(config.Trace(), &ctx)
65+
var (
66+
ctx = context.Background()
67+
onDone = trace.TableOnInit(config.Trace(), &ctx)
68+
)
6769
if builder == nil {
6870
builder = func(ctx context.Context) (s Session, err error) {
6971
return newSession(ctx, cc, config)
@@ -315,7 +317,7 @@ func (c *client) get(ctx context.Context, opts ...getOption) (s Session, err err
315317
const maxAttempts = 100
316318
for ; s == nil && err == nil && i < maxAttempts; i++ {
317319
if c.isClosed() {
318-
return nil, xerrors.WithStackTrace(errSessionPoolClosed)
320+
return nil, xerrors.WithStackTrace(errAlreadyClosed)
319321
}
320322

321323
// First, we try to get session from idle
@@ -415,7 +417,7 @@ func (c *client) waitFromCh(ctx context.Context, t trace.Table) (s Session, err
415417

416418
// Put returns session to the client for further reuse.
417419
// If client is already closed Put() calls s.Close(ctx) and returns
418-
// errSessionPoolClosed.
420+
// errAlreadyClosed.
419421
// If client is overflow calls s.Close(ctx) and returns
420422
// errSessionPoolOverflow.
421423
//
@@ -432,7 +434,7 @@ func (c *client) Put(ctx context.Context, s Session) (err error) {
432434

433435
switch {
434436
case c.closed:
435-
err = xerrors.WithStackTrace(errSessionPoolClosed)
437+
err = xerrors.WithStackTrace(errAlreadyClosed)
436438

437439
case c.idle.Len() >= c.limit:
438440
err = xerrors.WithStackTrace(errSessionPoolOverflow)
@@ -473,7 +475,7 @@ func (c *client) Close(ctx context.Context) (err error) {
473475
}()
474476

475477
if c.isClosed() {
476-
return
478+
return xerrors.WithStackTrace(errAlreadyClosed)
477479
}
478480

479481
c.mu.Lock()
@@ -540,7 +542,7 @@ func retryOptions(trace trace.Table, opts ...table.Option) table.Options {
540542
// Warning: if deadline without deadline or cancellation func Retry will be worked infinite
541543
func (c *client) Do(ctx context.Context, op table.Operation, opts ...table.Option) (err error) {
542544
if c.isClosed() {
543-
return xerrors.WithStackTrace(errSessionPoolClosed)
545+
return xerrors.WithStackTrace(errAlreadyClosed)
544546
}
545547
opts = append(opts, table.WithTrace(c.config.Trace()))
546548
return do(
@@ -554,7 +556,7 @@ func (c *client) Do(ctx context.Context, op table.Operation, opts ...table.Optio
554556

555557
func (c *client) DoTx(ctx context.Context, op table.TxOperation, opts ...table.Option) (err error) {
556558
if c.isClosed() {
557-
return xerrors.WithStackTrace(errSessionPoolClosed)
559+
return xerrors.WithStackTrace(errAlreadyClosed)
558560
}
559561
return doTx(
560562
ctx,

internal/table/client_test.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,10 @@ func TestSessionPoolCloseWhenWaiting(t *testing.T) {
230230
const timeout = time.Second
231231
select {
232232
case err := <-got:
233-
if !xerrors.Is(err, errSessionPoolClosed) {
233+
if !xerrors.Is(err, errAlreadyClosed) {
234234
t.Fatalf(
235235
"unexpected error: %v; want %v",
236-
err, errSessionPoolClosed,
236+
err, errAlreadyClosed,
237237
)
238238
}
239239
case <-time.After(timeout):
@@ -310,10 +310,10 @@ func TestSessionPoolClose(t *testing.T) {
310310
t.Fatalf("unexpected session close")
311311
}
312312

313-
if err := p.Put(context.Background(), s3); !xerrors.Is(err, errSessionPoolClosed) {
313+
if err := p.Put(context.Background(), s3); !xerrors.Is(err, errAlreadyClosed) {
314314
t.Errorf(
315315
"unexpected Put() error: %v; want %v",
316-
err, errSessionPoolClosed,
316+
err, errAlreadyClosed,
317317
)
318318
}
319319
wg.Wait()
@@ -366,7 +366,7 @@ func TestRaceWgClosed(t *testing.T) {
366366
return nil
367367
},
368368
)
369-
if xerrors.Is(err, errSessionPoolClosed) {
369+
if xerrors.Is(err, errAlreadyClosed) {
370370
return
371371
}
372372
}
@@ -474,7 +474,6 @@ func TestSessionPoolRacyGet(t *testing.T) {
474474
}
475475
create := make(chan createReq)
476476
p := newClient(
477-
context.Background(),
478477
nil,
479478
(&StubBuilder{
480479
Limit: 1,
@@ -1357,7 +1356,6 @@ func newClientWithStubBuilder(
13571356
options ...config.Option,
13581357
) *client {
13591358
return newClient(
1360-
context.Background(),
13611359
cc,
13621360
(&StubBuilder{
13631361
T: t,

internal/table/session_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,6 @@ func TestSessionOperationModeOnExecuteDataQuery(t *testing.T) {
331331
for _, srcDst := range fromTo {
332332
t.Run(srcDst.srcMode.String()+"->"+srcDst.dstMode.String(), func(t *testing.T) {
333333
client := newClient(
334-
context.Background(),
335334
testutil.NewDB(
336335
testutil.WithInvokeHandlers(
337336
testutil.InvokeHandlers{

0 commit comments

Comments
 (0)