VtGateV3 sequence testcases in GO by ajeetj · Pull Request #3 · planetscale/vitess

ajeetj · 2019-10-04T11:38:30Z

Rewrote VtGateV3 sequence testcases from Python to Go.
Related python code reference https://github.com/planetscale/vitess/blob/master/test/vtgatev3_test.py#L1575-L1609

This is the first PR for review and to collect feedback.

Note: We all are merging changes related to vtgate in "tal_vtgate" repo.

Signed-off-by: Ajeet jain <ajeet@planetscale.com>

ajeetj · 2019-10-04T11:39:47Z

go/vt/vtgate/endtoend/sequence_test.go

+	defer conn.Close()
+
+	//Why below query doesn't work for seq table?
+	//exec(t, conn, "insert into sequence_test_seq(id, next_id, cache) values(0,1,10)")


@deepthi Do you know why the above line will not work?
I had to pass this insert as a setup DDL to make it work.

What error did you get?

I get below error: Is there any restriction that the seq table should be prepopulated before bringing the cluster?

E1007 11:53:54.143075 17761 server.go:270] mysql_server caught panic: runtime error: index out of range [0] with length 0 /usr/local/go/src/runtime/panic.go:75 (0x102f322) goPanicIndex: panic(boundsError{x: int64(x), signed: true, y: y, code: boundsIndex}) /Users/ajeetjain/ps/vitess/go/vt/vtgate/engine/insert.go:394 (0x1a27ca0) io/vitess/go/vt/vtgate/engine.(*Insert).getInsertShardedRoute: keyspaceIDs, err := ins.processPrimary(vcursor, vindexRowsValues[0], ins.Table.ColumnVindexes[0], bindVars) /Users/ajeetjain/ps/vitess/go/vt/vtgate/engine/insert.go:272 (0x1a25488) io/vitess/go/vt/vtgate/engine.(*Insert).execInsertSharded: rss, queries, err := ins.getInsertShardedRoute(vcursor, bindVars) /Users/ajeetjain/ps/vitess/go/vt/vtgate/engine/insert.go:222 (0x1a249f2) io/vitess/go/vt/vtgate/engine.(*Insert).Execute: return ins.execInsertSharded(vcursor, bindVars) /Users/ajeetjain/ps/vitess/go/vt/vtgate/executor.go:304 (0x1a780c8) io/vitess/go/vt/vtgate.(*Executor).handleExec: qr, err := plan.Instructions.Execute(vcursor, bindVars, true) /Users/ajeetjain/ps/vitess/go/vt/vtgate/executor.go:210 (0x1a769f7) io/vitess/go/vt/vtgate.(*Executor).execute: qr, err := e.handleExec(ctx, safeSession, sql, bindVars, destKeyspace, destTabletType, dest, logStats) /Users/ajeetjain/ps/vitess/go/vt/vtgate/executor.go:134 (0x1a7659b) io/vitess/go/vt/vtgate.(*Executor).Execute: result, err = e.execute(ctx, safeSession, sql, bindVars, logStats) /Users/ajeetjain/ps/vitess/go/vt/vtgate/vtgate.go:289 (0x1a9a741) io/vitess/go/vt/vtgate.(*VTGate).Execute: qr, err = vtg.executor.Execute(ctx, "Execute", NewSafeSession(session), sql, bindVariables) /Users/ajeetjain/ps/vitess/go/vt/vtgate/plugin_mysql_server.go:206 (0x1a883d5) io/vitess/go/vt/vtgate.(*vtgateHandler).ComQuery: session, result, err := vh.vtg.Execute(ctx, session, query, make(map[string]*querypb.BindVariable)) /Users/ajeetjain/ps/vitess/go/mysql/conn.go:1070 (0x148e647) io/vitess/go/mysql.(*Conn).execQuery: err := handler.ComQuery(c, query, func(qr *sqltypes.Result) error { /Users/ajeetjain/ps/vitess/go/mysql/conn.go:768 (0x148dec0) io/vitess/go/mysql.(*Conn).handleNextCommand: if err := c.execQuery(sql, handler, more); err != nil { /Users/ajeetjain/ps/vitess/go/mysql/server.go:452 (0x14a8e0e) io/vitess/go/mysql.(*Listener).handle: err := c.handleNextCommand(l.handler) /usr/local/go/src/runtime/asm_amd64.s:1357 (0x1060b30) goexit: BYTE $0x90 // NOP --- FAIL: TestSeq (0.01s) sequence_test.go:36: EOF (errno 2013) (sqlstate HY000) during query: insert into sequence_test_seq(id, next_id, cache) values(0,1,10) I1007 11:53:54.149751 17761 engine.go:278]

This is because sequence_test_seq is in a sharded keyspace, but no vindex is defined for it.

ajeetj · 2019-10-04T11:40:58Z

go/vt/vtgate/endtoend/main_test.go

+					// Type: sqltypes.Int64,
+				}, {
+					Name: "cache",
+					// Type: sqltypes.Int64,


@deepthi Do we need to list all the columns from a sequence table in VSchema?

I don't think so. Something like this should work (in an unsharded keyspace):

{ "tables": { "test_seq": { "type": "sequence" } } }

Yes, I tested that yesterday it worked. I don't remember the error I was getting before.

ajeetj · 2019-10-09T11:12:34Z

@deepthi The above tests runs fine for 1st and 2nd time but fails if I keep executing them multiple times. I am not able to figure out why is that. I tried to use transaction begin/commit or sync.WaitGroup or the sleep function but saw the same behavior.
Could this be because I am creating the sequence table in a sharded keyspace?

deepthi · 2019-10-09T15:32:25Z

@deepthi The above tests runs fine for 1st and 2nd time but fails if I keep executing them multiple times. I am not able to figure out why is that. I tried to use transaction begin/commit or sync.WaitGroup or the sleep function but saw the same behavior.
Could this be because I am creating the sequence table in a sharded keyspace?

This means that the test's tearDown is not cleaning things up properly.
But you bring up a good point. Why is the sequence table in the sharded keyspace? Is that what the original python test was doing? This might be the cause of the panic you were getting.

ajeetj · 2019-10-09T15:42:04Z

I couldn't find a way to add multiple keyspaces in the current go implementation using the VTCOMBO. And then I noticed in the documents that it is possible to put the sequence table in a sharded keyspace by pinning it to a single shard. So I was trying with that. Teardown is working fine as first test is always passing. Its the test 2 and 3 which fails intermittently, I doubt if I am setting up them wrong.

…

On Wed, 9 Oct, 2019, 9:02 PM Deepthi Sigireddi, ***@***.***> wrote: @deepthi <https://github.com/deepthi> The above tests runs fine for 1st and 2nd time but fails if I keep executing them multiple times. I am not able to figure out why is that. I tried to use transaction begin/commit or sync.WaitGroup or the sleep function but saw the same behavior. Could this be because I am creating the sequence table in a sharded keyspace? This means that the test's tearDown is not cleaning things up properly. But you bring up a good point. Why is the sequence table in the sharded keyspace? Is that what the original python test was doing? This might be the cause of the panic you were getting. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3?email_source=notifications&email_token=AAJNZOCG5BY24DPP2J7ZWHTQNX2Q3A5CNFSM4I5OWDW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAYJQAA#issuecomment-540055552>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJNZOHKSCIAGBYVTIFF3DDQNX2Q3ANCNFSM4I5OWDWQ> .

deepthi · 2019-10-10T01:43:38Z

I couldn't find a way to add multiple keyspaces in the current go implementation using the VTCOMBO. And then I noticed in the documents that it is possible to put the sequence table in a sharded keyspace by pinning it to a single shard. So I was trying with that. Teardown is working fine as first test is always passing. Its the test 2 and 3 which fails intermittently, I doubt if I am setting up them wrong.

I see that VTTestTopology does allow you to have multiple keyspaces. If you were to add another unsharded keyspace to the list it has, and separate the schema and vschema for sequence_test_seq, is that causing a problem later on?

ajeetj · 2019-10-10T02:53:59Z

I couldn't find a way to add multiple keyspaces in the current go implementation using the VTCOMBO. And then I noticed in the documents that it is possible to put the sequence table in a sharded keyspace by pinning it to a single shard. So I was trying with that. Teardown is working fine as first test is always passing. Its the test 2 and 3 which fails intermittently, I doubt if I am setting up them wrong.

I see that VTTestTopology does allow you to have multiple keyspaces. If you were to add another unsharded keyspace to the list it has, and separate the schema and vschema for sequence_test_seq, is that causing a problem later on?

Yes, I tried that as well yesterday but was stuck in the config.SchemaDir. It complains that it has already set. I also notice below comment on the "InitSchemas" function

// InitSchemas is a shortcut for tests that just want to setup a single
// keyspace with a single SQL file, and/or a vschema.
// It creates a temporary directory, and puts the schema/vschema in there.
// It then sets the right value for cfg.SchemaDir.
// At the end of the test, the caller should os.RemoveAll(cfg.SchemaDir).
func (cfg *Config) InitSchemas(keyspace, schema string, vschema *vschemapb.Keyspace) error {

I couldn't find any other implementation which uses multiple VSchema for GO endtoend testing. Maybe we will need to priorities writing the new cluster code now.

deepthi · 2019-10-10T03:01:29Z

Can you try either:
a) change InitSchemas to work for multiple keyspaces?
or
b) write another function that will do that?

The preference is (a)

deepthi · 2019-10-10T03:04:38Z

Teardown is working fine as first test is always passing. Its the test 2 and 3 which fails intermittently, I doubt if I am setting up them wrong.

I see what you mean. First run passes and then it fails either on [[INT64(5)]] or [[INT64(11)]]
It is a mystery to me right now. Maybe fixing the keyspace / schema creation will help.

ajeetj · 2019-10-10T03:07:42Z

Can you try either:
a) change InitSchemas to work for multiple keyspaces?
or
b) write another function that will do that?

