Merge pull request #5373 from tinyspeck/txp-pool-error-improvements

Better granularity for errors that come out of the transaction pool

go/pools/resource_pool.go

@@ -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
 )
 
@@ -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:
 	}
 

go/pools/resource_pool_test.go

@@ -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)
 	}

go/vt/vttablet/tabletserver/tabletserver_test.go

@@ -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)
 	}

go/vt/vttablet/tabletserver/tx_pool.go

@@ -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")

go/vt/vttablet/tabletserver/tx_pool_test.go

@@ -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)