From adc03bf36dec9772ebdbc7c98fb04bcdcaf8d968 Mon Sep 17 00:00:00 2001 From: Richard Date: Thu, 17 Sep 2020 01:13:22 -0700 Subject: [PATCH] Change GRPC message limit SQL error type (#6630) * impl+log Signed-off-by: Richard Bailey * correctly escape regex Signed-off-by: Richard Bailey * simple tests, remove logging Signed-off-by: Richard Bailey * years are dumb Signed-off-by: Richard Bailey * handle the other tow RE cases. this is ... mabye a bit much with the factoring out but maybe not Signed-off-by: Richard Bailey * fix up comments, move impls around Signed-off-by: Richard Bailey * better better error comment <_< Signed-off-by: Richard Bailey * assert! Signed-off-by: Richard Bailey * to run test suite Signed-off-by: Richard Bailey * fix up testsv Signed-off-by: Richard Bailey * tests pass; remove dead code-as-comments Signed-off-by: Richard Bailey * pull out unnecessary processing Signed-off-by: Richard Bailey * don't differentiate trailing vs leading clarification Signed-off-by: Richard Bailey --- go/mysql/sql_error.go | 13 ++++++- go/mysql/sql_error_test.go | 45 ++++++++++++++++++++++++ go/vt/vtgate/executor.go | 2 +- go/vt/vtgate/executor_test.go | 4 +-- go/vt/vtgate/legacy_scatter_conn_test.go | 2 +- go/vt/vtgate/scatter_conn.go | 2 +- 6 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 go/mysql/sql_error_test.go diff --git a/go/mysql/sql_error.go b/go/mysql/sql_error.go index fcbaa8c810b..42f65b0a96b 100644 --- a/go/mysql/sql_error.go +++ b/go/mysql/sql_error.go @@ -120,7 +120,7 @@ func NewSQLErrorFromError(err error) error { case vtrpcpb.Code_UNAUTHENTICATED: num = ERAccessDeniedError case vtrpcpb.Code_RESOURCE_EXHAUSTED: - num = ERTooManyUserConnections + num = demuxResourceExhaustedErrors(err.Error()) case vtrpcpb.Code_FAILED_PRECONDITION: num = ERUnknownError case vtrpcpb.Code_ABORTED: @@ -161,3 +161,14 @@ func NewSQLErrorFromError(err error) error { } return serr } + +var isGRPCOverflowRE = regexp.MustCompile(`.*grpc: received message larger than max \(\d+ vs. \d+\)`) + +func demuxResourceExhaustedErrors(msg string) int { + switch { + case isGRPCOverflowRE.Match([]byte(msg)): + return ERNetPacketTooLarge + default: + return ERTooManyUserConnections + } +} diff --git a/go/mysql/sql_error_test.go b/go/mysql/sql_error_test.go new file mode 100644 index 00000000000..1d8d0339ffe --- /dev/null +++ b/go/mysql/sql_error_test.go @@ -0,0 +1,45 @@ +/* +Copyright 2020 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mysql + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDumuxResourceExhaustedErrors(t *testing.T) { + type testCase struct { + msg string + want int + } + + cases := []testCase{ + testCase{"misc", ERTooManyUserConnections}, + testCase{"grpc: received message larger than max (99282 vs. 1234): trailer", ERNetPacketTooLarge}, + testCase{"grpc: received message larger than max (1234 vs. 1234)", ERNetPacketTooLarge}, + testCase{"header: grpc: received message larger than max (1234 vs. 1234)", ERNetPacketTooLarge}, + // This should be explicitly handled by returning ERNetPacketTooLarge from the execturo directly + // and therefore shouldn't need to be teased out of another error. + testCase{"in-memory row count exceeded allowed limit of 13", ERTooManyUserConnections}, + } + + for _, c := range cases { + got := demuxResourceExhaustedErrors(c.msg) + assert.Equalf(t, c.want, got, c.msg) + } +} diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index c815d62fdf8..3f72ac0217c 100644 --- a/go/vt/vtgate/executor.go +++ b/go/vt/vtgate/executor.go @@ -1450,7 +1450,7 @@ func (e *Executor) getPlan(vcursor *vcursorImpl, sql string, comments sqlparser. statement := stmt bindVarNeeds := sqlparser.BindVarNeeds{} if !sqlparser.IgnoreMaxPayloadSizeDirective(statement) && !isValidPayloadSize(query) { - return nil, vterrors.New(vtrpcpb.Code_RESOURCE_EXHAUSTED, "query payload size above threshold") + return nil, mysql.NewSQLError(mysql.ERNetPacketTooLarge, "", "query payload size above threshold") } ignoreMaxMemoryRows := sqlparser.IgnoreMaxMaxMemoryRowsDirective(stmt) vcursor.SetIgnoreMaxMemoryRows(ignoreMaxMemoryRows) diff --git a/go/vt/vtgate/executor_test.go b/go/vt/vtgate/executor_test.go index 70900cbea3f..735a422c3db 100644 --- a/go/vt/vtgate/executor_test.go +++ b/go/vt/vtgate/executor_test.go @@ -92,7 +92,7 @@ func TestExecutorMaxMemoryRowsExceeded(t *testing.T) { err string }{ {"select /*vt+ IGNORE_MAX_MEMORY_ROWS=1 */ * from main1", ""}, - {"select * from main1", "in-memory row count exceeded allowed limit of 3"}, + {"select * from main1", "in-memory row count exceeded allowed limit of 3 (errno 1153) (sqlstate HY000)"}, } for _, test := range testCases { @@ -1896,7 +1896,7 @@ func TestExecutorMaxPayloadSizeExceeded(t *testing.T) { for _, query := range testMaxPayloadSizeExceeded { _, err := executor.Execute(context.Background(), "TestExecutorMaxPayloadSizeExceeded", session, query, nil) require.NotNil(t, err) - assert.EqualError(t, err, "query payload size above threshold") + assert.EqualError(t, err, "query payload size above threshold (errno 1153) (sqlstate HY000)") } assert.Equal(t, warningCount, warnings.Counts()["WarnPayloadSizeExceeded"], "warnings count") diff --git a/go/vt/vtgate/legacy_scatter_conn_test.go b/go/vt/vtgate/legacy_scatter_conn_test.go index f5bc4a4f2af..a97a51353d6 100644 --- a/go/vt/vtgate/legacy_scatter_conn_test.go +++ b/go/vt/vtgate/legacy_scatter_conn_test.go @@ -315,7 +315,7 @@ func TestMaxMemoryRows(t *testing.T) { err string }{ {true, ""}, - {false, "in-memory row count exceeded allowed limit of 3"}, + {false, "in-memory row count exceeded allowed limit of 3 (errno 1153) (sqlstate HY000)"}, } for _, test := range testCases { diff --git a/go/vt/vtgate/scatter_conn.go b/go/vt/vtgate/scatter_conn.go index 342f80778f5..505653b5e58 100644 --- a/go/vt/vtgate/scatter_conn.go +++ b/go/vt/vtgate/scatter_conn.go @@ -233,7 +233,7 @@ func (stc *ScatterConn) ExecuteMultiShard( ) if !ignoreMaxMemoryRows && len(qr.Rows) > *maxMemoryRows { - return nil, []error{vterrors.Errorf(vtrpcpb.Code_RESOURCE_EXHAUSTED, "in-memory row count exceeded allowed limit of %d", *maxMemoryRows)} + return nil, []error{mysql.NewSQLError(mysql.ERNetPacketTooLarge, "", "in-memory row count exceeded allowed limit of %d", *maxMemoryRows)} } return qr, allErrors.GetErrors()