The preference is (a)

OK, Will look into this today. Thanks

ajeetj · 2019-10-22T10:07:43Z

Rewrote the above using a new cluster which is working fine.
Ref: #3

@deepthi
Do we need to debug why this was failing?

deepthi · 2019-10-22T15:57:00Z

Rewrote the above using a new cluster which is working fine.
Ref: #3

@deepthi
Do we need to debug why this was failing?

Not needed.

Format vtcompose

…ion/cancellation This is a combination of 4 commits. This is the commit message #2: Check that routing rules are deleted in unit test This is the commit message #3: Fix typo This is the commit message #4: Refresh all tablets in a shard when updating blacklisted tables Signed-off-by: Rohit Nayak <rohit@planetscale.com> Signed-off-by: Andres Taylor <andres@planetscale.com>

Signed-off-by: Matt Lord <mattalord@gmail.com>

* Implement VDiff2 delete command Also add --verbose flag for VDiff output Signed-off-by: Matt Lord <mattalord@gmail.com> * Delete vdiff metadata associated with a deleted workflow Signed-off-by: Matt Lord <mattalord@gmail.com> * Tweaks after self review Signed-off-by: Matt Lord <mattalord@gmail.com> * Self review #2 Signed-off-by: Matt Lord <mattalord@gmail.com> * Spelling is herd Signed-off-by: Matt Lord <mattalord@gmail.com> * Add test for vdiff data cleanup on workflow deletion Signed-off-by: Matt Lord <mattalord@gmail.com> * Caught mistake on self-review #3 Signed-off-by: Matt Lord <mattalord@gmail.com> * Add a couple more delete tests Signed-off-by: Matt Lord <mattalord@gmail.com> * Clear out vdiff_log table as well on workflow deletion Signed-off-by: Matt Lord <mattalord@gmail.com>

