From 05d1d8008f0679ca790d064b6c995417f6111757 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Wed, 22 Mar 2023 01:33:14 +0100 Subject: [PATCH 01/12] WIP Signed-off-by: Tim Vaillancourt --- go/vt/vttablet/tabletserver/query_executor.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index ba9f12bf5cf..aef38bce8ac 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -934,6 +934,10 @@ func (qre *QueryExecutor) generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, b buf.WriteString(qre.marginComments.Leading) qre.marginComments.Leading = buf.String() } + if qre.tsv.config.QueryTimeoutMethod == "mysql" { + timeout := qre.tsv.config.Oltp.QueryTimeout + query = addMySQLMaxExecutionTimeComment(query, qre.tsv.config.QueryTimeout) + } if qre.marginComments.Leading == "" && qre.marginComments.Trailing == "" { return query, query, nil @@ -947,6 +951,15 @@ func (qre *QueryExecutor) generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, b return buf.String(), query, nil } +func addMySQLMaxExecutionTimeComment(query string, queryTimeoutMs int64) string { + fields := strings.SplitN(query, " ", 2) + return strings.Join([]string{ + fields[0], + fmt.Sprintf("/*+ MAX_EXECUTION_TIME(%d) */", queryTimeoutMs), + fields[1], + }, " ") +} + func rewriteOUTParamError(err error) error { sqlErr, ok := err.(*mysql.SQLError) if !ok { From 63379ffb04cb69a377efe28fe9df6d8f201ce2c4 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 4 Nov 2023 02:40:02 +0100 Subject: [PATCH 02/12] WIP again Signed-off-by: Tim Vaillancourt --- go/vt/vttablet/tabletserver/query_executor.go | 38 ++++++++++++------- .../tabletserver/query_executor_test.go | 13 +++++++ .../vttablet/tabletserver/tabletenv/config.go | 17 ++++++--- go/vt/vttablet/tabletserver/tabletserver.go | 7 +++- 4 files changed, 55 insertions(+), 20 deletions(-) diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index aef38bce8ac..53ee3c9769b 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -65,8 +65,9 @@ type QueryExecutor struct { } const ( - streamRowsSize = 256 - maxQueryBufferDuration = 15 * time.Second + streamRowsSize = 256 + maxQueryBufferDuration = 15 * time.Second + queryTimeoutMysqlMaxWait = time.Second ) var ( @@ -918,6 +919,7 @@ func (qre *QueryExecutor) generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, b if err != nil { return "", "", vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "%s", err) } + query = addMySQLOptimizerHints(qre.tsv.config, query) if qre.tsv.config.AnnotateQueries { username := callerid.GetPrincipal(callerid.EffectiveCallerIDFromContext(qre.ctx)) if username == "" { @@ -934,10 +936,6 @@ func (qre *QueryExecutor) generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, b buf.WriteString(qre.marginComments.Leading) qre.marginComments.Leading = buf.String() } - if qre.tsv.config.QueryTimeoutMethod == "mysql" { - timeout := qre.tsv.config.Oltp.QueryTimeout - query = addMySQLMaxExecutionTimeComment(query, qre.tsv.config.QueryTimeout) - } if qre.marginComments.Leading == "" && qre.marginComments.Trailing == "" { return query, query, nil @@ -951,13 +949,27 @@ func (qre *QueryExecutor) generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, b return buf.String(), query, nil } -func addMySQLMaxExecutionTimeComment(query string, queryTimeoutMs int64) string { - fields := strings.SplitN(query, " ", 2) - return strings.Join([]string{ - fields[0], - fmt.Sprintf("/*+ MAX_EXECUTION_TIME(%d) */", queryTimeoutMs), - fields[1], - }, " ") +func addMySQLOptimizerHints(config *tabletenv.TabletConfig, query string) string { + hints := make([]string, 0) + + if config.Oltp.QueryTimeoutMethod == "mysql" && config.Oltp.QueryTimeoutSeconds > 0 { + // The MAX_EXECUTION_TIME(N) hint sets a statement execution timeout of N milliseconds. + // https://dev.mysql.com/doc/refman/8.0/en/optimizer-hints.html#optimizer-hints-execution-time + hints = append(hints, + fmt.Sprintf("MAX_EXECUTION_TIME(%d)", config.Oltp.QueryTimeoutSeconds.Get().Milliseconds()), + ) + } + + if len(hints) > 0 { + fields := strings.SplitN(query, " ", 2) + return strings.Join([]string{ + fields[0], + "/*+ " + strings.Join(hints, " ") + " */", + fields[1], + }, " ") + } + + return query } func rewriteOUTParamError(err error) error { diff --git a/go/vt/vttablet/tabletserver/query_executor_test.go b/go/vt/vttablet/tabletserver/query_executor_test.go index 10dd261f975..4b7be6b1b00 100644 --- a/go/vt/vttablet/tabletserver/query_executor_test.go +++ b/go/vt/vttablet/tabletserver/query_executor_test.go @@ -1786,3 +1786,16 @@ func TestQueryExecSchemaReloadCount(t *testing.T) { }) } } + +func TestAddMySQLOptimizerHints(t *testing.T) { + config := tabletenv.NewDefaultConfig() + { + config.Oltp.QueryTimeoutMethod = "vttablet" + t.Logf("sql: %v", addMySQLOptimizerHints(config, "select * from something")) + } + { + config.Oltp.QueryTimeoutMethod = "mysql" + config.Oltp.QueryTimeoutSeconds = tabletenv.Seconds(1) + t.Logf("sql: %v", addMySQLOptimizerHints(config, "select * from something")) + } +} diff --git a/go/vt/vttablet/tabletserver/tabletenv/config.go b/go/vt/vttablet/tabletserver/tabletenv/config.go index 14516354c9d..92ceb375772 100644 --- a/go/vt/vttablet/tabletserver/tabletenv/config.go +++ b/go/vt/vttablet/tabletserver/tabletenv/config.go @@ -37,12 +37,13 @@ import ( // These constants represent values for various config parameters. const ( - Enable = "enable" - Disable = "disable" - Dryrun = "dryRun" - NotOnPrimary = "notOnPrimary" - Polling = "polling" - Heartbeat = "heartbeat" + Enable = "enable" + Disable = "disable" + Dryrun = "dryRun" + NotOnPrimary = "notOnPrimary" + Polling = "polling" + Heartbeat = "heartbeat" + DefaultQueryTimeoutMethod = "vttablet" ) var ( @@ -183,6 +184,8 @@ func registerTabletEnvFlags(fs *pflag.FlagSet) { fs.Int64Var(¤tConfig.RowStreamer.MaxMySQLReplLagSecs, "vreplication_copy_phase_max_mysql_replication_lag", 43200, "The maximum MySQL replication lag (in seconds) that can exist on a vstreamer (source) before starting another round of copying rows. This helps to limit the impact on the source tablet.") fs.BoolVar(¤tConfig.EnableViews, "queryserver-enable-views", false, "Enable views support in vttablet.") + + fs.StringVar(¤tConfig.Oltp.QueryTimeoutMethod, "query-timeout-method", defaultConfig.Oltp.QueryTimeoutMethod, "The method to be used to kill MySQL queries, options: 'vttablet' and 'mysql'. 'vttablet' issues a MySQL KILL operation whereas 'mysql' pushes the kill to MySQL.") } var ( @@ -346,6 +349,7 @@ type OlapConfig struct { // OltpConfig contains the config for oltp settings. type OltpConfig struct { QueryTimeoutSeconds Seconds `json:"queryTimeoutSeconds,omitempty"` + QueryTimeoutMethod string `json:"queryTimeoutMethod,omitempty"` TxTimeoutSeconds Seconds `json:"txTimeoutSeconds,omitempty"` MaxRows int `json:"maxRows,omitempty"` WarnRows int `json:"warnRows,omitempty"` @@ -518,6 +522,7 @@ var defaultConfig = TabletConfig{ }, Oltp: OltpConfig{ QueryTimeoutSeconds: 30, + QueryTimeoutMethod: DefaultQueryTimeoutMethod, TxTimeoutSeconds: 30, MaxRows: 10000, }, diff --git a/go/vt/vttablet/tabletserver/tabletserver.go b/go/vt/vttablet/tabletserver/tabletserver.go index 61c7bb356de..bbc835fa17d 100644 --- a/go/vt/vttablet/tabletserver/tabletserver.go +++ b/go/vt/vttablet/tabletserver/tabletserver.go @@ -159,7 +159,12 @@ func NewTabletServer(name string, config *tabletenv.TabletConfig, topoServer *to topoServer: topoServer, alias: proto.Clone(alias).(*topodatapb.TabletAlias), } - tsv.QueryTimeout.Store(config.Oltp.QueryTimeoutSeconds.Get().Nanoseconds()) + + queryTimeoutNanos := config.Oltp.QueryTimeoutSeconds.Get().Nanoseconds() + if config.Oltp.QueryTimeoutMethod == "mysql" { + queryTimeoutNanos = queryTimeoutNanos + queryTimeoutMysqlMaxWait.Nanoseconds() + } + tsv.QueryTimeout.Store(queryTimeoutNanos) tsOnce.Do(func() { srvTopoServer = srvtopo.NewResilientServer(topoServer, "TabletSrvTopo") }) From 14ddbc880ec5e826ad21e0046f6a26fd970ba390 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 4 Nov 2023 03:10:58 +0100 Subject: [PATCH 03/12] Fix tests after rebase Signed-off-by: Tim Vaillancourt --- go/vt/vttablet/tabletserver/query_executor.go | 12 +++++------ .../tabletserver/query_executor_test.go | 18 ++++++++++------ .../vttablet/tabletserver/tabletenv/config.go | 21 ++++++++++--------- go/vt/vttablet/tabletserver/tabletserver.go | 3 ++- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index c3a9f719ff5..d505021ae71 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -24,11 +24,9 @@ import ( "sync" "time" + "vitess.io/vitess/go/mysql/collations" "vitess.io/vitess/go/mysql/replication" "vitess.io/vitess/go/mysql/sqlerror" - - "vitess.io/vitess/go/mysql/collations" - "vitess.io/vitess/go/pools" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/trace" @@ -68,6 +66,7 @@ type QueryExecutor struct { const ( streamRowsSize = 256 + queryTimeoutMethodMySQL = "mysql" queryTimeoutMysqlMaxWait = time.Second ) @@ -824,7 +823,7 @@ func (qre *QueryExecutor) generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, b if err != nil { return "", "", vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "%s", err) } - query = addMySQLOptimizerHints(qre.tsv.config, query) + query = addMysqlOptimizerHintsToQuery(qre.tsv.config, query) if qre.tsv.config.AnnotateQueries { username := callerid.GetPrincipal(callerid.EffectiveCallerIDFromContext(qre.ctx)) if username == "" { @@ -854,10 +853,11 @@ func (qre *QueryExecutor) generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, b return buf.String(), query, nil } -func addMySQLOptimizerHints(config *tabletenv.TabletConfig, query string) string { +func addMysqlOptimizerHintsToQuery(config *tabletenv.TabletConfig, query string) string { hints := make([]string, 0) - if config.Oltp.QueryTimeoutMethod == "mysql" && config.Oltp.QueryTimeoutSeconds > 0 { + switch config.Oltp.QueryTimeoutMethod.String() { + case tabletenv.QueryTimeoutMethodMysql: // The MAX_EXECUTION_TIME(N) hint sets a statement execution timeout of N milliseconds. // https://dev.mysql.com/doc/refman/8.0/en/optimizer-hints.html#optimizer-hints-execution-time hints = append(hints, diff --git a/go/vt/vttablet/tabletserver/query_executor_test.go b/go/vt/vttablet/tabletserver/query_executor_test.go index 393fb9dfdaf..e523cf59457 100644 --- a/go/vt/vttablet/tabletserver/query_executor_test.go +++ b/go/vt/vttablet/tabletserver/query_executor_test.go @@ -28,6 +28,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "vitess.io/vitess/go/flagutil" "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/mysql/fakesqldb" "vitess.io/vitess/go/sqltypes" @@ -1794,16 +1795,21 @@ func TestQueryExecSchemaReloadCount(t *testing.T) { } } -func TestAddMySQLOptimizerHints(t *testing.T) { +func TestAddMysqlOptimizerHintsToQuery(t *testing.T) { config := tabletenv.NewDefaultConfig() { - config.Oltp.QueryTimeoutMethod = "vttablet" - t.Logf("sql: %v", addMySQLOptimizerHints(config, "select * from something")) + assert.Equal(t, + `select * from something`, + addMysqlOptimizerHintsToQuery(config, "select * from something"), + ) } { - config.Oltp.QueryTimeoutMethod = "mysql" - config.Oltp.QueryTimeoutSeconds = tabletenv.Seconds(1) - t.Logf("sql: %v", addMySQLOptimizerHints(config, "select * from something")) + config.Oltp.QueryTimeoutMethod.Set(tabletenv.QueryTimeoutMethodMysql) + config.Oltp.QueryTimeoutSeconds = flagutil.NewDeprecatedFloat64Seconds(t.Name(), time.Second) + assert.Equal(t, + `select /*+ MAX_EXECUTION_TIME(1000) */ * from something`, + addMysqlOptimizerHintsToQuery(config, "select * from something"), + ) } } diff --git a/go/vt/vttablet/tabletserver/tabletenv/config.go b/go/vt/vttablet/tabletserver/tabletenv/config.go index 016d84d62f5..8069b7db0ee 100644 --- a/go/vt/vttablet/tabletserver/tabletenv/config.go +++ b/go/vt/vttablet/tabletserver/tabletenv/config.go @@ -44,13 +44,14 @@ import ( // These constants represent values for various config parameters. const ( - Enable = "enable" - Disable = "disable" - Dryrun = "dryRun" - NotOnPrimary = "notOnPrimary" - Polling = "polling" - Heartbeat = "heartbeat" - DefaultQueryTimeoutMethod = "vttablet" + Enable = "enable" + Disable = "disable" + Dryrun = "dryRun" + NotOnPrimary = "notOnPrimary" + Polling = "polling" + Heartbeat = "heartbeat" + QueryTimeoutMethodVttablet = "vttablet" + QueryTimeoutMethodMysql = "mysql" ) var ( @@ -237,7 +238,7 @@ func registerTabletEnvFlags(fs *pflag.FlagSet) { fs.BoolVar(¤tConfig.EnableViews, "queryserver-enable-views", false, "Enable views support in vttablet.") fs.BoolVar(¤tConfig.EnablePerWorkloadTableMetrics, "enable-per-workload-table-metrics", defaultConfig.EnablePerWorkloadTableMetrics, "If true, query counts and query error metrics include a label that identifies the workload") - fs.StringVar(¤tConfig.Oltp.QueryTimeoutMethod, "query-timeout-method", defaultConfig.Oltp.QueryTimeoutMethod, "The method to be used to kill MySQL queries, options: 'vttablet' and 'mysql'. 'vttablet' issues a MySQL KILL operation whereas 'mysql' pushes the kill to MySQL.") + fs.Var(currentConfig.Oltp.QueryTimeoutMethod, "query-timeout-method", "The method to be used to kill MySQL queries, options: 'vttablet' and 'mysql'. 'vttablet' issues a MySQL KILL operation whereas 'mysql' pushes the kill to MySQL.") } var ( @@ -474,7 +475,7 @@ func (cfg *OlapConfig) MarshalJSON() ([]byte, error) { // OltpConfig contains the config for oltp settings. type OltpConfig struct { QueryTimeoutSeconds flagutil.DeprecatedFloat64Seconds `json:"queryTimeoutSeconds,omitempty"` - QueryTimeoutMethod string `json:"queryTimeoutMethod,omitempty"` + QueryTimeoutMethod *flagutil.StringEnum `json:"queryTimeoutMethod,omitempty"` TxTimeoutSeconds flagutil.DeprecatedFloat64Seconds `json:"txTimeoutSeconds,omitempty"` MaxRows int `json:"maxRows,omitempty"` WarnRows int `json:"warnRows,omitempty"` @@ -783,7 +784,7 @@ var defaultConfig = TabletConfig{ }, Oltp: OltpConfig{ QueryTimeoutSeconds: flagutil.NewDeprecatedFloat64Seconds("queryserver-config-query-timeout", 30*time.Second), - QueryTimeoutMethod: DefaultQueryTimeoutMethod, + QueryTimeoutMethod: flagutil.NewStringEnum("query-timeout-method", QueryTimeoutMethodVttablet, []string{QueryTimeoutMethodVttablet, QueryTimeoutMethodMysql}), TxTimeoutSeconds: flagutil.NewDeprecatedFloat64Seconds("queryserver-config-transaction-timeout", 30*time.Second), MaxRows: 10000, }, diff --git a/go/vt/vttablet/tabletserver/tabletserver.go b/go/vt/vttablet/tabletserver/tabletserver.go index 1ed7aa5658b..0ad6e681342 100644 --- a/go/vt/vttablet/tabletserver/tabletserver.go +++ b/go/vt/vttablet/tabletserver/tabletserver.go @@ -163,7 +163,8 @@ func NewTabletServer(ctx context.Context, name string, config *tabletenv.TabletC } queryTimeoutNanos := config.Oltp.QueryTimeoutSeconds.Get().Nanoseconds() - if config.Oltp.QueryTimeoutMethod == "mysql" { + switch config.Oltp.QueryTimeoutMethod.String() { + case tabletenv.QueryTimeoutMethodMysql: queryTimeoutNanos = queryTimeoutNanos + queryTimeoutMysqlMaxWait.Nanoseconds() } tsv.QueryTimeout.Store(queryTimeoutNanos) From 9c362500ffcf4fb42226d65a2c9319d4d13dffff Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 4 Nov 2023 03:13:19 +0100 Subject: [PATCH 04/12] Remove unused const Signed-off-by: Tim Vaillancourt --- go/vt/vttablet/tabletserver/query_executor.go | 1 - 1 file changed, 1 deletion(-) diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index d505021ae71..f66fa46325c 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -66,7 +66,6 @@ type QueryExecutor struct { const ( streamRowsSize = 256 - queryTimeoutMethodMySQL = "mysql" queryTimeoutMysqlMaxWait = time.Second ) From ea0e7ce33598dd182968493dbd9613adc3ba8640 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 4 Nov 2023 03:17:15 +0100 Subject: [PATCH 05/12] Update flag help Signed-off-by: Tim Vaillancourt --- go/vt/vttablet/tabletserver/tabletenv/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/vttablet/tabletserver/tabletenv/config.go b/go/vt/vttablet/tabletserver/tabletenv/config.go index 8069b7db0ee..60995e2446d 100644 --- a/go/vt/vttablet/tabletserver/tabletenv/config.go +++ b/go/vt/vttablet/tabletserver/tabletenv/config.go @@ -238,7 +238,7 @@ func registerTabletEnvFlags(fs *pflag.FlagSet) { fs.BoolVar(¤tConfig.EnableViews, "queryserver-enable-views", false, "Enable views support in vttablet.") fs.BoolVar(¤tConfig.EnablePerWorkloadTableMetrics, "enable-per-workload-table-metrics", defaultConfig.EnablePerWorkloadTableMetrics, "If true, query counts and query error metrics include a label that identifies the workload") - fs.Var(currentConfig.Oltp.QueryTimeoutMethod, "query-timeout-method", "The method to be used to kill MySQL queries, options: 'vttablet' and 'mysql'. 'vttablet' issues a MySQL KILL operation whereas 'mysql' pushes the kill to MySQL.") + fs.Var(currentConfig.Oltp.QueryTimeoutMethod, "query-timeout-method", "The preferred method to timeout/kill MySQL queries, options: 'vttablet' and 'mysql'. 'vttablet' issues a MySQL KILL operation, 'mysql' pushes the kill to MySQL with a fallback to a KILL.") } var ( From 3d554ee65510f36cc54a6225fd9b403fd7fa2d9b Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 4 Nov 2023 03:37:32 +0100 Subject: [PATCH 06/12] Add hints to SELECT only Signed-off-by: Tim Vaillancourt --- go/vt/vttablet/tabletserver/query_executor.go | 10 ++++++++-- go/vt/vttablet/tabletserver/query_executor_test.go | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index e118441f8ca..0a403163b7e 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -822,7 +822,7 @@ func (qre *QueryExecutor) generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, b if err != nil { return "", "", vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "%s", err) } - query = addMysqlOptimizerHintsToQuery(qre.tsv.config, query) + query = addMysqlOptimizerHintsToQuery(qre.tsv.config, qre.plan.PlanID, query) if qre.tsv.config.AnnotateQueries { username := callerid.GetPrincipal(callerid.EffectiveCallerIDFromContext(qre.ctx)) if username == "" { @@ -852,7 +852,11 @@ func (qre *QueryExecutor) generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, b return buf.String(), query, nil } -func addMysqlOptimizerHintsToQuery(config *tabletenv.TabletConfig, query string) string { +func addMysqlOptimizerHintsToQuery(config *tabletenv.TabletConfig, planType p.PlanType, query string) string { + if planType != p.PlanSelect { + return query + } + hints := make([]string, 0) switch config.Oltp.QueryTimeoutMethod.String() { @@ -865,6 +869,8 @@ func addMysqlOptimizerHintsToQuery(config *tabletenv.TabletConfig, query string) } if len(hints) > 0 { + // MySQL optimizer hints must come immediately after the 1st + // field/verb, which should always be "select" or "SELECT". fields := strings.SplitN(query, " ", 2) return strings.Join([]string{ fields[0], diff --git a/go/vt/vttablet/tabletserver/query_executor_test.go b/go/vt/vttablet/tabletserver/query_executor_test.go index fb8d3f7b51a..45621b09989 100644 --- a/go/vt/vttablet/tabletserver/query_executor_test.go +++ b/go/vt/vttablet/tabletserver/query_executor_test.go @@ -1800,7 +1800,7 @@ func TestAddMysqlOptimizerHintsToQuery(t *testing.T) { { assert.Equal(t, `select * from something`, - addMysqlOptimizerHintsToQuery(config, "select * from something"), + addMysqlOptimizerHintsToQuery(config, planbuilder.PlanSelect, "select * from something"), ) } { @@ -1808,7 +1808,13 @@ func TestAddMysqlOptimizerHintsToQuery(t *testing.T) { config.Oltp.QueryTimeoutSeconds = flagutil.NewDeprecatedFloat64Seconds(t.Name(), time.Second) assert.Equal(t, `select /*+ MAX_EXECUTION_TIME(1000) */ * from something`, - addMysqlOptimizerHintsToQuery(config, "select * from something"), + addMysqlOptimizerHintsToQuery(config, planbuilder.PlanSelect, "select * from something"), + ) + } + { + assert.Equal(t, + `insert into something (id, value) values(1, 2)`, + addMysqlOptimizerHintsToQuery(config, planbuilder.PlanInsert, "insert into something (id, value) values(1, 2)"), ) } } From b83858ea018553fc9ec16b3b1b17429815c67c77 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Thu, 25 Jan 2024 00:16:47 +0100 Subject: [PATCH 07/12] WIP Signed-off-by: Tim Vaillancourt --- go/vt/sqlparser/comments.go | 19 +++++ go/vt/vttablet/tabletserver/query_executor.go | 73 +++++++++++-------- .../tabletserver/query_executor_test.go | 39 ++++------ .../vttablet/tabletserver/tabletenv/config.go | 29 ++++---- go/vt/vttablet/tabletserver/tabletserver.go | 20 +++-- 5 files changed, 104 insertions(+), 76 deletions(-) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index 84b73f8e81c..c8126e454d6 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -17,6 +17,7 @@ limitations under the License. package sqlparser import ( + "fmt" "strconv" "strings" "unicode" @@ -418,3 +419,21 @@ func GetWorkloadNameFromStatement(statement Statement) string { return workloadName } + +func AddMysqlOptimizerHintsComment(query string, hints map[string]any) string { + hintsSlice := make([]string, 0, len(hints)) + for hint, val := range hints { + hintsSlice = append(hintsSlice, fmt.Sprintf("%s(%v)", hint, val)) + } + if len(hintsSlice) > 0 { + // MySQL optimizer hints must come immediately after the 1st + // field/verb, which should always be "select" or "SELECT". + fields := strings.SplitN(query, " ", 2) + return strings.Join([]string{ + fields[0], + "/*+ " + strings.Join(hintsSlice, " ") + " */", + fields[1], + }, " ") + } + return query +} diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index 0a403163b7e..d13e4b516a3 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "io" + "strconv" "strings" "sync" "time" @@ -39,6 +40,7 @@ import ( "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vtgate/evalengine" "vitess.io/vitess/go/vt/vttablet/tabletserver/connpool" + "vitess.io/vitess/go/vt/vttablet/tabletserver/planbuilder" p "vitess.io/vitess/go/vt/vttablet/tabletserver/planbuilder" "vitess.io/vitess/go/vt/vttablet/tabletserver/rules" eschema "vitess.io/vitess/go/vt/vttablet/tabletserver/schema" @@ -65,8 +67,7 @@ type QueryExecutor struct { } const ( - streamRowsSize = 256 - queryTimeoutMysqlMaxWait = time.Second + streamRowsSize = 256 ) var ( @@ -99,6 +100,15 @@ func allocStreamResult() *sqltypes.Result { return streamResultPool.Get().(*sqltypes.Result) } +func (qre *QueryExecutor) isSelect() bool { + switch qre.plan.PlanID { + case planbuilder.PlanSelect, planbuilder.PlanSelectImpossible: + return true + default: + return false + } +} + func (qre *QueryExecutor) shouldConsolidate() bool { co := qre.options.GetConsolidator() switch co { @@ -822,7 +832,6 @@ func (qre *QueryExecutor) generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, b if err != nil { return "", "", vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "%s", err) } - query = addMysqlOptimizerHintsToQuery(qre.tsv.config, qre.plan.PlanID, query) if qre.tsv.config.AnnotateQueries { username := callerid.GetPrincipal(callerid.EffectiveCallerIDFromContext(qre.ctx)) if username == "" { @@ -844,42 +853,46 @@ func (qre *QueryExecutor) generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, b return query, query, nil } + var mysqlOptimizerHints string + if qre.isSelect() { + mysqlOptimizerHints = buildMysqlOptimizerHints(qre.tsv) + } + var buf strings.Builder - buf.Grow(len(qre.marginComments.Leading) + len(query) + len(qre.marginComments.Trailing)) - buf.WriteString(qre.marginComments.Leading) - buf.WriteString(query) - buf.WriteString(qre.marginComments.Trailing) - return buf.String(), query, nil -} + if mysqlOptimizerHints != "" { + fields := strings.SplitN(query, " ", 2) + queryPrefix := fields[0] + " " + queryNonPrefix := " " + fields[1] -func addMysqlOptimizerHintsToQuery(config *tabletenv.TabletConfig, planType p.PlanType, query string) string { - if planType != p.PlanSelect { - return query + buf.Grow(len(qre.marginComments.Leading) + len(queryPrefix) + len(mysqlOptimizerHints) + len(queryNonPrefix) + len(qre.marginComments.Trailing)) + buf.WriteString(qre.marginComments.Leading) + buf.WriteString(queryPrefix) + buf.WriteString(mysqlOptimizerHints) + buf.WriteString(queryNonPrefix) + buf.WriteString(qre.marginComments.Trailing) + } else { + buf.Grow(len(qre.marginComments.Leading) + len(query) + len(qre.marginComments.Trailing)) + buf.WriteString(qre.marginComments.Leading) + buf.WriteString(query) + buf.WriteString(qre.marginComments.Trailing) } + return buf.String(), query, nil +} - hints := make([]string, 0) - - switch config.Oltp.QueryTimeoutMethod.String() { - case tabletenv.QueryTimeoutMethodMysql: +func buildMysqlOptimizerHints(tsv *TabletServer) string { + var buf strings.Builder + if tsv.config.Oltp.QueryTimeoutPushdown { // The MAX_EXECUTION_TIME(N) hint sets a statement execution timeout of N milliseconds. // https://dev.mysql.com/doc/refman/8.0/en/optimizer-hints.html#optimizer-hints-execution-time - hints = append(hints, - fmt.Sprintf("MAX_EXECUTION_TIME(%d)", config.Oltp.QueryTimeoutSeconds.Get().Milliseconds()), - ) + queryTimeoutStr := strconv.FormatInt(tsv.loadQueryTimeout(), 64) + buf.Grow(len(queryTimeoutStr)) + buf.WriteString(queryTimeoutStr) } - if len(hints) > 0 { - // MySQL optimizer hints must come immediately after the 1st - // field/verb, which should always be "select" or "SELECT". - fields := strings.SplitN(query, " ", 2) - return strings.Join([]string{ - fields[0], - "/*+ " + strings.Join(hints, " ") + " */", - fields[1], - }, " ") + if len(optimizerHints) == 0 { + return "" } - - return query + return "/*+ " + strings.Join(optimizerHints, " ") + " */" } func rewriteOUTParamError(err error) error { diff --git a/go/vt/vttablet/tabletserver/query_executor_test.go b/go/vt/vttablet/tabletserver/query_executor_test.go index 45621b09989..6a08f394c2b 100644 --- a/go/vt/vttablet/tabletserver/query_executor_test.go +++ b/go/vt/vttablet/tabletserver/query_executor_test.go @@ -28,7 +28,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "vitess.io/vitess/go/flagutil" "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/mysql/fakesqldb" "vitess.io/vitess/go/sqltypes" @@ -37,6 +36,7 @@ import ( "vitess.io/vitess/go/vt/callinfo" "vitess.io/vitess/go/vt/callinfo/fakecallinfo" "vitess.io/vitess/go/vt/sidecardb" + "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/tableacl" "vitess.io/vitess/go/vt/tableacl/simpleacl" "vitess.io/vitess/go/vt/topo/memorytopo" @@ -1795,28 +1795,21 @@ func TestQueryExecSchemaReloadCount(t *testing.T) { } } -func TestAddMysqlOptimizerHintsToQuery(t *testing.T) { - config := tabletenv.NewDefaultConfig() - { - assert.Equal(t, - `select * from something`, - addMysqlOptimizerHintsToQuery(config, planbuilder.PlanSelect, "select * from something"), - ) - } - { - config.Oltp.QueryTimeoutMethod.Set(tabletenv.QueryTimeoutMethodMysql) - config.Oltp.QueryTimeoutSeconds = flagutil.NewDeprecatedFloat64Seconds(t.Name(), time.Second) - assert.Equal(t, - `select /*+ MAX_EXECUTION_TIME(1000) */ * from something`, - addMysqlOptimizerHintsToQuery(config, planbuilder.PlanSelect, "select * from something"), - ) - } - { - assert.Equal(t, - `insert into something (id, value) values(1, 2)`, - addMysqlOptimizerHintsToQuery(config, planbuilder.PlanInsert, "insert into something (id, value) values(1, 2)"), - ) - } +func TestGenerateFinalSQL(t *testing.T) { + // generateFinalSQL(parsedQuery *sqlparser.ParsedQuery, bindVars map[string]*querypb.BindVariable) + db := setUpQueryExecutorTest(t) + defer db.Close() + tsv := newTestTabletServer(context.Background(), noFlags, db) + defer tsv.StopService() + + qre := newTestQueryExecutor(context.Background(), tsv, `select * from something`, 0) + query, noComments, err := qre.generateFinalSQL( + &sqlparser.ParsedQuery{Query: `select * from something`}, + map[string]*querypb.BindVariable{}, + ) + assert.Nil(t, err) + assert.Equal(t, `select * from something`, query) + t.Logf("noComments: %s", noComments) } type mockTxThrottler struct { diff --git a/go/vt/vttablet/tabletserver/tabletenv/config.go b/go/vt/vttablet/tabletserver/tabletenv/config.go index d7faadee20f..a738855fe89 100644 --- a/go/vt/vttablet/tabletserver/tabletenv/config.go +++ b/go/vt/vttablet/tabletserver/tabletenv/config.go @@ -44,14 +44,12 @@ import ( // These constants represent values for various config parameters. const ( - Enable = "enable" - Disable = "disable" - Dryrun = "dryRun" - NotOnPrimary = "notOnPrimary" - Polling = "polling" - Heartbeat = "heartbeat" - QueryTimeoutMethodVttablet = "vttablet" - QueryTimeoutMethodMysql = "mysql" + Enable = "enable" + Disable = "disable" + Dryrun = "dryRun" + NotOnPrimary = "notOnPrimary" + Polling = "polling" + Heartbeat = "heartbeat" ) var ( @@ -229,7 +227,8 @@ func registerTabletEnvFlags(fs *pflag.FlagSet) { fs.BoolVar(¤tConfig.EnableViews, "queryserver-enable-views", false, "Enable views support in vttablet.") fs.BoolVar(¤tConfig.EnablePerWorkloadTableMetrics, "enable-per-workload-table-metrics", defaultConfig.EnablePerWorkloadTableMetrics, "If true, query counts and query error metrics include a label that identifies the workload") - fs.Var(currentConfig.Oltp.QueryTimeoutMethod, "query-timeout-method", "The preferred method to timeout/kill MySQL queries, options: 'vttablet' and 'mysql'. 'vttablet' issues a MySQL KILL operation, 'mysql' pushes the kill to MySQL with a fallback to a KILL.") + fs.BoolVar(¤tConfig.Oltp.QueryTimeoutPushdown, "query-timeout-pushdown", false, "Attempt to push-down timing-out of queries to MySQL with a fallback to a MySQL KILL operation.") + fs.DurationVar(¤tConfig.Oltp.QueryTimeoutPushdownWait, "query-timeout-pushdown-wait", time.Second, "Max time to wait for MySQL to kill a query before sending a fallback KILL operation. Requires --query-timeout-pushdown") } var ( @@ -465,11 +464,12 @@ func (cfg *OlapConfig) MarshalJSON() ([]byte, error) { // OltpConfig contains the config for oltp settings. type OltpConfig struct { - QueryTimeoutSeconds flagutil.DeprecatedFloat64Seconds `json:"queryTimeoutSeconds,omitempty"` - QueryTimeoutMethod *flagutil.StringEnum `json:"queryTimeoutMethod,omitempty"` - TxTimeoutSeconds flagutil.DeprecatedFloat64Seconds `json:"txTimeoutSeconds,omitempty"` - MaxRows int `json:"maxRows,omitempty"` - WarnRows int `json:"warnRows,omitempty"` + QueryTimeoutSeconds flagutil.DeprecatedFloat64Seconds `json:"queryTimeoutSeconds,omitempty"` + QueryTimeoutPushdown bool `json:"queryTimeoutPushdown,omitempty"` + QueryTimeoutPushdownWait time.Duration `json:"queryTimeoutPushdownWait,omitempty"` + TxTimeoutSeconds flagutil.DeprecatedFloat64Seconds `json:"txTimeoutSeconds,omitempty"` + MaxRows int `json:"maxRows,omitempty"` + WarnRows int `json:"warnRows,omitempty"` } func (cfg *OltpConfig) MarshalJSON() ([]byte, error) { @@ -775,7 +775,6 @@ var defaultConfig = TabletConfig{ }, Oltp: OltpConfig{ QueryTimeoutSeconds: flagutil.NewDeprecatedFloat64Seconds("queryserver-config-query-timeout", 30*time.Second), - QueryTimeoutMethod: flagutil.NewStringEnum("query-timeout-method", QueryTimeoutMethodVttablet, []string{QueryTimeoutMethodVttablet, QueryTimeoutMethodMysql}), TxTimeoutSeconds: flagutil.NewDeprecatedFloat64Seconds("queryserver-config-transaction-timeout", 30*time.Second), MaxRows: 10000, }, diff --git a/go/vt/vttablet/tabletserver/tabletserver.go b/go/vt/vttablet/tabletserver/tabletserver.go index 4ed347eddb8..3b344d10459 100644 --- a/go/vt/vttablet/tabletserver/tabletserver.go +++ b/go/vt/vttablet/tabletserver/tabletserver.go @@ -161,13 +161,7 @@ func NewTabletServer(ctx context.Context, name string, config *tabletenv.TabletC topoServer: topoServer, alias: alias.CloneVT(), } - - queryTimeoutNanos := config.Oltp.QueryTimeoutSeconds.Get().Nanoseconds() - switch config.Oltp.QueryTimeoutMethod.String() { - case tabletenv.QueryTimeoutMethodMysql: - queryTimeoutNanos = queryTimeoutNanos + queryTimeoutMysqlMaxWait.Nanoseconds() - } - tsv.QueryTimeout.Store(queryTimeoutNanos) + tsv.QueryTimeout.Store(config.Oltp.QueryTimeoutSeconds.Get().Nanoseconds()) tsOnce.Do(func() { srvTopoServer = srvtopo.NewResilientServer(ctx, topoServer, "TabletSrvTopo") }) @@ -242,6 +236,13 @@ func (tsv *TabletServer) loadQueryTimeout() time.Duration { return time.Duration(tsv.QueryTimeout.Load()) } +func (tsv *TabletServer) loadQueryTimeoutWithPushdownWait() time.Duration { + if tsv.config.Oltp.QueryTimeoutPushdown { + return tsv.loadQueryTimeout() + tsv.config.Oltp.QueryTimeoutPushdownWait + } + return tsv.loadQueryTimeout() +} + // onlineDDLExecutorToggleTableBuffer is called by onlineDDLExecutor as a callback function. onlineDDLExecutor // uses it to start/stop query buffering for a given table. // It is onlineDDLExecutor's responsibility to make sure beffering is stopped after some definite amount of time. @@ -494,7 +495,7 @@ func (tsv *TabletServer) Begin(ctx context.Context, target *querypb.Target, opti func (tsv *TabletServer) begin(ctx context.Context, target *querypb.Target, savepointQueries []string, reservedID int64, settings []string, options *querypb.ExecuteOptions) (state queryservice.TransactionState, err error) { state.TabletAlias = tsv.alias err = tsv.execRequest( - ctx, tsv.loadQueryTimeout(), + ctx, tsv.loadQueryTimeoutWithPushdownWait(), "Begin", "begin", nil, target, options, false, /* allowOnShutdown */ func(ctx context.Context, logStats *tabletenv.LogStats) error { @@ -766,6 +767,9 @@ func (tsv *TabletServer) Execute(ctx context.Context, target *querypb.Target, sq func (tsv *TabletServer) execute(ctx context.Context, target *querypb.Target, sql string, bindVariables map[string]*querypb.BindVariable, transactionID int64, reservedID int64, settings []string, options *querypb.ExecuteOptions) (result *sqltypes.Result, err error) { allowOnShutdown := false timeout := tsv.loadQueryTimeout() + if tsv.config.Oltp.QueryTimeoutPushdown { + return timeout + tsv.config.Oltp.QueryTimeoutPushdownWait + } if transactionID != 0 { allowOnShutdown = true // Execute calls happen for OLTP only, so we can directly fetch the From b30fcab597df33aa270f948d62486ef3e0b756ae Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 9 Feb 2024 00:43:46 +0100 Subject: [PATCH 08/12] WIP Signed-off-by: Tim Vaillancourt --- go/vt/sqlparser/comments.go | 44 +++++++++---------- go/vt/sqlparser/comments_test.go | 32 ++++++++++++++ go/vt/vttablet/tabletserver/query_executor.go | 2 +- .../vttablet/tabletserver/tabletenv/config.go | 12 ++--- 4 files changed, 60 insertions(+), 30 deletions(-) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index 6771dd72e5c..665ada50d49 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -20,6 +20,7 @@ import ( "fmt" "strconv" "strings" + "time" "unicode" vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" @@ -63,6 +64,9 @@ const ( // between zero and MaxPriorityValue. MaxPriorityValue = 100 + // OptimizerHintMaxExecutionTime is the optimizer hint used in MySQL to set the max execution time for a query. + OptimizerHintMaxExecutionTime = "MAX_EXECUTION_TIME" + // OptimizerHintSetVar is the optimizer hint used in MySQL to set the value of a specific session variable for a query. OptimizerHintSetVar = "SET_VAR" ) @@ -318,11 +322,22 @@ func (c *ParsedComments) GetMySQLSetVarValue(key string) string { return "" } +// SetMySQLMaxExecutionTime sets a query level maximum execution time using a /*+ MAX_EXECUTION_TIME() */ MySQL optimizer hint. +func (c *ParsedComments) SetMySQLMaxExecutionTime(maxExecutionTime time.Duration) (newComments Comments) { + return c.SetMySQLOptimizerHint(OptimizerHintMaxExecutionTime, strconv.FormatInt(maxExecutionTime.Milliseconds(), 10)) +} + // SetMySQLSetVarValue updates or sets the value of the given variable as part of a /*+ SET_VAR() */ MySQL optimizer hint. func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComments Comments) { + value = fmt.Sprintf("%v=%v", key, value) + return c.SetMySQLOptimizerHint(OptimizerHintSetVar, value) +} + +// SetMySQLOptimizerHint updates or sets the value of a MySQL optimizer hint. +func (c *ParsedComments) SetMySQLOptimizerHint(hint, value string) (newComments Comments) { if c == nil { // If we have no parsed comments, then we create a new one with the required optimizer hint and return it. - newComments = append(newComments, fmt.Sprintf("/*+ %v(%v=%v) */", OptimizerHintSetVar, key, value)) + newComments = append(newComments, fmt.Sprintf("/*+ %v(%v) */", hint, value)) return } seenFirstOhComment := false @@ -354,6 +369,7 @@ func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComme ohContent := commentStr[ohContentStart:ohContentEnd] // Check if the optimizer hint name matches `SET_VAR`. if strings.EqualFold(strings.TrimSpace(ohName), OptimizerHintSetVar) { + key, setValue, _ := strings.Cut(value, "=") // If it does, then we cut the string at the first occurrence of "=". // That gives us the name of the variable, and the value that it is being set to. // If the variable matches what we are looking for, we can change its value. @@ -365,17 +381,17 @@ func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComme } if strings.EqualFold(strings.TrimSpace(setVarName), key) { keyPresent = true - finalComment += fmt.Sprintf(" %v(%v=%v)", ohName, strings.TrimSpace(setVarName), value) + finalComment += fmt.Sprintf(" %v(%v=%v)", ohName, strings.TrimSpace(setVarName), setValue) } } else { // If it doesn't match, we add it to our final comment and move on. finalComment += fmt.Sprintf(" %v(%v)", ohName, ohContent) } } - // If we haven't found any SET_VAR optimizer hint with the matching variable, + // If we haven't found any optimizer hint with the matching variable, // then we add a new optimizer hint to introduce this variable. if !keyPresent { - finalComment += fmt.Sprintf(" %v(%v=%v)", OptimizerHintSetVar, key, value) + finalComment += fmt.Sprintf(" %v(%v)", hint, value) } finalComment += " */" @@ -384,7 +400,7 @@ func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComme // If we have not seen even a single comment that has the optimizer hint prefix, // then we add a new optimizer hint to introduce this variable. if !seenFirstOhComment { - newComments = append(newComments, fmt.Sprintf("/*+ %v(%v=%v) */", OptimizerHintSetVar, key, value)) + newComments = append(newComments, fmt.Sprintf("/*+ %v(%v) */", hint, value)) } return newComments } @@ -644,21 +660,3 @@ func GetWorkloadNameFromStatement(statement Statement) string { return workloadName } - -func AddMysqlOptimizerHintsComment(query string, hints map[string]any) string { - hintsSlice := make([]string, 0, len(hints)) - for hint, val := range hints { - hintsSlice = append(hintsSlice, fmt.Sprintf("%s(%v)", hint, val)) - } - if len(hintsSlice) > 0 { - // MySQL optimizer hints must come immediately after the 1st - // field/verb, which should always be "select" or "SELECT". - fields := strings.SplitN(query, " ", 2) - return strings.Join([]string{ - fields[0], - "/*+ " + strings.Join(hintsSlice, " ") + " */", - fields[1], - }, " ") - } - return query -} diff --git a/go/vt/sqlparser/comments_test.go b/go/vt/sqlparser/comments_test.go index dd22fd7000c..5d053649d2e 100644 --- a/go/vt/sqlparser/comments_test.go +++ b/go/vt/sqlparser/comments_test.go @@ -20,6 +20,7 @@ import ( "fmt" "reflect" "testing" + "time" "github.com/stretchr/testify/require" @@ -614,6 +615,37 @@ func TestGetMySQLSetVarValue(t *testing.T) { } } +func TestSetMySQLMaxExecutionTime(t *testing.T) { + tests := []struct { + name string + comments []string + maxExecTime time.Duration + commentsWanted Comments + }{ + { + name: "No comments", + comments: nil, + maxExecTime: time.Second * 30, + commentsWanted: []string{"/*+ MAX_EXECUTION_TIME(30000) */"}, + }, + { + name: "Add to comments", + comments: []string{"/*+ SET_VAR(sort_buffer_size = 16M) */"}, + maxExecTime: time.Second * 30, + commentsWanted: []string{"/*+ SET_VAR(sort_buffer_size = 16M) MAX_EXECUTION_TIME(30000) */"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &ParsedComments{ + comments: tt.comments, + } + newComments := c.SetMySQLMaxExecutionTime(tt.maxExecTime) + require.EqualValues(t, tt.commentsWanted, newComments) + }) + } +} + func TestSetMySQLSetVarValue(t *testing.T) { tests := []struct { name string diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index 9b7c93eab49..3a79a60731e 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -25,7 +25,7 @@ import ( "sync" "time" - "vitess.io/vitess/go/mysql/collations" + "vitess.io/vitess/go/mysql/collations" "vitess.io/vitess/go/mysql/replication" "vitess.io/vitess/go/mysql/sqlerror" "vitess.io/vitess/go/pools/smartconnpool" diff --git a/go/vt/vttablet/tabletserver/tabletenv/config.go b/go/vt/vttablet/tabletserver/tabletenv/config.go index 0802446ad94..fdc871af440 100644 --- a/go/vt/vttablet/tabletserver/tabletenv/config.go +++ b/go/vt/vttablet/tabletserver/tabletenv/config.go @@ -557,12 +557,12 @@ func (cfg *OlapConfig) UnmarshalJSON(data []byte) (err error) { // OltpConfig contains the config for oltp settings. type OltpConfig struct { - QueryTimeout time.Duration `json:"queryTimeoutSeconds,omitempty"` - TxTimeout time.Duration `json:"txTimeoutSeconds,omitempty"` - QueryTimeoutPushdown bool `json:"queryTimeoutPushdown,omitempty"` - QueryTimeoutPushdownWait time.Duration `json:"queryTimeoutPushdownWait,omitempty"` - MaxRows int `json:"maxRows,omitempty"` - WarnRows int `json:"warnRows,omitempty"` + QueryTimeout time.Duration `json:"queryTimeoutSeconds,omitempty"` + TxTimeout time.Duration `json:"txTimeoutSeconds,omitempty"` + QueryTimeoutPushdown bool `json:"queryTimeoutPushdown,omitempty"` + QueryTimeoutPushdownWait time.Duration `json:"queryTimeoutPushdownWait,omitempty"` + MaxRows int `json:"maxRows,omitempty"` + WarnRows int `json:"warnRows,omitempty"` } func (cfg *OltpConfig) MarshalJSON() ([]byte, error) { From 8d66347f31f1a0d219474a39eb8bbe9dc5f59f93 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 9 Feb 2024 00:56:27 +0100 Subject: [PATCH 09/12] cleanup Signed-off-by: Tim Vaillancourt --- go/vt/sqlparser/comments.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index 665ada50d49..09a92c67ff9 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -324,20 +324,23 @@ func (c *ParsedComments) GetMySQLSetVarValue(key string) string { // SetMySQLMaxExecutionTime sets a query level maximum execution time using a /*+ MAX_EXECUTION_TIME() */ MySQL optimizer hint. func (c *ParsedComments) SetMySQLMaxExecutionTime(maxExecutionTime time.Duration) (newComments Comments) { - return c.SetMySQLOptimizerHint(OptimizerHintMaxExecutionTime, strconv.FormatInt(maxExecutionTime.Milliseconds(), 10)) + return c.SetMySQLOptimizerHint(OptimizerHintMaxExecutionTime, "", strconv.FormatInt(maxExecutionTime.Milliseconds(), 10)) } // SetMySQLSetVarValue updates or sets the value of the given variable as part of a /*+ SET_VAR() */ MySQL optimizer hint. func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComments Comments) { - value = fmt.Sprintf("%v=%v", key, value) - return c.SetMySQLOptimizerHint(OptimizerHintSetVar, value) + return c.SetMySQLOptimizerHint(OptimizerHintSetVar, key, value) } // SetMySQLOptimizerHint updates or sets the value of a MySQL optimizer hint. -func (c *ParsedComments) SetMySQLOptimizerHint(hint, value string) (newComments Comments) { +func (c *ParsedComments) SetMySQLOptimizerHint(hint, key, value string) (newComments Comments) { + keyAndValue := value + if key != "" { + keyAndValue = fmt.Sprintf("%v=%v", key, value) + } if c == nil { // If we have no parsed comments, then we create a new one with the required optimizer hint and return it. - newComments = append(newComments, fmt.Sprintf("/*+ %v(%v) */", hint, value)) + newComments = append(newComments, fmt.Sprintf("/*+ %v(%v) */", hint, keyAndValue)) return } seenFirstOhComment := false @@ -369,7 +372,6 @@ func (c *ParsedComments) SetMySQLOptimizerHint(hint, value string) (newComments ohContent := commentStr[ohContentStart:ohContentEnd] // Check if the optimizer hint name matches `SET_VAR`. if strings.EqualFold(strings.TrimSpace(ohName), OptimizerHintSetVar) { - key, setValue, _ := strings.Cut(value, "=") // If it does, then we cut the string at the first occurrence of "=". // That gives us the name of the variable, and the value that it is being set to. // If the variable matches what we are looking for, we can change its value. @@ -381,7 +383,7 @@ func (c *ParsedComments) SetMySQLOptimizerHint(hint, value string) (newComments } if strings.EqualFold(strings.TrimSpace(setVarName), key) { keyPresent = true - finalComment += fmt.Sprintf(" %v(%v=%v)", ohName, strings.TrimSpace(setVarName), setValue) + finalComment += fmt.Sprintf(" %v(%v=%v)", ohName, strings.TrimSpace(setVarName), value) } } else { // If it doesn't match, we add it to our final comment and move on. @@ -391,7 +393,7 @@ func (c *ParsedComments) SetMySQLOptimizerHint(hint, value string) (newComments // If we haven't found any optimizer hint with the matching variable, // then we add a new optimizer hint to introduce this variable. if !keyPresent { - finalComment += fmt.Sprintf(" %v(%v)", hint, value) + finalComment += fmt.Sprintf(" %v(%v)", hint, keyAndValue) } finalComment += " */" @@ -400,7 +402,7 @@ func (c *ParsedComments) SetMySQLOptimizerHint(hint, value string) (newComments // If we have not seen even a single comment that has the optimizer hint prefix, // then we add a new optimizer hint to introduce this variable. if !seenFirstOhComment { - newComments = append(newComments, fmt.Sprintf("/*+ %v(%v) */", hint, value)) + newComments = append(newComments, fmt.Sprintf("/*+ %v(%v) */", hint, keyAndValue)) } return newComments } From 0a3290e12fa16199b8dd9f8cd39bc9afd858a74f Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 9 Feb 2024 01:01:19 +0100 Subject: [PATCH 10/12] skip strconv.FormatInt() call Signed-off-by: Tim Vaillancourt --- go/vt/sqlparser/comments.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index 09a92c67ff9..338d0b64463 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -324,7 +324,7 @@ func (c *ParsedComments) GetMySQLSetVarValue(key string) string { // SetMySQLMaxExecutionTime sets a query level maximum execution time using a /*+ MAX_EXECUTION_TIME() */ MySQL optimizer hint. func (c *ParsedComments) SetMySQLMaxExecutionTime(maxExecutionTime time.Duration) (newComments Comments) { - return c.SetMySQLOptimizerHint(OptimizerHintMaxExecutionTime, "", strconv.FormatInt(maxExecutionTime.Milliseconds(), 10)) + return c.SetMySQLOptimizerHint(OptimizerHintMaxExecutionTime, "", maxExecutionTime.Milliseconds()) } // SetMySQLSetVarValue updates or sets the value of the given variable as part of a /*+ SET_VAR() */ MySQL optimizer hint. @@ -333,7 +333,7 @@ func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComme } // SetMySQLOptimizerHint updates or sets the value of a MySQL optimizer hint. -func (c *ParsedComments) SetMySQLOptimizerHint(hint, key, value string) (newComments Comments) { +func (c *ParsedComments) SetMySQLOptimizerHint(hint, key string, value interface{}) (newComments Comments) { keyAndValue := value if key != "" { keyAndValue = fmt.Sprintf("%v=%v", key, value) From 58146a214ecfe2b51d5fc30957dea88cfb61205d Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 9 Feb 2024 01:04:02 +0100 Subject: [PATCH 11/12] add docs to comment Signed-off-by: Tim Vaillancourt --- go/vt/sqlparser/comments.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index 338d0b64463..0e906c87c1b 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -65,9 +65,11 @@ const ( MaxPriorityValue = 100 // OptimizerHintMaxExecutionTime is the optimizer hint used in MySQL to set the max execution time for a query. + // https://dev.mysql.com/doc/refman/8.0/en/optimizer-hints.html#optimizer-hints-execution-time OptimizerHintMaxExecutionTime = "MAX_EXECUTION_TIME" // OptimizerHintSetVar is the optimizer hint used in MySQL to set the value of a specific session variable for a query. + // https://dev.mysql.com/doc/refman/8.0/en/optimizer-hints.html#optimizer-hints-set-var OptimizerHintSetVar = "SET_VAR" ) From 9b1df720767e5b622e82ad2ebcbf00946d973f03 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 9 Feb 2024 04:56:31 +0100 Subject: [PATCH 12/12] generic setMySQLOptimizerHint Signed-off-by: Tim Vaillancourt --- go/vt/sqlparser/comments.go | 16 ++++++++-------- go/vt/sqlparser/comments_test.go | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index 0e906c87c1b..6075791baaf 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -324,29 +324,29 @@ func (c *ParsedComments) GetMySQLSetVarValue(key string) string { return "" } -// SetMySQLMaxExecutionTime sets a query level maximum execution time using a /*+ MAX_EXECUTION_TIME() */ MySQL optimizer hint. -func (c *ParsedComments) SetMySQLMaxExecutionTime(maxExecutionTime time.Duration) (newComments Comments) { - return c.SetMySQLOptimizerHint(OptimizerHintMaxExecutionTime, "", maxExecutionTime.Milliseconds()) +// SetMySQLMaxExecutionTimeValue sets the maximum execution time for a query using a /*+ MAX_EXECUTION_TIME() */ MySQL optimizer hint. +func (c *ParsedComments) SetMySQLMaxExecutionTimeValue(maxExecutionTime time.Duration) (newComments Comments) { + return setMySQLOptimizerHint(c.comments, OptimizerHintMaxExecutionTime, "" /* no key */, maxExecutionTime.Milliseconds()) } // SetMySQLSetVarValue updates or sets the value of the given variable as part of a /*+ SET_VAR() */ MySQL optimizer hint. func (c *ParsedComments) SetMySQLSetVarValue(key string, value string) (newComments Comments) { - return c.SetMySQLOptimizerHint(OptimizerHintSetVar, key, value) + return setMySQLOptimizerHint(c.comments, OptimizerHintSetVar, key, value) } -// SetMySQLOptimizerHint updates or sets the value of a MySQL optimizer hint. -func (c *ParsedComments) SetMySQLOptimizerHint(hint, key string, value interface{}) (newComments Comments) { +// setMySQLOptimizerHint updates or sets the value of a MySQL optimizer hint. +func setMySQLOptimizerHint(comments Comments, hint, key string, value interface{}) (newComments Comments) { keyAndValue := value if key != "" { keyAndValue = fmt.Sprintf("%v=%v", key, value) } - if c == nil { + if len(comments) == 0 { // If we have no parsed comments, then we create a new one with the required optimizer hint and return it. newComments = append(newComments, fmt.Sprintf("/*+ %v(%v) */", hint, keyAndValue)) return } seenFirstOhComment := false - for _, commentStr := range c.comments { + for _, commentStr := range comments { // Skip all the comments that don't start with the query optimizer prefix. // Also, since MySQL only parses the first comment that has the optimizer hint prefix and ignores the following ones, // we skip over all the comments that come after we have seen the first comment with the optimizer hint. diff --git a/go/vt/sqlparser/comments_test.go b/go/vt/sqlparser/comments_test.go index ac1bd8d3398..518b85d914b 100644 --- a/go/vt/sqlparser/comments_test.go +++ b/go/vt/sqlparser/comments_test.go @@ -601,7 +601,7 @@ func TestGetMySQLSetVarValue(t *testing.T) { } } -func TestSetMySQLMaxExecutionTime(t *testing.T) { +func TestSetMySQLMaxExecutionTimeValue(t *testing.T) { tests := []struct { name string comments []string @@ -617,8 +617,8 @@ func TestSetMySQLMaxExecutionTime(t *testing.T) { { name: "Add to comments", comments: []string{"/*+ SET_VAR(sort_buffer_size = 16M) */"}, - maxExecTime: time.Second * 30, - commentsWanted: []string{"/*+ SET_VAR(sort_buffer_size = 16M) MAX_EXECUTION_TIME(30000) */"}, + maxExecTime: time.Minute, + commentsWanted: []string{"/*+ SET_VAR(sort_buffer_size = 16M) MAX_EXECUTION_TIME(60000) */"}, }, } for _, tt := range tests { @@ -626,7 +626,7 @@ func TestSetMySQLMaxExecutionTime(t *testing.T) { c := &ParsedComments{ comments: tt.comments, } - newComments := c.SetMySQLMaxExecutionTime(tt.maxExecTime) + newComments := c.SetMySQLMaxExecutionTimeValue(tt.maxExecTime) require.EqualValues(t, tt.commentsWanted, newComments) }) }