From 354bff981b5c23abf65bd010cea0aab6ff26e1e9 Mon Sep 17 00:00:00 2001 From: "vitess-bot[bot]" <108069721+vitess-bot[bot]@users.noreply.github.com> Date: Thu, 25 Apr 2024 12:43:45 +0200 Subject: [PATCH 1/3] Improve performance for `BaseShowTablesWithSizes` query. (#15713) Signed-off-by: Arthur Schreiber Co-authored-by: Deepthi Sigireddi --- go/mysql/flavor_mysql.go | 28 ++------- go/vt/vttablet/endtoend/misc_test.go | 87 ++++++++++++++++++++++++---- 2 files changed, 81 insertions(+), 34 deletions(-) diff --git a/go/mysql/flavor_mysql.go b/go/mysql/flavor_mysql.go index c0a06540368..f413c8ef1fb 100644 --- a/go/mysql/flavor_mysql.go +++ b/go/mysql/flavor_mysql.go @@ -329,31 +329,15 @@ GROUP BY t.table_name, t.table_type, t.create_time, t.table_comment` // TablesWithSize80 is a query to select table along with size for mysql 8.0 // -// We join with a subquery that materializes the data from `information_schema.innodb_sys_tablespaces` -// early for performance reasons. This effectively causes only a single read of `information_schema.innodb_tablespaces` -// per query. // Note the following: -// - We use UNION ALL to deal differently with partitioned tables vs. non-partitioned tables. -// Originally, the query handled both, but that introduced "WHERE ... OR" conditions that led to poor query -// optimization. By separating to UNION ALL we remove all "OR" conditions. +// - We use a single query to fetch both partitioned and non-partitioned tables. This is because +// accessing `information_schema.innodb_tablespaces` is expensive on servers with many tablespaces, +// and every query that loads the table needs to perform full table scans on it. Doing a single +// table scan is more efficient than doing more than one. // - We utilize `INFORMATION_SCHEMA`.`TABLES`.`CREATE_OPTIONS` column to do early pruning before the JOIN. // - `TABLES`.`TABLE_NAME` has `utf8mb4_0900_ai_ci` collation. `INNODB_TABLESPACES`.`NAME` has `utf8mb3_general_ci`. // We normalize the collation to get better query performance (we force the casting at the time of our choosing) -// - `create_options` is NULL for views, and therefore we need an additional UNION ALL to include views const TablesWithSize80 = `SELECT t.table_name, - t.table_type, - UNIX_TIMESTAMP(t.create_time), - t.table_comment, - i.file_size, - i.allocated_size - FROM information_schema.tables t - LEFT JOIN information_schema.innodb_tablespaces i - ON i.name = CONCAT(t.table_schema, '/', t.table_name) COLLATE utf8mb3_general_ci - WHERE - t.table_schema = database() AND not t.create_options <=> 'partitioned' -UNION ALL - SELECT - t.table_name, t.table_type, UNIX_TIMESTAMP(t.create_time), t.table_comment, @@ -361,9 +345,9 @@ UNION ALL SUM(i.allocated_size) FROM information_schema.tables t LEFT JOIN information_schema.innodb_tablespaces i - ON i.name LIKE (CONCAT(t.table_schema, '/', t.table_name, '#p#%') COLLATE utf8mb3_general_ci ) + ON i.name LIKE CONCAT(t.table_schema, '/', t.table_name, IF(t.create_options <=> 'partitioned', '#p#%', '')) COLLATE utf8mb3_general_ci WHERE - t.table_schema = database() AND t.create_options <=> 'partitioned' + t.table_schema = database() GROUP BY t.table_schema, t.table_name, t.table_type, t.create_time, t.table_comment ` diff --git a/go/vt/vttablet/endtoend/misc_test.go b/go/vt/vttablet/endtoend/misc_test.go index 568036f672e..c054179c20a 100644 --- a/go/vt/vttablet/endtoend/misc_test.go +++ b/go/vt/vttablet/endtoend/misc_test.go @@ -906,28 +906,91 @@ func TestShowTablesWithSizes(t *testing.T) { _, err := conn.ExecuteFetch(query, 1, false) require.NoError(t, err) } - expectTables := map[string]([]string){ // TABLE_TYPE, TABLE_COMMENT - "show_tables_with_sizes_t1": {"BASE TABLE", ""}, - "show_tables_with_sizes_v1": {"VIEW", "VIEW"}, - "show_tables_with_sizes_employees": {"BASE TABLE", ""}, + + expectedTables := []string{ + "show_tables_with_sizes_t1", + "show_tables_with_sizes_v1", + "show_tables_with_sizes_employees", } + actualTables := []string{} rs, err := conn.ExecuteFetch(conn.BaseShowTablesWithSizes(), -1, false) require.NoError(t, err) require.NotEmpty(t, rs.Rows) - assert.GreaterOrEqual(t, len(rs.Rows), len(expectTables)) - matchedTables := map[string]bool{} + assert.GreaterOrEqual(t, len(rs.Rows), len(expectedTables)) + for _, row := range rs.Rows { + assert.Equal(t, 6, len(row)) + tableName := row[0].ToString() - vals, ok := expectTables[tableName] - if ok { - assert.Equal(t, vals[0], row[1].ToString()) // TABLE_TYPE - assert.Equal(t, vals[1], row[3].ToString()) // TABLE_COMMENT - matchedTables[tableName] = true + if tableName == "show_tables_with_sizes_t1" { + // TABLE_TYPE + assert.Equal(t, "BASE TABLE", row[1].ToString()) + + assert.True(t, row[2].IsIntegral()) + createTime, err := row[2].ToCastInt64() + assert.NoError(t, err) + assert.Greater(t, createTime, int64(0)) + + // TABLE_COMMENT + assert.Equal(t, "", row[3].ToString()) + + assert.True(t, row[4].IsDecimal()) + fileSize, err := row[4].ToCastInt64() + assert.NoError(t, err) + assert.Greater(t, fileSize, int64(0)) + + assert.True(t, row[4].IsDecimal()) + allocatedSize, err := row[5].ToCastInt64() + assert.NoError(t, err) + assert.Greater(t, allocatedSize, int64(0)) + + actualTables = append(actualTables, tableName) + } else if tableName == "show_tables_with_sizes_v1" { + // TABLE_TYPE + assert.Equal(t, "VIEW", row[1].ToString()) + + assert.True(t, row[2].IsIntegral()) + createTime, err := row[2].ToCastInt64() + assert.NoError(t, err) + assert.Greater(t, createTime, int64(0)) + + // TABLE_COMMENT + assert.Equal(t, "VIEW", row[3].ToString()) + + assert.True(t, row[4].IsNull()) + assert.True(t, row[5].IsNull()) + + actualTables = append(actualTables, tableName) + } else if tableName == "show_tables_with_sizes_employees" { + // TABLE_TYPE + assert.Equal(t, "BASE TABLE", row[1].ToString()) + + assert.True(t, row[2].IsIntegral()) + createTime, err := row[2].ToCastInt64() + assert.NoError(t, err) + assert.Greater(t, createTime, int64(0)) + + // TABLE_COMMENT + assert.Equal(t, "", row[3].ToString()) + + assert.True(t, row[4].IsDecimal()) + fileSize, err := row[4].ToCastInt64() + assert.NoError(t, err) + assert.Greater(t, fileSize, int64(0)) + + assert.True(t, row[5].IsDecimal()) + allocatedSize, err := row[5].ToCastInt64() + assert.NoError(t, err) + assert.Greater(t, allocatedSize, int64(0)) + + actualTables = append(actualTables, tableName) } } - assert.Equalf(t, len(expectTables), len(matchedTables), "%v", matchedTables) + + assert.Equal(t, len(expectedTables), len(actualTables)) + assert.ElementsMatch(t, expectedTables, actualTables) } // TestTuple tests that bind variables having tuple values work with vttablet. From 38960d069ecb89ca9bfb564eab13b5abb8a088de Mon Sep 17 00:00:00 2001 From: Arthur Schreiber Date: Thu, 25 Apr 2024 16:34:23 +0000 Subject: [PATCH 2/3] Attempt fixing failing test case. Signed-off-by: Arthur Schreiber --- go/test/endtoend/vtgate/queries/normalize/normalize_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/test/endtoend/vtgate/queries/normalize/normalize_test.go b/go/test/endtoend/vtgate/queries/normalize/normalize_test.go index 735a26fc00c..53d9eb5de7d 100644 --- a/go/test/endtoend/vtgate/queries/normalize/normalize_test.go +++ b/go/test/endtoend/vtgate/queries/normalize/normalize_test.go @@ -40,7 +40,7 @@ func TestNormalizeAllFields(t *testing.T) { defer conn.Close() insertQuery := `insert into t1 values (1, "chars", "variable chars", x'73757265', 0x676F, 0.33, 9.99, 1, "1976-06-08", "small", "b", "{\"key\":\"value\"}", point(1,5), b'011', 0b0101)` - normalizedInsertQuery := `insert into t1 values (:vtg1 /* INT64 */, :vtg2 /* VARCHAR */, :vtg3 /* VARCHAR */, :vtg4 /* HEXVAL */, :vtg5 /* HEXNUM */, :vtg6 /* DECIMAL */, :vtg7 /* DECIMAL */, :vtg8 /* INT64 */, :vtg9 /* VARCHAR */, :vtg10 /* VARCHAR */, :vtg11 /* VARCHAR */, :vtg12 /* VARCHAR */, point(:vtg13 /* INT64 */, :vtg14 /* INT64 */), :vtg15 /* BITNUM */, :vtg16 /* BITNUM */)` + normalizedInsertQuery := `insert into t1 values (:vtg1 /* INT64 */, :vtg2 /* VARCHAR */, :vtg3 /* VARCHAR */, :vtg4 /* HEXVAL */, :vtg5 /* HEXNUM */, :vtg6 /* DECIMAL(3, 2) */, :vtg7 /* DECIMAL(3, 2) */, :vtg8 /* INT64 */, :vtg9 /* VARCHAR */, :vtg10 /* VARCHAR */, :vtg11 /* VARCHAR */, :vtg12 /* VARCHAR */, point(:vtg13 /* INT64 */, :vtg14 /* INT64 */), :vtg15 /* BITNUM */, :vtg16 /* BITNUM */)` vtgateVersion, err := cluster.GetMajorVersion("vtgate") require.NoError(t, err) if vtgateVersion < 19 { From a294f60ea7054352a1a8a3fe48850e70c3937c7c Mon Sep 17 00:00:00 2001 From: Arthur Schreiber Date: Thu, 25 Apr 2024 16:59:55 +0000 Subject: [PATCH 3/3] Maybe try this. Signed-off-by: Arthur Schreiber --- go/test/endtoend/vtgate/queries/normalize/normalize_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/go/test/endtoend/vtgate/queries/normalize/normalize_test.go b/go/test/endtoend/vtgate/queries/normalize/normalize_test.go index 53d9eb5de7d..a3637ef5230 100644 --- a/go/test/endtoend/vtgate/queries/normalize/normalize_test.go +++ b/go/test/endtoend/vtgate/queries/normalize/normalize_test.go @@ -40,9 +40,13 @@ func TestNormalizeAllFields(t *testing.T) { defer conn.Close() insertQuery := `insert into t1 values (1, "chars", "variable chars", x'73757265', 0x676F, 0.33, 9.99, 1, "1976-06-08", "small", "b", "{\"key\":\"value\"}", point(1,5), b'011', 0b0101)` - normalizedInsertQuery := `insert into t1 values (:vtg1 /* INT64 */, :vtg2 /* VARCHAR */, :vtg3 /* VARCHAR */, :vtg4 /* HEXVAL */, :vtg5 /* HEXNUM */, :vtg6 /* DECIMAL(3, 2) */, :vtg7 /* DECIMAL(3, 2) */, :vtg8 /* INT64 */, :vtg9 /* VARCHAR */, :vtg10 /* VARCHAR */, :vtg11 /* VARCHAR */, :vtg12 /* VARCHAR */, point(:vtg13 /* INT64 */, :vtg14 /* INT64 */), :vtg15 /* BITNUM */, :vtg16 /* BITNUM */)` + + normalizedInsertQuery := `insert into t1 values (:vtg1 /* INT64 */, :vtg2 /* VARCHAR */, :vtg3 /* VARCHAR */, :vtg4 /* HEXVAL */, :vtg5 /* HEXNUM */, :vtg6 /* DECIMAL(3,2) */, :vtg7 /* DECIMAL(3,2) */, :vtg8 /* INT64 */, :vtg9 /* VARCHAR */, :vtg10 /* VARCHAR */, :vtg11 /* VARCHAR */, :vtg12 /* VARCHAR */, point(:vtg13 /* INT64 */, :vtg14 /* INT64 */), :vtg15 /* BITNUM */, :vtg16 /* BITNUM */)` vtgateVersion, err := cluster.GetMajorVersion("vtgate") require.NoError(t, err) + if vtgateVersion < 20 { + normalizedInsertQuery = `insert into t1 values (:vtg1 /* INT64 */, :vtg2 /* VARCHAR */, :vtg3 /* VARCHAR */, :vtg4 /* HEXVAL */, :vtg5 /* HEXNUM */, :vtg6 /* DECIMAL */, :vtg7 /* DECIMAL */, :vtg8 /* INT64 */, :vtg9 /* VARCHAR */, :vtg10 /* VARCHAR */, :vtg11 /* VARCHAR */, :vtg12 /* VARCHAR */, point(:vtg13 /* INT64 */, :vtg14 /* INT64 */), :vtg15 /* BITNUM */, :vtg16 /* BITNUM */)` + } if vtgateVersion < 19 { normalizedInsertQuery = `insert into t1 values (:vtg1 /* INT64 */, :vtg2 /* VARCHAR */, :vtg3 /* VARCHAR */, :vtg4 /* HEXVAL */, :vtg5 /* HEXNUM */, :vtg6 /* DECIMAL */, :vtg7 /* DECIMAL */, :vtg8 /* INT64 */, :vtg9 /* VARCHAR */, :vtg10 /* VARCHAR */, :vtg11 /* VARCHAR */, :vtg12 /* VARCHAR */, point(:vtg13 /* INT64 */, :vtg14 /* INT64 */), :vtg15 /* HEXNUM */, :vtg16 /* HEXNUM */)` }