Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better granularity for errors that come out of the transaction pool #5373

Merged
merged 2 commits into from Nov 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion go/pools/resource_pool.go
Expand Up @@ -38,6 +38,9 @@ var (
// ErrTimeout is returned if a resource get times out.
ErrTimeout = errors.New("resource pool timed out")

// ErrCtxTimeout is returned if a ctx is already expired by the time the resource pool is used
ErrCtxTimeout = errors.New("resource pool context already expired")

prefillTimeout = 30 * time.Second
)

Expand Down Expand Up @@ -198,7 +201,7 @@ func (rp *ResourcePool) get(ctx context.Context) (resource Resource, err error)
// If ctx has already expired, avoid racing with rp's resource channel.
select {
case <-ctx.Done():
return nil, ErrTimeout
return nil, ErrCtxTimeout
default:
}

Expand Down
2 changes: 1 addition & 1 deletion go/pools/resource_pool_test.go
Expand Up @@ -639,7 +639,7 @@ func TestExpired(t *testing.T) {
p.Put(r)
}
cancel()
want := "resource pool timed out"
want := "resource pool context already expired"
if err == nil || err.Error() != want {
t.Errorf("got %v, want %s", err, want)
}
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttablet/tabletserver/tabletserver_test.go
Expand Up @@ -962,7 +962,7 @@ func TestTabletServerBeginFail(t *testing.T) {
defer cancel()
tsv.Begin(ctx, &target, nil)
_, err = tsv.Begin(ctx, &target, nil)
want := "transaction pool connection limit exceeded"
want := "transaction pool aborting request due to already expired context"
if err == nil || err.Error() != want {
t.Fatalf("Begin err: %v, want %v", err, want)
}
Expand Down
3 changes: 3 additions & 0 deletions go/vt/vttablet/tabletserver/tx_pool.go
Expand Up @@ -245,6 +245,9 @@ func (axp *TxPool) Begin(ctx context.Context, options *querypb.ExecuteOptions) (
switch err {
case connpool.ErrConnPoolClosed:
return 0, "", err
case pools.ErrCtxTimeout:
axp.LogActive()
return 0, "", vterrors.Errorf(vtrpcpb.Code_RESOURCE_EXHAUSTED, "transaction pool aborting request due to already expired context")
case pools.ErrTimeout:
axp.LogActive()
return 0, "", vterrors.Errorf(vtrpcpb.Code_RESOURCE_EXHAUSTED, "transaction pool connection limit exceeded")
Expand Down
19 changes: 19 additions & 0 deletions go/vt/vttablet/tabletserver/tx_pool_test.go
Expand Up @@ -470,6 +470,25 @@ func TestTxPoolBeginWithError(t *testing.T) {
}
}

func TestTxPoolCancelledContextError(t *testing.T) {
db := fakesqldb.New(t)
defer db.Close()
db.AddRejectedQuery("begin", errRejected)
txPool := newTxPool()
txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams())
defer txPool.Close()
ctx, cancel := context.WithCancel(context.Background())
cancel()
_, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{})
want := "transaction pool aborting request due to already expired context"
if err == nil || !strings.Contains(err.Error(), want) {
t.Errorf("Unexpected error: %v, want %s", err, want)
}
if got, want := vterrors.Code(err), vtrpcpb.Code_RESOURCE_EXHAUSTED; got != want {
t.Errorf("wrong error code error: got = %v, want = %v", got, want)
}
}

func TestTxPoolRollbackFail(t *testing.T) {
sql := "alter table test_table add test_column int"
db := fakesqldb.New(t)
Expand Down