зеркало из https://github.com/github/vitess-gh.git
Merge pull request #2364 from erzel/fixing_panic_for_error_split_query
Fixed the panic-handling code in vttablet.
This commit is contained in:
Erez Louidor
GitHub
Коммит
daacee8f04
|
|
@ -64,7 +64,8 @@ func Errorf(msg string, args ...interface{}) error {
|
|||
return stackError{fmt.Errorf(msg, args...), stack}
|
||||
}
|
||||
|
||||
// Taken from runtime/debug.go
|
||||
// Stack is taken from runtime/debug.go
|
||||
// calldepth is the number of (bottommost) frames to skip.
|
||||
func Stack(calldepth int) []byte {
|
||||
return stack(calldepth)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -971,12 +971,12 @@ func (tsv *TabletServer) ExecuteBatch(ctx context.Context, target *querypb.Targe
|
|||
return nil, err
|
||||
}
|
||||
defer tsv.endRequest(false)
|
||||
defer tsv.handleError("batch", nil, &err, nil)
|
||||
defer tsv.handlePanicAndSendLogStats("batch", nil, &err, nil)
|
||||
|
||||
if asTransaction {
|
||||
transactionID, err = tsv.Begin(ctx, target)
|
||||
if err != nil {
|
||||
return nil, tsv.handleErrorNoPanic("batch", nil, err, nil)
|
||||
return nil, tsv.handleError("batch", nil, err, nil)
|
||||
}
|
||||
// If transaction was not committed by the end, it means
|
||||
// that there was an error, roll it back.
|
||||
|
|
@ -990,14 +990,14 @@ func (tsv *TabletServer) ExecuteBatch(ctx context.Context, target *querypb.Targe
|
|||
for _, bound := range queries {
|
||||
localReply, err := tsv.Execute(ctx, target, bound.Sql, bound.BindVariables, transactionID, options)
|
||||
if err != nil {
|
||||
return nil, tsv.handleErrorNoPanic("batch", nil, err, nil)
|
||||
return nil, tsv.handleError("batch", nil, err, nil)
|
||||
}
|
||||
results = append(results, *localReply)
|
||||
}
|
||||
if asTransaction {
|
||||
if err = tsv.Commit(ctx, target, transactionID); err != nil {
|
||||
transactionID = 0
|
||||
return nil, tsv.handleErrorNoPanic("batch", nil, err, nil)
|
||||
return nil, tsv.handleError("batch", nil, err, nil)
|
||||
}
|
||||
transactionID = 0
|
||||
}
|
||||
|
|
@ -1081,7 +1081,7 @@ func (tsv *TabletServer) SplitQuery(
|
|||
defer sqlExecuter.done()
|
||||
algorithmObject, err := createSplitQueryAlgorithmObject(algorithm, splitParams, sqlExecuter)
|
||||
if err != nil {
|
||||
return err
|
||||
return splitQueryToTabletError(err)
|
||||
}
|
||||
splits, err = splitquery.NewSplitter(splitParams, algorithmObject).Split()
|
||||
if err != nil {
|
||||
|
|
@ -1105,7 +1105,7 @@ func (tsv *TabletServer) execRequest(
|
|||
logStats.Target = target
|
||||
logStats.OriginalSQL = sql
|
||||
logStats.BindVariables = bindVariables
|
||||
defer tsv.handleError(sql, bindVariables, &err, logStats)
|
||||
defer tsv.handlePanicAndSendLogStats(sql, bindVariables, &err, logStats)
|
||||
if err = tsv.startRequest(target, isTx, allowOnShutdown); err != nil {
|
||||
return err
|
||||
}
|
||||
|
|
@ -1117,23 +1117,51 @@ func (tsv *TabletServer) execRequest(
|
|||
|
||||
err = exec(ctx, logStats)
|
||||
if err != nil {
|
||||
return tsv.handleErrorNoPanic(sql, bindVariables, err, logStats)
|
||||
return tsv.handleError(sql, bindVariables, err, logStats)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// handleError handles panics during query execution and sets
|
||||
// the supplied error return value.
|
||||
func (tsv *TabletServer) handleError(sql string, bindVariables map[string]interface{}, err *error, logStats *LogStats) {
|
||||
func (tsv *TabletServer) handlePanicAndSendLogStats(
|
||||
sql string,
|
||||
bindVariables map[string]interface{},
|
||||
err *error,
|
||||
logStats *LogStats,
|
||||
) {
|
||||
if x := recover(); x != nil {
|
||||
*err = tsv.handleErrorNoPanic(sql, bindVariables, x, logStats)
|
||||
terr, ok := x.(*TabletError)
|
||||
if ok {
|
||||
// If the thrown error is a *TabletError then it's a (deprecated)
|
||||
// use of panic to report a "regular" (i.e. non-logic) error;
|
||||
// thus we call handleError.
|
||||
// TODO(erez): Remove this once we report all regular
|
||||
// errors without panic()'s.
|
||||
*err = tsv.handleError(sql, bindVariables, terr, logStats)
|
||||
} else {
|
||||
errorMessage := fmt.Sprintf(
|
||||
"Uncaught panic for %v:\n%v\n%s",
|
||||
querytypes.QueryAsString(sql, bindVariables),
|
||||
x,
|
||||
tb.Stack(4) /* Skip the last 4 boiler-plate frames. */)
|
||||
log.Errorf(errorMessage)
|
||||
*err = NewTabletError(vtrpcpb.ErrorCode_UNKNOWN_ERROR, errorMessage)
|
||||
tsv.qe.queryServiceStats.InternalErrors.Add("Panic", 1)
|
||||
if logStats != nil {
|
||||
logStats.Error = nil
|
||||
}
|
||||
}
|
||||
}
|
||||
if logStats != nil {
|
||||
logStats.Send()
|
||||
}
|
||||
}
|
||||
|
||||
func (tsv *TabletServer) handleErrorNoPanic(sql string, bindVariables map[string]interface{}, err interface{}, logStats *LogStats) error {
|
||||
func (tsv *TabletServer) handleError(
|
||||
sql string,
|
||||
bindVariables map[string]interface{},
|
||||
err error,
|
||||
logStats *LogStats,
|
||||
) error {
|
||||
var terr *TabletError
|
||||
defer func() {
|
||||
if logStats != nil {
|
||||
|
|
@ -1142,9 +1170,7 @@ func (tsv *TabletServer) handleErrorNoPanic(sql string, bindVariables map[string
|
|||
}()
|
||||
terr, ok := err.(*TabletError)
|
||||
if !ok {
|
||||
log.Errorf("Uncaught panic for %v:\n%v\n%s", querytypes.QueryAsString(sql, bindVariables), err, tb.Stack(4))
|
||||
tsv.qe.queryServiceStats.InternalErrors.Add("Panic", 1)
|
||||
return NewTabletError(vtrpcpb.ErrorCode_UNKNOWN_ERROR, "%v: uncaught panic for %v", err, querytypes.QueryAsString(sql, bindVariables))
|
||||
panic(fmt.Sprintf("Got an error that is not a TabletError: %v", err))
|
||||
}
|
||||
var myError error
|
||||
if tsv.config.TerseErrors && terr.SQLError != 0 && len(bindVariables) != 0 {
|
||||
|
|
@ -1356,7 +1382,7 @@ func createSplitQueryAlgorithmObject(
|
|||
}
|
||||
|
||||
// splitQueryToTabletError converts the given error assumed to be returned from the
|
||||
// splitquery-package into a TabletError suitablet to be returned to the caller.
|
||||
// splitquery-package into a TabletError suitable to be returned to the caller.
|
||||
// It returns nil if 'err' is nil.
|
||||
func splitQueryToTabletError(err error) error {
|
||||
if err == nil {
|
||||
|
|
|
|||
|
|
@ -1489,6 +1489,40 @@ func TestTabletServerSplitQueryInvalidParams(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// Tests that using Equal Splits on a string column returns an error.
|
||||
func TestTabletServerSplitQueryEqualSplitsOnStringColumn(t *testing.T) {
|
||||
db := setUpTabletServerTest()
|
||||
testUtils := newTestUtils()
|
||||
config := testUtils.newQueryServiceConfig()
|
||||
tsv := NewTabletServer(config)
|
||||
dbconfigs := testUtils.newDBConfigs(db)
|
||||
target := querypb.Target{TabletType: topodatapb.TabletType_RDONLY}
|
||||
err := tsv.StartService(target, dbconfigs, testUtils.newMysqld(&dbconfigs))
|
||||
if err != nil {
|
||||
t.Fatalf("StartService failed: %v", err)
|
||||
}
|
||||
defer tsv.StopService()
|
||||
ctx := context.Background()
|
||||
sql := "select * from test_table"
|
||||
_, err = tsv.SplitQuery(
|
||||
ctx,
|
||||
&querypb.Target{TabletType: topodatapb.TabletType_RDONLY},
|
||||
sql,
|
||||
nil, /* bindVariables */
|
||||
// EQUAL_SPLITS should not work on a string column.
|
||||
[]string{"name_string"}, /* splitColumns */
|
||||
10, /* splitCount */
|
||||
0, /* numRowsPerQueryPart */
|
||||
querypb.SplitQueryRequest_EQUAL_SPLITS)
|
||||
want :=
|
||||
"error: splitquery: using the EQUAL_SPLITS algorithm in SplitQuery" +
|
||||
" requires having a numeric (integral or float) split-column." +
|
||||
" Got type: {Name: 'name_string', Type: VARCHAR}"
|
||||
if err.Error() != want {
|
||||
t.Fatalf("got: %v, want: %v", err, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleExecUnknownError(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
logStats := NewLogStats(ctx, "TestHandleExecError")
|
||||
|
|
@ -1496,7 +1530,7 @@ func TestHandleExecUnknownError(t *testing.T) {
|
|||
testUtils := newTestUtils()
|
||||
config := testUtils.newQueryServiceConfig()
|
||||
tsv := NewTabletServer(config)
|
||||
defer tsv.handleError("select * from test_table", nil, &err, logStats)
|
||||
defer tsv.handlePanicAndSendLogStats("select * from test_table", nil, &err, logStats)
|
||||
panic("unknown exec error")
|
||||
}
|
||||
|
||||
|
|
@ -1513,7 +1547,7 @@ func TestHandleExecTabletError(t *testing.T) {
|
|||
testUtils := newTestUtils()
|
||||
config := testUtils.newQueryServiceConfig()
|
||||
tsv := NewTabletServer(config)
|
||||
defer tsv.handleError("select * from test_table", nil, &err, logStats)
|
||||
defer tsv.handlePanicAndSendLogStats("select * from test_table", nil, &err, logStats)
|
||||
panic(NewTabletError(vtrpcpb.ErrorCode_INTERNAL_ERROR, "tablet error"))
|
||||
}
|
||||
|
||||
|
|
@ -1531,7 +1565,7 @@ func TestTerseErrorsNonSQLError(t *testing.T) {
|
|||
config := testUtils.newQueryServiceConfig()
|
||||
tsv := NewTabletServer(config)
|
||||
tsv.config.TerseErrors = true
|
||||
defer tsv.handleError("select * from test_table", nil, &err, logStats)
|
||||
defer tsv.handlePanicAndSendLogStats("select * from test_table", nil, &err, logStats)
|
||||
panic(NewTabletError(vtrpcpb.ErrorCode_INTERNAL_ERROR, "tablet error"))
|
||||
}
|
||||
|
||||
|
|
@ -1549,7 +1583,11 @@ func TestTerseErrorsBindVars(t *testing.T) {
|
|||
config := testUtils.newQueryServiceConfig()
|
||||
tsv := NewTabletServer(config)
|
||||
tsv.config.TerseErrors = true
|
||||
defer tsv.handleError("select * from test_table", map[string]interface{}{"a": 1}, &err, logStats)
|
||||
defer tsv.handlePanicAndSendLogStats(
|
||||
"select * from test_table",
|
||||
map[string]interface{}{"a": 1},
|
||||
&err,
|
||||
logStats)
|
||||
panic(&TabletError{
|
||||
ErrorCode: vtrpcpb.ErrorCode_DEADLINE_EXCEEDED,
|
||||
Message: "msg",
|
||||
|
|
@ -1572,7 +1610,7 @@ func TestTerseErrorsNoBindVars(t *testing.T) {
|
|||
config := testUtils.newQueryServiceConfig()
|
||||
tsv := NewTabletServer(config)
|
||||
tsv.config.TerseErrors = true
|
||||
defer tsv.handleError("select * from test_table", nil, &err, logStats)
|
||||
defer tsv.handlePanicAndSendLogStats("select * from test_table", nil, &err, logStats)
|
||||
panic(&TabletError{
|
||||
ErrorCode: vtrpcpb.ErrorCode_DEADLINE_EXCEEDED,
|
||||
Message: "msg",
|
||||
|
|
@ -1713,10 +1751,34 @@ func getSupportedQueries() map[string]*sqltypes.Result {
|
|||
},
|
||||
},
|
||||
"select * from test_table where 1 != 1": {
|
||||
Fields: getTestTableFields(),
|
||||
Fields: []*querypb.Field{{
|
||||
Name: "pk",
|
||||
Type: sqltypes.Int32,
|
||||
}, {
|
||||
Name: "name",
|
||||
Type: sqltypes.Int32,
|
||||
}, {
|
||||
Name: "addr",
|
||||
Type: sqltypes.Int32,
|
||||
}, {
|
||||
Name: "name_string",
|
||||
Type: sqltypes.VarChar,
|
||||
}},
|
||||
},
|
||||
"select * from `test_table` where 1 != 1": {
|
||||
Fields: getTestTableFields(),
|
||||
Fields: []*querypb.Field{{
|
||||
Name: "pk",
|
||||
Type: sqltypes.Int32,
|
||||
}, {
|
||||
Name: "name",
|
||||
Type: sqltypes.Int32,
|
||||
}, {
|
||||
Name: "addr",
|
||||
Type: sqltypes.Int32,
|
||||
}, {
|
||||
Name: "name_string",
|
||||
Type: sqltypes.VarChar,
|
||||
}},
|
||||
},
|
||||
baseShowTables: {
|
||||
RowsAffected: 1,
|
||||
|
|
@ -1735,7 +1797,7 @@ func getSupportedQueries() map[string]*sqltypes.Result {
|
|||
},
|
||||
},
|
||||
"describe `test_table`": {
|
||||
RowsAffected: 3,
|
||||
RowsAffected: 4,
|
||||
Rows: [][]sqltypes.Value{
|
||||
{
|
||||
sqltypes.MakeString([]byte("pk")),
|
||||
|
|
@ -1761,11 +1823,19 @@ func getSupportedQueries() map[string]*sqltypes.Result {
|
|||
sqltypes.MakeString([]byte("1")),
|
||||
sqltypes.MakeString([]byte{}),
|
||||
},
|
||||
{
|
||||
sqltypes.MakeString([]byte("name_string")), /* Field */
|
||||
sqltypes.MakeString([]byte("varchar(10)")), /* Type */
|
||||
sqltypes.MakeString([]byte{}), /* NULL */
|
||||
sqltypes.MakeString([]byte{}), /* Key */
|
||||
sqltypes.MakeString([]byte("foo")), /* Default Value */
|
||||
sqltypes.MakeString([]byte{}), /* Extra */
|
||||
},
|
||||
},
|
||||
},
|
||||
// for SplitQuery because it needs a primary key column
|
||||
"show index from `test_table`": {
|
||||
RowsAffected: 2,
|
||||
RowsAffected: 3,
|
||||
Rows: [][]sqltypes.Value{
|
||||
{
|
||||
sqltypes.MakeString([]byte{}),
|
||||
|
|
@ -1785,6 +1855,15 @@ func getSupportedQueries() map[string]*sqltypes.Result {
|
|||
sqltypes.MakeString([]byte{}),
|
||||
sqltypes.MakeString([]byte("300")),
|
||||
},
|
||||
{
|
||||
sqltypes.MakeString([]byte{}),
|
||||
sqltypes.MakeString([]byte{}),
|
||||
sqltypes.MakeString([]byte("name_string_INDEX")),
|
||||
sqltypes.MakeString([]byte{}),
|
||||
sqltypes.MakeString([]byte("name_string")),
|
||||
sqltypes.MakeString([]byte{}),
|
||||
sqltypes.MakeString([]byte("100")),
|
||||
},
|
||||
},
|
||||
},
|
||||
"begin": {},
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче