From 1d7cb7a4a9f3177c8722555f54e00531b104dfd4 Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Wed, 7 Jul 2021 07:49:41 -0700 Subject: [PATCH 1/3] add MultiShardAutocommitDirective utility Signed-off-by: Michael Demmer --- go/vt/sqlparser/comments.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/go/vt/sqlparser/comments.go b/go/vt/sqlparser/comments.go index f4bc122b95c..1b5d91d297b 100644 --- a/go/vt/sqlparser/comments.go +++ b/go/vt/sqlparser/comments.go @@ -274,6 +274,30 @@ func (d CommentDirectives) IsSet(key string) bool { return false } +// MultiShardAutocommitDirective returns true if multishard autocommit directive is set to true in query. +func MultiShardAutocommitDirective(stmt Statement) bool { + switch stmt := stmt.(type) { + case *Insert: + directives := ExtractCommentDirectives(stmt.Comments) + if directives.IsSet(DirectiveMultiShardAutocommit) { + return true + } + case *Update: + directives := ExtractCommentDirectives(stmt.Comments) + if directives.IsSet(DirectiveMultiShardAutocommit) { + return true + } + case *Delete: + directives := ExtractCommentDirectives(stmt.Comments) + if directives.IsSet(DirectiveMultiShardAutocommit) { + return true + } + default: + return false + } + return false +} + // SkipQueryPlanCacheDirective returns true if skip query plan cache directive is set to true in query. func SkipQueryPlanCacheDirective(stmt Statement) bool { switch stmt := stmt.(type) { From 4f47da6b2646f51735ffa75aef08283d26587254 Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Wed, 7 Jul 2021 07:52:08 -0700 Subject: [PATCH 2/3] add multishard autocommit support for bypass routing Signed-off-by: Michael Demmer Signed-off-by: Andrew Mason --- go/vt/vtgate/engine/send.go | 5 ++++- go/vt/vtgate/planbuilder/bypass.go | 11 ++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/go/vt/vtgate/engine/send.go b/go/vt/vtgate/engine/send.go index 1934b9f6743..f168be15af4 100644 --- a/go/vt/vtgate/engine/send.go +++ b/go/vt/vtgate/engine/send.go @@ -45,6 +45,9 @@ type Send struct { // SingleShardOnly specifies that the query must be send to only single shard SingleShardOnly bool + // MultishardAutocommit specifies that a multishard transaction query can autocommit + MultishardAutocommit bool + noInputs } @@ -97,7 +100,7 @@ func (s *Send) Execute(vcursor VCursor, bindVars map[string]*querypb.BindVariabl canAutocommit := false if s.IsDML { - canAutocommit = len(rss) == 1 && vcursor.AutocommitApproval() + canAutocommit = (len(rss) == 1 || s.MultishardAutocommit) && vcursor.AutocommitApproval() } rollbackOnError := s.IsDML // for non-dml queries, there's no need to do a rollback diff --git a/go/vt/vtgate/planbuilder/bypass.go b/go/vt/vtgate/planbuilder/bypass.go index 9b7fbc0a039..29be8ef08c3 100644 --- a/go/vt/vtgate/planbuilder/bypass.go +++ b/go/vt/vtgate/planbuilder/bypass.go @@ -37,10 +37,11 @@ func buildPlanForBypass(stmt sqlparser.Statement, vschema ContextVSchema) (engin return nil, err } return &engine.Send{ - Keyspace: keyspace, - TargetDestination: vschema.Destination(), - Query: sqlparser.String(stmt), - IsDML: sqlparser.IsDMLStatement(stmt), - SingleShardOnly: false, + Keyspace: keyspace, + TargetDestination: vschema.Destination(), + Query: sqlparser.String(stmt), + IsDML: sqlparser.IsDMLStatement(stmt), + SingleShardOnly: false, + MultishardAutocommit: sqlparser.MultiShardAutocommitDirective(stmt), }, nil } From 9e3859f162d28762df258096a7e8042b9ddb8490 Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Wed, 7 Jul 2021 12:07:14 -0700 Subject: [PATCH 3/3] add tests for bypass multishard autocommit Signed-off-by: Michael Demmer Signed-off-by: Andrew Mason --- go/vt/vtgate/engine/send.go | 3 + go/vt/vtgate/engine/send_test.go | 60 ++++--- go/vt/vtgate/planbuilder/plan_test.go | 25 ++- .../testdata/bypass_keyrange_cases.txt | 163 ++++++++++++++++++ ...ypass_cases.txt => bypass_shard_cases.txt} | 0 5 files changed, 228 insertions(+), 23 deletions(-) create mode 100644 go/vt/vtgate/planbuilder/testdata/bypass_keyrange_cases.txt rename go/vt/vtgate/planbuilder/testdata/{bypass_cases.txt => bypass_shard_cases.txt} (100%) diff --git a/go/vt/vtgate/engine/send.go b/go/vt/vtgate/engine/send.go index f168be15af4..fbdf859176d 100644 --- a/go/vt/vtgate/engine/send.go +++ b/go/vt/vtgate/engine/send.go @@ -159,6 +159,9 @@ func (s *Send) description() PrimitiveDescription { "IsDML": s.IsDML, "SingleShardOnly": s.SingleShardOnly, } + if s.MultishardAutocommit { + other["MultishardAutocommit"] = true + } return PrimitiveDescription{ OperatorType: "Send", Keyspace: s.Keyspace, diff --git a/go/vt/vtgate/engine/send_test.go b/go/vt/vtgate/engine/send_test.go index a0423e9dfd5..38703e26b05 100644 --- a/go/vt/vtgate/engine/send_test.go +++ b/go/vt/vtgate/engine/send_test.go @@ -31,14 +31,15 @@ import ( func TestSendTable(t *testing.T) { type testCase struct { - testName string - sharded bool - shards []string - destination key.Destination - expectedQueryLog []string - expectedError string - isDML bool - singleShardOnly bool + testName string + sharded bool + shards []string + destination key.Destination + expectedQueryLog []string + expectedError string + isDML bool + singleShardOnly bool + multiShardAutocommit bool } singleShard := []string{"0"} @@ -53,7 +54,8 @@ func TestSendTable(t *testing.T) { `ResolveDestinations ks [] Destinations:DestinationAllShards()`, `ExecuteMultiShard ks.0: dummy_query {} false false`, }, - isDML: false, + isDML: false, + multiShardAutocommit: false, }, { testName: "sharded with no autocommit", @@ -64,7 +66,8 @@ func TestSendTable(t *testing.T) { `ResolveDestinations ks [] Destinations:DestinationShard(20-)`, `ExecuteMultiShard ks.DestinationShard(20-): dummy_query {} false false`, }, - isDML: false, + isDML: false, + multiShardAutocommit: false, }, { testName: "unsharded", @@ -75,7 +78,8 @@ func TestSendTable(t *testing.T) { `ResolveDestinations ks [] Destinations:DestinationAllShards()`, `ExecuteMultiShard ks.0: dummy_query {} true true`, }, - isDML: true, + isDML: true, + multiShardAutocommit: false, }, { testName: "sharded with single shard destination", @@ -86,7 +90,8 @@ func TestSendTable(t *testing.T) { `ResolveDestinations ks [] Destinations:DestinationShard(20-)`, `ExecuteMultiShard ks.DestinationShard(20-): dummy_query {} true true`, }, - isDML: true, + isDML: true, + multiShardAutocommit: false, }, { testName: "sharded with multi shard destination", @@ -97,7 +102,20 @@ func TestSendTable(t *testing.T) { `ResolveDestinations ks [] Destinations:DestinationAllShards()`, `ExecuteMultiShard ks.-20: dummy_query {} ks.20-: dummy_query {} true false`, }, - isDML: true, + isDML: true, + multiShardAutocommit: false, + }, + { + testName: "sharded with multi shard destination and autocommit", + sharded: true, + shards: twoShards, + destination: key.DestinationAllShards{}, + expectedQueryLog: []string{ + `ResolveDestinations ks [] Destinations:DestinationAllShards()`, + `ExecuteMultiShard ks.-20: dummy_query {} ks.20-: dummy_query {} true true`, + }, + isDML: true, + multiShardAutocommit: true, }, { testName: "sharded with multi shard destination", @@ -107,9 +125,10 @@ func TestSendTable(t *testing.T) { expectedQueryLog: []string{ `ResolveDestinations ks [] Destinations:DestinationAllShards()`, }, - expectedError: "Unexpected error, DestinationKeyspaceID mapping to multiple shards: dummy_query, got: DestinationAllShards()", - isDML: true, - singleShardOnly: true, + expectedError: "Unexpected error, DestinationKeyspaceID mapping to multiple shards: dummy_query, got: DestinationAllShards()", + isDML: true, + singleShardOnly: true, + multiShardAutocommit: false, }, } @@ -120,10 +139,11 @@ func TestSendTable(t *testing.T) { Name: "ks", Sharded: tc.sharded, }, - Query: "dummy_query", - TargetDestination: tc.destination, - IsDML: tc.isDML, - SingleShardOnly: tc.singleShardOnly, + Query: "dummy_query", + TargetDestination: tc.destination, + IsDML: tc.isDML, + SingleShardOnly: tc.singleShardOnly, + MultishardAutocommit: tc.multiShardAutocommit, } vc := &loggingVCursor{shards: tc.shards} _, err := send.Execute(vc, map[string]*querypb.BindVariable{}, false) diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index 5a340d50110..a73e62e2971 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -205,10 +205,29 @@ func TestOne(t *testing.T) { testFile(t, "onecase.txt", "", vschema) } -func TestBypassPlanningFromFile(t *testing.T) { +func TestBypassPlanningShardTargetFromFile(t *testing.T) { testOutputTempDir, err := ioutil.TempDir("", "plan_test") require.NoError(t, err) defer os.RemoveAll(testOutputTempDir) + + vschema := &vschemaWrapper{ + v: loadSchema(t, "schema_test.json"), + keyspace: &vindexes.Keyspace{ + Name: "main", + Sharded: false, + }, + tabletType: topodatapb.TabletType_MASTER, + dest: key.DestinationShard("-80")} + + testFile(t, "bypass_shard_cases.txt", testOutputTempDir, vschema) +} +func TestBypassPlanningKeyrangeTargetFromFile(t *testing.T) { + testOutputTempDir, err := ioutil.TempDir("", "plan_test") + require.NoError(t, err) + defer os.RemoveAll(testOutputTempDir) + + keyRange, _ := key.ParseShardingSpec("-") + vschema := &vschemaWrapper{ v: loadSchema(t, "schema_test.json"), keyspace: &vindexes.Keyspace{ @@ -216,10 +235,10 @@ func TestBypassPlanningFromFile(t *testing.T) { Sharded: false, }, tabletType: topodatapb.TabletType_MASTER, - dest: key.DestinationShard("-80"), + dest: key.DestinationExactKeyRange{KeyRange: keyRange[0]}, } - testFile(t, "bypass_cases.txt", testOutputTempDir, vschema) + testFile(t, "bypass_keyrange_cases.txt", testOutputTempDir, vschema) } func TestWithDefaultKeyspaceFromFile(t *testing.T) { diff --git a/go/vt/vtgate/planbuilder/testdata/bypass_keyrange_cases.txt b/go/vt/vtgate/planbuilder/testdata/bypass_keyrange_cases.txt new file mode 100644 index 00000000000..a9bb3e93249 --- /dev/null +++ b/go/vt/vtgate/planbuilder/testdata/bypass_keyrange_cases.txt @@ -0,0 +1,163 @@ +# select bypass +"select count(*), col from unsharded" +{ + "QueryType": "SELECT", + "Original": "select count(*), col from unsharded", + "Instructions": { + "OperatorType": "Send", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "ExactKeyRange(-)", + "Query": "select count(*), col from unsharded" + } +} +Gen4 plan same as above + +# update bypass +"update user set val = 1 where id = 18446744073709551616 and id = 1" +{ + "QueryType": "UPDATE", + "Original": "update user set val = 1 where id = 18446744073709551616 and id = 1", + "Instructions": { + "OperatorType": "Send", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "ExactKeyRange(-)", + "IsDML": true, + "Query": "update `user` set val = 1 where id = 18446744073709551616 and id = 1" + } +} +Gen4 plan same as above + +# update bypass autocommit +"update /*vt+ MULTI_SHARD_AUTOCOMMIT=1 */ user set val = 1 where id = 18446744073709551616 and id = 1" +{ + "QueryType": "UPDATE", + "Original": "update /*vt+ MULTI_SHARD_AUTOCOMMIT=1 */ user set val = 1 where id = 18446744073709551616 and id = 1", + "Instructions": { + "OperatorType": "Send", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "ExactKeyRange(-)", + "IsDML": true, + "MultishardAutocommit": true, + "Query": "update /*vt+ MULTI_SHARD_AUTOCOMMIT=1 */ `user` set val = 1 where id = 18446744073709551616 and id = 1" + } +} +Gen4 plan same as above + +# delete bypass +"DELETE FROM USER WHERE ID = 42" +{ + "QueryType": "DELETE", + "Original": "DELETE FROM USER WHERE ID = 42", + "Instructions": { + "OperatorType": "Send", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "ExactKeyRange(-)", + "IsDML": true, + "Query": "delete from `USER` where ID = 42" + } +} +Gen4 plan same as above + +# insert bypass: not supported +"INSERT INTO USER (ID, NAME) VALUES (42, 'ms X')" +"INSERT not supported when targeting a key range: targetString" +Gen4 plan same as above + +# bypass query for into outfile s3 +"select count(*), col from unsharded into outfile S3 'x.txt'" +{ + "QueryType": "SELECT", + "Original": "select count(*), col from unsharded into outfile S3 'x.txt'", + "Instructions": { + "OperatorType": "Send", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "ExactKeyRange(-)", + "Query": "select count(*), col from unsharded into outfile s3 'x.txt'" + } +} +Gen4 plan same as above + +# Select outfile +"select * from user into outfile S3 'x.txt'" +{ + "QueryType": "SELECT", + "Original": "select * from user into outfile S3 'x.txt'", + "Instructions": { + "OperatorType": "Send", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "ExactKeyRange(-)", + "Query": "select * from `user` into outfile s3 'x.txt'" + } +} +Gen4 plan same as above + +"load data from s3 'x.txt' into table x" +{ + "QueryType": "OTHER", + "Original": "load data from s3 'x.txt' into table x", + "Instructions": { + "OperatorType": "Send", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "ExactKeyRange(-)", + "IsDML": true, + "Query": "load data from s3 'x.txt' into table x", + "SingleShardOnly": true + } +} +Gen4 plan same as above + +"load data from s3 'x.txt'" +{ + "QueryType": "OTHER", + "Original": "load data from s3 'x.txt'", + "Instructions": { + "OperatorType": "Send", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "ExactKeyRange(-)", + "IsDML": true, + "Query": "load data from s3 'x.txt'", + "SingleShardOnly": true + } +} +Gen4 plan same as above + +# create table +"create /* test */ table t1(id bigint, primary key(id)) /* comments */" +{ + "QueryType": "DDL", + "Original": "create /* test */ table t1(id bigint, primary key(id)) /* comments */", + "Instructions": { + "OperatorType": "Send", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetDestination": "ExactKeyRange(-)", + "Query": "create /* test */ table t1(id bigint, primary key(id)) /* comments */" + } +} +Gen4 plan same as above diff --git a/go/vt/vtgate/planbuilder/testdata/bypass_cases.txt b/go/vt/vtgate/planbuilder/testdata/bypass_shard_cases.txt similarity index 100% rename from go/vt/vtgate/planbuilder/testdata/bypass_cases.txt rename to go/vt/vtgate/planbuilder/testdata/bypass_shard_cases.txt