From f2d5d1cf5438d861127b3a65984cb39843225901 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 12 Aug 2024 21:36:21 +0530 Subject: [PATCH] allow innodb_lock_wait_timeout as system variable (#16574) Signed-off-by: Harshit Gangal --- .../vtgate/reservedconn/sysvar_test.go | 32 +++++++++++++ go/vt/sqlparser/ast_rewriting.go | 4 +- go/vt/sysvars/sysvars.go | 2 +- go/vt/vtgate/executor_select_test.go | 47 +++++++++++++++++++ go/vt/vtgate/executor_set_test.go | 7 +++ 5 files changed, 90 insertions(+), 2 deletions(-) diff --git a/go/test/endtoend/vtgate/reservedconn/sysvar_test.go b/go/test/endtoend/vtgate/reservedconn/sysvar_test.go index 564cc671d5f..e7e0cfb0259 100644 --- a/go/test/endtoend/vtgate/reservedconn/sysvar_test.go +++ b/go/test/endtoend/vtgate/reservedconn/sysvar_test.go @@ -461,3 +461,35 @@ func TestSysVarTxIsolation(t *testing.T) { // second run, to ensuring the setting is applied on the session and not just on next query after settings. utils.AssertContains(t, conn, "select @@transaction_isolation, connection_id()", `SERIALIZABLE`) } + +// TestSysVarInnodbWaitTimeout tests the innodb_lock_wait_timeout system variable +func TestSysVarInnodbWaitTimeout(t *testing.T) { + conn, err := mysql.Connect(context.Background(), &vtParams) + require.NoError(t, err) + defer conn.Close() + + // default from mysql + utils.AssertMatches(t, conn, "select @@innodb_lock_wait_timeout", `[[UINT64(20)]]`) + utils.AssertMatches(t, conn, "select @@global.innodb_lock_wait_timeout", `[[UINT64(20)]]`) + // ensuring it goes to mysql + utils.AssertContains(t, conn, "select @@innodb_lock_wait_timeout", `UINT64(20)`) + utils.AssertContains(t, conn, "select @@global.innodb_lock_wait_timeout", `UINT64(20)`) + + // setting to different value. + utils.Exec(t, conn, "set @@innodb_lock_wait_timeout = 120") + utils.AssertMatches(t, conn, "select @@innodb_lock_wait_timeout", `[[INT64(120)]]`) + // ensuring it goes to mysql + utils.AssertContains(t, conn, "select @@global.innodb_lock_wait_timeout, connection_id()", `UINT64(20)`) + utils.AssertContains(t, conn, "select @@innodb_lock_wait_timeout, connection_id()", `INT64(120)`) + // second run, to ensuring the setting is applied on the session and not just on next query after settings. + utils.AssertContains(t, conn, "select @@innodb_lock_wait_timeout, connection_id()", `INT64(120)`) + + // changing setting to different value. + utils.Exec(t, conn, "set @@innodb_lock_wait_timeout = 240") + utils.AssertMatches(t, conn, "select @@innodb_lock_wait_timeout", `[[INT64(240)]]`) + // ensuring it goes to mysql + utils.AssertContains(t, conn, "select @@global.innodb_lock_wait_timeout, connection_id()", `UINT64(20)`) + utils.AssertContains(t, conn, "select @@innodb_lock_wait_timeout, connection_id()", `INT64(240)`) + // second run, to ensuring the setting is applied on the session and not just on next query after settings. + utils.AssertContains(t, conn, "select @@innodb_lock_wait_timeout, connection_id()", `INT64(240)`) +} diff --git a/go/vt/sqlparser/ast_rewriting.go b/go/vt/sqlparser/ast_rewriting.go index 64de1f9d920..ef46b124875 100644 --- a/go/vt/sqlparser/ast_rewriting.go +++ b/go/vt/sqlparser/ast_rewriting.go @@ -301,10 +301,12 @@ func (er *astRewriter) rewriteVariable(cursor *Cursor, node *Variable) { if v, isSet := cursor.Parent().(*SetExpr); isSet && v.Var == node { return } + // no rewriting for global scope variable. + // this should be returned from the underlying database. switch node.Scope { case VariableScope: er.udvRewrite(cursor, node) - case GlobalScope, SessionScope, NextTxScope: + case SessionScope, NextTxScope: er.sysVarRewrite(cursor, node) } } diff --git a/go/vt/sysvars/sysvars.go b/go/vt/sysvars/sysvars.go index c8037563ca1..297ed956bf8 100644 --- a/go/vt/sysvars/sysvars.go +++ b/go/vt/sysvars/sysvars.go @@ -191,6 +191,7 @@ var ( {Name: ForeignKeyChecks, IsBoolean: true, SupportSetVar: true}, {Name: "group_concat_max_len", SupportSetVar: true}, {Name: "information_schema_stats_expiry"}, + {Name: "innodb_lock_wait_timeout"}, {Name: "max_heap_table_size", SupportSetVar: true}, {Name: "max_seeks_for_key", SupportSetVar: true}, {Name: "max_tmp_tables"}, @@ -246,7 +247,6 @@ var ( {Name: "collation_server"}, {Name: "completion_type"}, {Name: "div_precision_increment", SupportSetVar: true}, - {Name: "innodb_lock_wait_timeout"}, {Name: "interactive_timeout"}, {Name: "lc_time_names"}, {Name: "lock_wait_timeout", SupportSetVar: true}, diff --git a/go/vt/vtgate/executor_select_test.go b/go/vt/vtgate/executor_select_test.go index fa448092550..6e3bcdd3eda 100644 --- a/go/vt/vtgate/executor_select_test.go +++ b/go/vt/vtgate/executor_select_test.go @@ -4348,3 +4348,50 @@ func TestStreamJoinQuery(t *testing.T) { utils.MustMatch(t, wantResult.Rows[idx], result.Rows[idx], "mismatched on: ", strconv.Itoa(idx)) } } + +// TestSysVarGlobalAndSession tests that global and session variables are set correctly. +// It also tests that setting a global variable does not affect the session variable and vice versa. +// Also, test what happens on running select @@global and select @@session for a system variable. +func TestSysVarGlobalAndSession(t *testing.T) { + executor, sbc1, _, _, _ := createExecutorEnv(t) + executor.normalize = true + session := NewAutocommitSession(&vtgatepb.Session{EnableSystemSettings: true, SystemVariables: map[string]string{}}) + + sbc1.SetResults([]*sqltypes.Result{ + sqltypes.MakeTestResult(sqltypes.MakeTestFields("innodb_lock_wait_timeout", "uint64"), "20"), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("innodb_lock_wait_timeout", "uint64"), "20"), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("1", "int64")), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("new", "uint64"), "40"), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("reserve_execute", "uint64")), + sqltypes.MakeTestResult(sqltypes.MakeTestFields("@@global.innodb_lock_wait_timeout", "uint64"), "20"), + }) + qr, err := executor.Execute(context.Background(), nil, "TestSetStmt", session, + "select @@innodb_lock_wait_timeout", nil) + require.NoError(t, err) + require.Equal(t, `[[UINT64(20)]]`, fmt.Sprintf("%v", qr.Rows)) + + qr, err = executor.Execute(context.Background(), nil, "TestSetStmt", session, + "select @@global.innodb_lock_wait_timeout", nil) + require.NoError(t, err) + require.Equal(t, `[[UINT64(20)]]`, fmt.Sprintf("%v", qr.Rows)) + + _, err = executor.Execute(context.Background(), nil, "TestSetStmt", session, + "set @@global.innodb_lock_wait_timeout = 120", nil) + require.NoError(t, err) + require.Empty(t, session.SystemVariables["innodb_lock_wait_timeout"]) + + _, err = executor.Execute(context.Background(), nil, "TestSetStmt", session, + "set @@innodb_lock_wait_timeout = 40", nil) + require.NoError(t, err) + require.EqualValues(t, "40", session.SystemVariables["innodb_lock_wait_timeout"]) + + qr, err = executor.Execute(context.Background(), nil, "TestSetStmt", session, + "select @@innodb_lock_wait_timeout", nil) + require.NoError(t, err) + require.Equal(t, `[[INT64(40)]]`, fmt.Sprintf("%v", qr.Rows)) + + qr, err = executor.Execute(context.Background(), nil, "TestSetStmt", session, + "select @@global.innodb_lock_wait_timeout", nil) + require.NoError(t, err) + require.Equal(t, `[[UINT64(20)]]`, fmt.Sprintf("%v", qr.Rows)) +} diff --git a/go/vt/vtgate/executor_set_test.go b/go/vt/vtgate/executor_set_test.go index 5e66899db44..2792c957edd 100644 --- a/go/vt/vtgate/executor_set_test.go +++ b/go/vt/vtgate/executor_set_test.go @@ -364,6 +364,13 @@ func TestExecutorSetOp(t *testing.T) { in: "set tx_isolation = 'read-committed'", sysVars: map[string]string{"tx_isolation": "'read-committed'"}, result: returnResult("tx_isolation", "varchar", "read-committed"), + }, { + in: "set @@innodb_lock_wait_timeout=120", + sysVars: map[string]string{"innodb_lock_wait_timeout": "120"}, + result: returnResult("innodb_lock_wait_timeout", "int64", "120"), + }, { + in: "set @@global.innodb_lock_wait_timeout=120", + result: returnResult("innodb_lock_wait_timeout", "int64", "120"), }} for _, tcase := range testcases { t.Run(tcase.in, func(t *testing.T) {