Skip to content

Commit

Permalink
Merge pull request vitessio#2192 from alainjobart/sharderror
Browse files Browse the repository at this point in the history 
Fixing a confusing vtgate error message.
  • Loading branch information
@alainjobart
alainjobart authored Oct 28, 2016
2 parents 56718c1 + c0ece0d commit 0853e53
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 27 deletions.
24 changes: 12 additions & 12 deletions go/vt/vtgate/gateway/discoverygateway_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,23 +124,23 @@ func testDiscoveryGatewayGeneric(t *testing.T, streaming bool, f func(dg Gateway
// no tablet
hc.Reset()
dg.tsc.ResetForTesting()
want := "shard, host: ks.0.replica, no valid tablet"
want := "target: ks.0.replica, no valid tablet"
err := f(dg, target)
verifyShardError(t, err, want, vtrpcpb.ErrorCode_INTERNAL_ERROR)

// tablet with error
hc.Reset()
dg.tsc.ResetForTesting()
hc.AddTestTablet("cell", "1.1.1.1", 1001, keyspace, shard, tabletType, false, 10, fmt.Errorf("no connection"))
want = "shard, host: ks.0.replica, no valid tablet"
want = "target: ks.0.replica, no valid tablet"
err = f(dg, target)
verifyShardError(t, err, want, vtrpcpb.ErrorCode_INTERNAL_ERROR)

// tablet without connection
hc.Reset()
dg.tsc.ResetForTesting()
ep1 := hc.AddTestTablet("cell", "1.1.1.1", 1001, keyspace, shard, tabletType, false, 10, nil).Tablet()
want = fmt.Sprintf(`shard, host: ks.0.replica, no valid tablet`)
want = fmt.Sprintf(`target: ks.0.replica, no valid tablet`)
err = f(dg, target)
verifyShardError(t, err, want, vtrpcpb.ErrorCode_INTERNAL_ERROR)

Expand All @@ -154,8 +154,8 @@ func testDiscoveryGatewayGeneric(t *testing.T, streaming bool, f func(dg Gateway
ep1 = sc1.Tablet()
ep2 := sc2.Tablet()
wants := map[string]int{
fmt.Sprintf(`shard, host: ks.0.replica, %+v, retry: err`, ep1): 0,
fmt.Sprintf(`shard, host: ks.0.replica, %+v, retry: err`, ep2): 0,
fmt.Sprintf(`target: ks.0.replica, used tablet: (%+v), retry: err`, ep1): 0,
fmt.Sprintf(`target: ks.0.replica, used tablet: (%+v), retry: err`, ep2): 0,
}
err = f(dg, target)
if _, ok := wants[fmt.Sprintf("%v", err)]; !ok {
Expand All @@ -172,8 +172,8 @@ func testDiscoveryGatewayGeneric(t *testing.T, streaming bool, f func(dg Gateway
ep1 = sc1.Tablet()
ep2 = sc2.Tablet()
wants = map[string]int{
fmt.Sprintf(`shard, host: ks.0.replica, %+v, fatal: err`, ep1): 0,
fmt.Sprintf(`shard, host: ks.0.replica, %+v, fatal: err`, ep2): 0,
fmt.Sprintf(`target: ks.0.replica, used tablet: (%+v), fatal: err`, ep1): 0,
fmt.Sprintf(`target: ks.0.replica, used tablet: (%+v), fatal: err`, ep2): 0,
}
err = f(dg, target)
if _, ok := wants[fmt.Sprintf("%v", err)]; !ok {
Expand All @@ -186,7 +186,7 @@ func testDiscoveryGatewayGeneric(t *testing.T, streaming bool, f func(dg Gateway
sc1 = hc.AddTestTablet("cell", "1.1.1.1", 1001, keyspace, shard, tabletType, true, 10, nil)
sc1.MustFailServer = 1
ep1 = sc1.Tablet()
want = fmt.Sprintf(`shard, host: ks.0.replica, %+v, error: err`, ep1)
want = fmt.Sprintf(`target: ks.0.replica, used tablet: (%+v), error: err`, ep1)
err = f(dg, target)
verifyShardError(t, err, want, vtrpcpb.ErrorCode_BAD_INPUT)

Expand All @@ -196,7 +196,7 @@ func testDiscoveryGatewayGeneric(t *testing.T, streaming bool, f func(dg Gateway
sc1 = hc.AddTestTablet("cell", "1.1.1.1", 1001, keyspace, shard, tabletType, true, 10, nil)
sc1.MustFailConn = 1
ep1 = sc1.Tablet()
want = fmt.Sprintf(`shard, host: ks.0.replica, %+v, error: conn`, ep1)
want = fmt.Sprintf(`target: ks.0.replica, used tablet: (%+v), error: conn`, ep1)
err = f(dg, target)
verifyShardError(t, err, want, vtrpcpb.ErrorCode_UNKNOWN_ERROR)

Expand Down Expand Up @@ -232,8 +232,8 @@ func testDiscoveryGatewayTransact(t *testing.T, streaming bool, f func(dg Gatewa
ep1 := sc1.Tablet()
ep2 := sc2.Tablet()
wants := map[string]int{
fmt.Sprintf(`shard, host: ks.0.replica, %+v, retry: err`, ep1): 0,
fmt.Sprintf(`shard, host: ks.0.replica, %+v, retry: err`, ep2): 0,
fmt.Sprintf(`target: ks.0.replica, used tablet: (%+v), retry: err`, ep1): 0,
fmt.Sprintf(`target: ks.0.replica, used tablet: (%+v), retry: err`, ep2): 0,
}
err := f(dg, target)
if _, ok := wants[fmt.Sprintf("%v", err)]; !ok {
Expand All @@ -246,7 +246,7 @@ func testDiscoveryGatewayTransact(t *testing.T, streaming bool, f func(dg Gatewa
sc1 = hc.AddTestTablet("cell", "1.1.1.1", 1001, keyspace, shard, tabletType, true, 10, nil)
sc1.MustFailConn = 1
ep1 = sc1.Tablet()
want := fmt.Sprintf(`shard, host: ks.0.replica, %+v, error: conn`, ep1)
want := fmt.Sprintf(`target: ks.0.replica, used tablet: (%+v), error: conn`, ep1)
err = f(dg, target)
verifyShardError(t, err, want, vtrpcpb.ErrorCode_UNKNOWN_ERROR)
}
Expand Down
6 changes: 3 additions & 3 deletions go/vt/vtgate/gateway/shard_error.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (e *ShardError) Error() string {
if e.ShardIdentifier == "" {
return fmt.Sprintf("%v", e.Err)
}
return fmt.Sprintf("shard, host: %s, %v", e.ShardIdentifier, e.Err)
return fmt.Sprintf("%s, %v", e.ShardIdentifier, e.Err)
}

// VtErrorCode returns the underlying Vitess error code.
Expand All @@ -52,9 +52,9 @@ func NewShardError(in error, target *querypb.Target, tablet *topodatapb.Tablet,
}
var shardIdentifier string
if tablet != nil {
shardIdentifier = fmt.Sprintf("%s.%s.%s, %+v", target.Keyspace, target.Shard, topoproto.TabletTypeLString(target.TabletType), tablet)
shardIdentifier = fmt.Sprintf("target: %s.%s.%s, used tablet: (%+v)", target.Keyspace, target.Shard, topoproto.TabletTypeLString(target.TabletType), tablet)
} else {
shardIdentifier = fmt.Sprintf("%s.%s.%s", target.Keyspace, target.Shard, topoproto.TabletTypeLString(target.TabletType))
shardIdentifier = fmt.Sprintf("target: %s.%s.%s", target.Keyspace, target.Shard, topoproto.TabletTypeLString(target.TabletType))
}

return &ShardError{
Expand Down
14 changes: 7 additions & 7 deletions go/vt/vtgate/resolver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ func testResolverGeneric(t *testing.T, name string, action func(res *Resolver) (
sbc0.MustFailServer = 1
sbc1.MustFailRetry = 1
_, err = action(res)
want1 := fmt.Sprintf("shard, host: %s.-20.master, alias:<cell:\"aa\" > hostname:\"-20\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"-20\" type:MASTER , error: err", name, name)
want2 := fmt.Sprintf("shard, host: %s.20-40.master, alias:<cell:\"aa\" > hostname:\"20-40\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"20-40\" type:MASTER , retry: err", name, name)
want1 := fmt.Sprintf("target: %s.-20.master, used tablet: (alias:<cell:\"aa\" > hostname:\"-20\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"-20\" type:MASTER ), error: err", name, name)
want2 := fmt.Sprintf("target: %s.20-40.master, used tablet: (alias:<cell:\"aa\" > hostname:\"20-40\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"20-40\" type:MASTER ), retry: err", name, name)
want := []string{want1, want2}
sort.Strings(want)
if err == nil {
Expand Down Expand Up @@ -237,8 +237,8 @@ func testResolverGeneric(t *testing.T, name string, action func(res *Resolver) (
sbc0.MustFailRetry = 1
sbc1.MustFailFatal = 1
_, err = action(res)
want1 = fmt.Sprintf("shard, host: %s.-20.master, alias:<cell:\"aa\" > hostname:\"-20\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"-20\" type:MASTER , retry: err", name, name)
want2 = fmt.Sprintf("shard, host: %s.20-40.master, alias:<cell:\"aa\" > hostname:\"20-40\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"20-40\" type:MASTER , fatal: err", name, name)
want1 = fmt.Sprintf("target: %s.-20.master, used tablet: (alias:<cell:\"aa\" > hostname:\"-20\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"-20\" type:MASTER ), retry: err", name, name)
want2 = fmt.Sprintf("target: %s.20-40.master, used tablet: (alias:<cell:\"aa\" > hostname:\"20-40\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"20-40\" type:MASTER ), fatal: err", name, name)
want = []string{want1, want2}
sort.Strings(want)
if err == nil {
Expand Down Expand Up @@ -382,7 +382,7 @@ func testResolverStreamGeneric(t *testing.T, name string, action func(res *Resol
hc.AddTestTablet("aa", "20-40", 1, name, "20-40", topodatapb.TabletType_MASTER, true, 1, nil)
sbc0.MustFailRetry = 1
_, err = action(res)
want := fmt.Sprintf("shard, host: %s.-20.master, alias:<cell:\"aa\" > hostname:\"-20\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"-20\" type:MASTER , retry: err", name, name)
want := fmt.Sprintf("target: %s.-20.master, used tablet: (alias:<cell:\"aa\" > hostname:\"-20\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"-20\" type:MASTER ), retry: err", name, name)
if err == nil || err.Error() != want {
t.Errorf("want\n%s\ngot\n%v", want, err)
}
Expand Down Expand Up @@ -501,7 +501,7 @@ func TestResolverExecBatchReresolve(t *testing.T) {
}

_, err := res.ExecuteBatch(context.Background(), topodatapb.TabletType_MASTER, false, nil, nil, buildBatchRequest)
want := "shard, host: TestResolverExecBatchReresolve.0.master, alias:<cell:\"aa\" > hostname:\"0\" port_map:<key:\"vt\" value:1 > keyspace:\"TestResolverExecBatchReresolve\" shard:\"0\" type:MASTER , retry: err"
want := "target: TestResolverExecBatchReresolve.0.master, used tablet: (alias:<cell:\"aa\" > hostname:\"0\" port_map:<key:\"vt\" value:1 > keyspace:\"TestResolverExecBatchReresolve\" shard:\"0\" type:MASTER ), retry: err"
if err == nil || err.Error() != want {
t.Errorf("want %s, got %v", want, err)
}
Expand Down Expand Up @@ -538,7 +538,7 @@ func TestResolverExecBatchAsTransaction(t *testing.T) {
}

_, err := res.ExecuteBatch(context.Background(), topodatapb.TabletType_MASTER, true, nil, nil, buildBatchRequest)
want := "shard, host: TestResolverExecBatchAsTransaction.0.master, alias:<cell:\"aa\" > hostname:\"0\" port_map:<key:\"vt\" value:1 > keyspace:\"TestResolverExecBatchAsTransaction\" shard:\"0\" type:MASTER , retry: err"
want := "target: TestResolverExecBatchAsTransaction.0.master, used tablet: (alias:<cell:\"aa\" > hostname:\"0\" port_map:<key:\"vt\" value:1 > keyspace:\"TestResolverExecBatchAsTransaction\" shard:\"0\" type:MASTER ), retry: err"
if err == nil || err.Error() != want {
t.Errorf("want %v, got %v", want, err)
}
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtgate/router_dml_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ func TestInsertFail(t *testing.T) {

sbclookup.MustFailServer = 1
_, err = routerExec(router, "insert into music(user_id, id) values (1, 2)", nil)
want = "execInsertSharded: lookup.Create: shard, host: TestUnsharded.0.master"
want = "execInsertSharded: lookup.Create: target: TestUnsharded.0.master"
if err == nil || !strings.HasPrefix(err.Error(), want) {
t.Errorf("routerExec: %v, want prefix %v", err, want)
}
Expand All @@ -657,7 +657,7 @@ func TestInsertFail(t *testing.T) {

sbc.MustFailServer = 1
_, err = routerExec(router, "insert into user(id, v, name) values (1, 2, 'myname')", nil)
want = "execInsertSharded: shard, host: TestRouter.-20.master"
want = "execInsertSharded: target: TestRouter.-20.master"
if err == nil || !strings.HasPrefix(err.Error(), want) {
t.Errorf("routerExec: %v, want prefix %v", err, want)
}
Expand Down
6 changes: 3 additions & 3 deletions go/vt/vtgate/scatter_conn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func testScatterConnGeneric(t *testing.T, name string, f func(sc *ScatterConn, s
sbc := hc.AddTestTablet("aa", "0", 1, name, "0", topodatapb.TabletType_REPLICA, true, 1, nil)
sbc.MustFailServer = 1
qr, err = f(sc, []string{"0"})
want := fmt.Sprintf("shard, host: %v.0.replica, alias:<cell:\"aa\" > hostname:\"0\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"0\" type:REPLICA , error: err", name, name)
want := fmt.Sprintf("target: %v.0.replica, used tablet: (alias:<cell:\"aa\" > hostname:\"0\" port_map:<key:\"vt\" value:1 > keyspace:\"%s\" shard:\"0\" type:REPLICA ), error: err", name, name)
// Verify server error string.
if err == nil || err.Error() != want {
t.Errorf("want %s, got %v", want, err)
Expand All @@ -148,7 +148,7 @@ func testScatterConnGeneric(t *testing.T, name string, f func(sc *ScatterConn, s
sbc1.MustFailServer = 1
_, err = f(sc, []string{"0", "1"})
// Verify server errors are consolidated.
want = fmt.Sprintf("shard, host: %v.0.replica, alias:<cell:\"aa\" > hostname:\"0\" port_map:<key:\"vt\" value:1 > keyspace:\"%v\" shard:\"0\" type:REPLICA , error: err\nshard, host: %v.1.replica, alias:<cell:\"aa\" > hostname:\"1\" port_map:<key:\"vt\" value:1 > keyspace:\"%v\" shard:\"1\" type:REPLICA , error: err", name, name, name, name)
want = fmt.Sprintf("target: %v.0.replica, used tablet: (alias:<cell:\"aa\" > hostname:\"0\" port_map:<key:\"vt\" value:1 > keyspace:\"%v\" shard:\"0\" type:REPLICA ), error: err\ntarget: %v.1.replica, used tablet: (alias:<cell:\"aa\" > hostname:\"1\" port_map:<key:\"vt\" value:1 > keyspace:\"%v\" shard:\"1\" type:REPLICA ), error: err", name, name, name, name)
verifyScatterConnError(t, err, want, vtrpcpb.ErrorCode_BAD_INPUT)
// Ensure that we tried only once.
if execCount := sbc0.ExecCount.Get(); execCount != 1 {
Expand All @@ -168,7 +168,7 @@ func testScatterConnGeneric(t *testing.T, name string, f func(sc *ScatterConn, s
sbc1.MustFailTxPool = 1
_, err = f(sc, []string{"0", "1"})
// Verify server errors are consolidated.
want = fmt.Sprintf("shard, host: %v.0.replica, alias:<cell:\"aa\" > hostname:\"0\" port_map:<key:\"vt\" value:1 > keyspace:\"%v\" shard:\"0\" type:REPLICA , error: err\nshard, host: %v.1.replica, alias:<cell:\"aa\" > hostname:\"1\" port_map:<key:\"vt\" value:1 > keyspace:\"%v\" shard:\"1\" type:REPLICA , tx_pool_full: err", name, name, name, name)
want = fmt.Sprintf("target: %v.0.replica, used tablet: (alias:<cell:\"aa\" > hostname:\"0\" port_map:<key:\"vt\" value:1 > keyspace:\"%v\" shard:\"0\" type:REPLICA ), error: err\ntarget: %v.1.replica, used tablet: (alias:<cell:\"aa\" > hostname:\"1\" port_map:<key:\"vt\" value:1 > keyspace:\"%v\" shard:\"1\" type:REPLICA ), tx_pool_full: err", name, name, name, name)
// We should only surface the higher priority error code
verifyScatterConnError(t, err, want, vtrpcpb.ErrorCode_BAD_INPUT)
// Ensure that we tried only once.
Expand Down

0 comments on commit 0853e53

Please sign in to comment.