Use a left join to make sure that tables with tablespace=innodb_system are included in the schema #12672

kovyrin · 2023-03-20T17:45:15Z

Description

To make sure the behaviour of the schema engine is the same for 5.7 and 8.0 in terms of handling of tables that have innodb tablespace set to innodb_system (those are not included in information_schema.innodb_tablespaces). This should make sure that vstreamer works as expected when some of the tables in a keyspace have been created with tablespace innodb_system option (because of https://bugs.mysql.com/bug.php?id=110383 or some other reason).

This issue exists in 14.x and the fix needs backporting AFAIU.

Related Issue(s)

Fixes #12669

Checklist

"Backport to:" labels have been added if this change should be back-ported
Tests were added or are not required
Did the new or modified tests pass consistently locally and on the CI
Documentation is not required

…m are not skipped when loading the schema (fixes vitessio#12669) Signed-off-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com>

kovyrin · 2023-03-20T19:23:58Z

Not sure how to (or if I should) write a test for this, but tested it locally as follows:

Checked out this branch locally
Built Vitess
Deployed a test cluster locally
Created a test table in the commerce keyspace
Used grpcurl to connect to vstream API
Inserted data into the test table
Observed vstream responses in the grpcurl console

mattlord

Makes sense to me -- we want all tables, even if they don't have an associated dedicated tablespace (I would guess this is also an issue when using general tablespaces, as that's what the system_tablespace is). Thank you for this, @kovyrin ! ❤️

mattlord

We could add a minimal test by e.g. creating the product table here in the system_tablespace: https://github.com/vitessio/vitess/blob/main/go/test/endtoend/vreplication/config_test.go#L37

@kovyrin make sense? Please let me know if you need any help.

@harshit-gangal maybe you have a better idea though? Schema Tracker tests may be better, e.g. here: https://github.com/vitessio/vitess/blob/main/go/test/endtoend/vtgate/schematracker/sharded/schema.sql

@kovyrin with this patch:

diff --git a/go/test/endtoend/vreplication/vstream_test.go b/go/test/endtoend/vreplication/vstream_test.go
index 1098632d87..2b3eaf4bbd 100644
--- a/go/test/endtoend/vreplication/vstream_test.go
+++ b/go/test/endtoend/vreplication/vstream_test.go
@@ -190,7 +190,7 @@ const vschemaUnsharded = `
 }
 `
 const schemaSharded = `
-create table customer(cid int, name varbinary(128), primary key(cid))  CHARSET=utf8mb4;
+create table customer(cid int, name varbinary(128), primary key(cid)) TABLESPACE innodb_system CHARSET=utf8mb4;
 `
 const vschemaSharded = `
 {

This fails on main:

$  go test -timeout 10m -v -failfast -run TestVStreamStopOnReshardTrue vitess.io/vitess/go/test/endtoend/vreplication

So that could serve as the test for your PR.

kovyrin · 2023-03-20T20:46:51Z

Having a very hard time running any of the existing tests locally. It reliably fails to start mysql both in vreplication end to end tests and in vtgate/schematracker :-(

One example:

❯ go test -timeout 30s -run ^TestVStreamFailover$ vitess.io/vitess/go/test/endtoend/vreplication
VTDATAROOT is vreple2e_268107
E0320 16:46:12.474533   72873 vtctldclient_process.go:101] CreateKeyspace does not support the --sidecar-db-name flag in vtctl version 15; ignoring...
E0320 16:46:13.084229   72873 vtorc_process.go:92] configuration - {
	"Debug": true,
	"ListenAddress": ":0",
	"MySQLTopologyUser": "orc_client_user",
	"MySQLTopologyPassword": "orc_client_user_password",
	"MySQLReplicaUser": "vt_repl",
	"MySQLReplicaPassword": "",
	"RecoveryPeriodBlockSeconds": 1
}
--- FAIL: TestVStreamFailover (14.75s)
    cluster_test.go:554: exit status 1 :: Unable to start mysql server for &{zone1-100 vttablet vreple2e_268107/tmp/vt_0000000100_querylog.txt 100 zone1-0000000100  16100 17091 0 {vtctl vtctl  etcd2 127.0.0.1:2379 /vitess/global 127.0.0.1:2379 / 15} vreple2e_268107/tmp 127.0.0.1 product replica 5 file vreple2e_268107/backups grpc-queryservice,grpc-tabletmanager,grpc-updatestream,grpc-throttler http://127.0.0.1:15000 vreple2e_268107/vt_0000000100 http://127.0.0.1:16100/debug/vars http://127.0.0.1:16100/queryz http://127.0.0.1:16100/debug/status_details false NOT_SERVING  0 PRIMARY  utf8mb4 http://127.0.0.1:16100/debug/consolidations [--queryserver-config-schema-reload-time 5 --enable-lag-throttler --heartbeat_enable --heartbeat_interval 250ms] <nil> <nil>}
FAIL
FAIL	vitess.io/vitess/go/test/endtoend/vreplication	15.366s
FAIL

Another one:

❯ go test -timeout 30s -run ^TestNewTable$ vitess.io/vitess/go/test/endtoend/vtgate/
E0320 16:41:20.982548   71466 vtctl_process.go:67] CreateKeyspace does not support the --sidecar-db-name flag in vtctl version 15; ignoring...
E0320 16:41:21.410968   71466 cluster_process.go:407] unable to start mysql process /opt/homebrew/bin/mysqlctl --log_dir vtroot_7101/tmp_7103 --tablet_uid 6660 --mysql_port 7109 init -- --init_db_sql_file /config/init_db.sql start: exit status 1
FAIL	vitess.io/vitess/go/test/endtoend/vtgate	4.094s
FAIL

Signed-off-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com>

kovyrin · 2023-03-20T21:30:17Z

Modified the customer schema to use the system tablespace. This should cover the change on 8.0 (breaks on main).

harshit-gangal

how do we get size for those tables?

mattlord · 2023-03-25T05:10:43Z

how do we get size for those tables?

@harshit-gangal Good question! We'd need to do more (although I'd argue that recognizing the table w/o a known size is still an improvement over not being aware of it at all).

This is one possibility:

## Current query 
mysql> SELECT t.table_name, t.table_type, UNIX_TIMESTAMP(t.create_time), t.table_comment,  SUM(i.file_size) as storage_size,  SUM(i.allocated_s
ize)  FROM information_schema.tables t LEFT JOIN information_schema.innodb_tablespaces i ON i.name LIKE CONCAT(database(), '/%') AND (i.name =
CONCAT(t.table_schema, '/', t.table_name) OR i.name LIKE CONCAT(t.table_schema, '/', t.table_name, '#p#%'))  WHERE t.table_schema = database()
 GROUP BY t.table_name, t.table_type, t.create_time, t.table_comment\G
*************************** 1. row ***************************
                   TABLE_NAME: corder
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679718422
                TABLE_COMMENT:
                 storage_size: 114688
        SUM(i.allocated_size): 114688
*************************** 2. row ***************************
                   TABLE_NAME: customer
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679718422
                TABLE_COMMENT:
                 storage_size: 114688
        SUM(i.allocated_size): 114688
*************************** 3. row ***************************
                   TABLE_NAME: test1
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679719746
                TABLE_COMMENT:
                 storage_size: NULL
        SUM(i.allocated_size): NULL
*************************** 4. row ***************************
                   TABLE_NAME: ti
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679720520
                TABLE_COMMENT:
                 storage_size: 688128
        SUM(i.allocated_size): 688128
4 rows in set (0.01 sec)

## New Query
mysql> SELECT t.table_name, t.table_type, UNIX_TIMESTAMP(t.create_time), t.table_comment, IFNULL(SUM(i.file_size), (select data_length+index_length from information_schema.tables where table_name=t.table_name)) as storage_size,  SUM(i.allocated_size)  FROM information_schema.tables t LEFT JOIN information_schema.innodb_tablespaces i ON i.name LIKE CONCAT(database(), '/%') AND (i.name = CONCAT(t.table_schema, '/', t.table_name) OR i.name LIKE CONCAT(t.table_schema, '/', t.table_name, '#p#%'))  WHERE t.table_schema = database()  GROUP BY t.table_name, t.table_type, t.create_time, t.table_comment\G
*************************** 1. row ***************************
                   TABLE_NAME: corder
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679718422
                TABLE_COMMENT:
                 storage_size: 114688
        SUM(i.allocated_size): 114688
*************************** 2. row ***************************
                   TABLE_NAME: customer
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679718422
                TABLE_COMMENT:
                 storage_size: 114688
        SUM(i.allocated_size): 114688
*************************** 3. row ***************************
                   TABLE_NAME: test1
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679719746
                TABLE_COMMENT:
                 storage_size: 16384
        SUM(i.allocated_size): NULL
*************************** 4. row ***************************
                   TABLE_NAME: ti
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679720520
                TABLE_COMMENT:
                 storage_size: 688128
        SUM(i.allocated_size): 688128
4 rows in set (0.00 sec)

This makes the query more complicated and heavier, so it's likely to be problematic for those with many thousands of tables.

As a practical matter, it may be better to accept NULL or "unknown" size in these rare cases. If a non-integral result is the problem, we could use this instead: IFNULL(SUM(i.file_size), 0) as storage_size. That doesn't add any overhead to the query:

mysql> SELECT t.table_name, t.table_type, UNIX_TIMESTAMP(t.create_time), t.table_comment, IFNULL(SUM(i.file_size), 0) as storage_size, IFNULL(SUM(i.allocated_size), 0) as allocated_size  FROM information_schema.tables t LEFT JOIN information_schema.innodb_tablespaces i ON i.name LIKE C
ONCAT(database(), '/%') AND (i.name = CONCAT(t.table_schema, '/', t.table_name) OR i.name LIKE CONCAT(t.table_schema, '/', t.table_name, '#p#%'))
*************************** 1. row ***************************
                   TABLE_NAME: corder
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679718422
                TABLE_COMMENT:
                 storage_size: 114688
               allocated_size: 114688
*************************** 2. row ***************************
                   TABLE_NAME: customer
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679718422
                TABLE_COMMENT:
                 storage_size: 114688
               allocated_size: 114688
*************************** 3. row ***************************
                   TABLE_NAME: test1
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679719746
                TABLE_COMMENT:
                 storage_size: 0
               allocated_size: 0
*************************** 4. row ***************************
                   TABLE_NAME: ti
                   TABLE_TYPE: BASE TABLE
UNIX_TIMESTAMP(t.create_time): 1679720520
                TABLE_COMMENT:
                 storage_size: 688128
               allocated_size: 688128
4 rows in set (0.01 sec)

…ce=innodb_system are included in the schema (#12672) (#12734) * Use a left join to make sure that tables with tablespace=innodb_system are not skipped when loading the schema (fixes #12669) Signed-off-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com> * Set tablespace on a test table to see if it breaks vstreamer Signed-off-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com> --------- Signed-off-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com> Co-authored-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com>

…ce=innodb_system are included in the schema (#12672) (#12735) * Use a left join to make sure that tables with tablespace=innodb_system are not skipped when loading the schema (fixes #12669) Signed-off-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com> * Set tablespace on a test table to see if it breaks vstreamer Signed-off-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com> --------- Signed-off-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com> Co-authored-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com>

Use a left join to make sure that tables with tablespace=innodb_syste…

6c3a7ee

…m are not skipped when loading the schema (fixes vitessio#12669) Signed-off-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com>

kovyrin force-pushed the fix-schema-query branch from cbc629e to 6c3a7ee Compare March 20, 2023 17:46

kovyrin mentioned this pull request Mar 20, 2023

Bug Report: No schema found for table in vstreamer for tables with TABLESPACE innodb_system #12669

Closed

kovyrin marked this pull request as ready for review March 20, 2023 19:24

kovyrin requested review from harshit-gangal, systay and mattlord as code owners March 20, 2023 19:24

mattlord added Type: Bug Component: Query Serving Backport to: release-16.0 labels Mar 20, 2023

mattlord approved these changes Mar 20, 2023

View reviewed changes

This comment was marked as outdated.

Sign in to view

mattlord requested changes Mar 20, 2023

View reviewed changes

Set tablespace on a test table to see if it breaks vstreamer

8216851

Signed-off-by: Oleksiy Kovyrin <oleksiy.kovyrin@shopify.com>

kovyrin requested a review from deepthi as a code owner March 20, 2023 21:15

kovyrin requested review from mattlord and removed request for deepthi, systay and harshit-gangal March 20, 2023 21:16

mattlord approved these changes Mar 21, 2023

View reviewed changes

mattlord requested review from arthurschreiber and harshit-gangal March 21, 2023 02:02

harshit-gangal reviewed Mar 21, 2023

View reviewed changes

This was referenced Mar 27, 2023

Release of v16.0.1 #12697

Closed

Release of v15.0.3 #12696

Closed

frouioui merged commit 0de7e01 into vitessio:main Mar 27, 2023

austenLacy mentioned this pull request Apr 26, 2023

Fixes the SwitchTraffic bug that wasn't respecting --dry_run for readonly and replica tablets during a resharding event Shopify/vitess#91

Merged

austenLacy mentioned this pull request Aug 1, 2023

Custom jwt http ACL policy Shopify/vitess#114

Closed

mattlord mentioned this pull request Aug 29, 2023

Fixing Mysqld.normalizedSchema() by formally parsing the query #13866

Closed

4 tasks

hmaurer mentioned this pull request Mar 21, 2024

oops #15542

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a left join to make sure that tables with tablespace=innodb_system are included in the schema #12672

Use a left join to make sure that tables with tablespace=innodb_system are included in the schema #12672

kovyrin commented Mar 20, 2023 •

edited

Loading

kovyrin commented Mar 20, 2023

mattlord left a comment

This comment was marked as outdated.

mattlord left a comment •

edited

Loading

kovyrin commented Mar 20, 2023

kovyrin commented Mar 20, 2023

harshit-gangal left a comment

mattlord commented Mar 25, 2023 •

edited

Loading

Use a left join to make sure that tables with tablespace=innodb_system are included in the schema #12672

Use a left join to make sure that tables with tablespace=innodb_system are included in the schema #12672

Conversation

kovyrin commented Mar 20, 2023 • edited Loading

Description

Related Issue(s)

Checklist

kovyrin commented Mar 20, 2023

mattlord left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

mattlord left a comment • edited Loading

Choose a reason for hiding this comment

kovyrin commented Mar 20, 2023

kovyrin commented Mar 20, 2023

harshit-gangal left a comment

Choose a reason for hiding this comment

mattlord commented Mar 25, 2023 • edited Loading

kovyrin commented Mar 20, 2023 •

edited

Loading

mattlord left a comment •

edited

Loading

mattlord commented Mar 25, 2023 •

edited

Loading