* decouple olap tx timeout from oltp tx timeout Since workload=olap bypasses the query timeouts (--queryserver-config-query-timeout) and also row limits, the natural assumption is that it also bypasses the transaction timeout. This is not the case, e.g. for a tablet where the --queryserver-config-transaction-timeout is 10. This commit: * Adds new CLI flag and YAML field to independently configure TX timeouts for OLAP workloads (--queryserver-config-olap-transaction-timeout). * Decouples TX kill interval from OLTP TX timeout via new CLI flag and YAML field (--queryserver-config-transaction-killer-interval). Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #1 Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #2 consolidate timeout logic in sc Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: remove unused tx killer flag Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: update 15_0_0_summary.md Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: fix race cond Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #3 -txProps.timeout, +sc.expiryTime Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #4 -atomic.Value for expiryTime Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: fix race cond (without atomic.Value) Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #5 -unused funcs, fix comments, set ticks interval once Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #5 +txkill tests Signed-off-by: Max Englander <max@planetscale.com> * revert fmt changes Signed-off-by: Max Englander <max@planetscale.com> * implement pr review suggestion Signed-off-by: Max Englander <max@planetscale.com> Signed-off-by: Max Englander <max@planetscale.com>

* decouple olap tx timeout from oltp tx timeout Since workload=olap bypasses the query timeouts (--queryserver-config-query-timeout) and also row limits, the natural assumption is that it also bypasses the transaction timeout. This is not the case, e.g. for a tablet where the --queryserver-config-transaction-timeout is 10. This commit: * Adds new CLI flag and YAML field to independently configure TX timeouts for OLAP workloads (--queryserver-config-olap-transaction-timeout). * Decouples TX kill interval from OLTP TX timeout via new CLI flag and YAML field (--queryserver-config-transaction-killer-interval). Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #1 Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #2 consolidate timeout logic in sc Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: remove unused tx killer flag Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: update 15_0_0_summary.md Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: fix race cond Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #3 -txProps.timeout, +sc.expiryTime Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #4 -atomic.Value for expiryTime Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: fix race cond (without atomic.Value) Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #5 -unused funcs, fix comments, set ticks interval once Signed-off-by: Max Englander <max@planetscale.com> * decouple ol{a,t}p tx timeouts: pr comments #5 +txkill tests Signed-off-by: Max Englander <max@planetscale.com> * fix flags Signed-off-by: Max Englander <max@planetscale.com> Signed-off-by: Max Englander <max@planetscale.com>

vtgate sequence tests

c4a0387

Signed-off-by: Ajeet jain <ajeet@planetscale.com>

ajeetj requested a review from sougou as a code owner October 4, 2019 11:38

ajeetj requested a review from deepthi October 4, 2019 11:38

ajeetj commented Oct 4, 2019

View reviewed changes

Merge branch 'tal_vtgate' into tal_vtgate_seq

f3d3fef

ajeetj closed this Oct 22, 2019

morgo pushed a commit that referenced this pull request Mar 31, 2020

Merge pull request #3 from keeferrourke/format-vtcompose

c54d960

Format vtcompose

mattlord added a commit that referenced this pull request Jun 29, 2022

Caught mistake on self-review #3

0f4b832

Signed-off-by: Matt Lord <mattalord@gmail.com>

harshit-gangal deleted the tal_vtgate_seq branch May 9, 2023 09:52

Conversation

ajeetj commented Oct 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ajeetj Oct 4, 2019

Choose a reason for hiding this comment

Uh oh!

deepthi Oct 7, 2019

Choose a reason for hiding this comment

Uh oh!

ajeetj Oct 7, 2019

Choose a reason for hiding this comment

Uh oh!

deepthi Oct 10, 2019

Choose a reason for hiding this comment

Uh oh!

ajeetj Oct 4, 2019

Choose a reason for hiding this comment

Uh oh!

deepthi Oct 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajeetj Oct 10, 2019

Choose a reason for hiding this comment

Uh oh!

ajeetj commented Oct 9, 2019

Uh oh!

deepthi commented Oct 9, 2019

Uh oh!

ajeetj commented Oct 9, 2019 via email

Uh oh!

deepthi commented Oct 10, 2019

Uh oh!

ajeetj commented Oct 10, 2019

Uh oh!

deepthi commented Oct 10, 2019

Uh oh!

deepthi commented Oct 10, 2019

Uh oh!

ajeetj commented Oct 10, 2019

Uh oh!

ajeetj commented Oct 22, 2019

Uh oh!

deepthi commented Oct 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ajeetj commented Oct 4, 2019 •

edited

Loading

deepthi Oct 10, 2019 •

edited

Loading