Skip to content

Commit

Permalink
VTTablet schema reload: include stats only on periodic reload, defaul…
Browse files Browse the repository at this point in the history
…t to "base table" query for the "with sizes" query for 5.7 only (vitessio#17855)

Signed-off-by: Rohit Nayak <rohit@planetscale.com>
  • Loading branch information
rohit-nayak-ps authored Feb 25, 2025
1 parent 965253a commit cfd4d29
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 90 deletions.
7 changes: 6 additions & 1 deletion go/mysql/flavor_mysql.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,12 @@ const InnoDBTableSizes = `

// baseShowTablesWithSizes is part of the Flavor interface.
func (mysqlFlavor57) baseShowTablesWithSizes() string {
return TablesWithSize57
// For 5.7, we use the base query instead of the query with sizes. Flavor57 should only be used
// for unmanaged tables during import. We don't need to know the size of the tables in that case since
// the size information is mainly used by Online DDL.
// The TablesWithSize57 query can be very non-performant on some external databases, for example on Aurora with
// a large number of tables, it can time out often.
return BaseShowTables
}

// baseShowInnodbTableSizes is part of the Flavor interface.
Expand Down
8 changes: 4 additions & 4 deletions go/vt/vttablet/endtoend/misc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1042,8 +1042,8 @@ func TestEngineReload(t *testing.T) {
assert.Zero(t, tbl.FileSize)
assert.Zero(t, tbl.AllocatedSize)
default:
assert.NotZero(t, tbl.FileSize)
assert.NotZero(t, tbl.AllocatedSize)
assert.Zero(t, tbl.FileSize)
assert.Zero(t, tbl.AllocatedSize)
}
})
}
Expand Down Expand Up @@ -1082,8 +1082,8 @@ func TestEngineReload(t *testing.T) {
assert.Zero(t, tbl.FileSize)
assert.Zero(t, tbl.AllocatedSize)
default:
assert.NotZero(t, tbl.FileSize)
assert.NotZero(t, tbl.AllocatedSize)
assert.Zero(t, tbl.FileSize)
assert.Zero(t, tbl.AllocatedSize)
}
})
}
Expand Down
7 changes: 4 additions & 3 deletions go/vt/vttablet/tabletserver/schema/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,8 @@ func (se *Engine) Open() error {
}

se.ticks.Start(func() {
if err := se.Reload(ctx); err != nil {
// update stats on periodic reloads
if err := se.reload(ctx, true); err != nil {
log.Errorf("periodic schema reload failed: %v", err)
}
})
Expand Down Expand Up @@ -369,7 +370,7 @@ func (se *Engine) Reload(ctx context.Context) error {
// It maintains the position at which the schema was reloaded and if the same position is provided
// (say by multiple vstreams) it returns the cached schema. In case of a newer or empty pos it always reloads the schema
func (se *Engine) ReloadAt(ctx context.Context, pos replication.Position) error {
return se.ReloadAtEx(ctx, pos, true)
return se.ReloadAtEx(ctx, pos, false)
}

// ReloadAtEx reloads the schema info from the db.
Expand Down Expand Up @@ -800,7 +801,7 @@ func (se *Engine) GetTableForPos(ctx context.Context, tableName sqlparser.Identi
// This also allows us to perform a just-in-time initialization of the cache if
// a vstreamer is the first one to access it.
if se.conns != nil { // Test Engines (NewEngineForTests()) don't have a conns pool
if err := se.reload(ctx, true); err != nil {
if err := se.reload(ctx, false); err != nil {
return nil, err
}
if st, ok := se.tables[tableNameStr]; ok {
Expand Down
105 changes: 23 additions & 82 deletions go/vt/vttablet/tabletserver/schema/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import (
"vitess.io/vitess/go/stats"
"vitess.io/vitess/go/test/utils"
"vitess.io/vitess/go/vt/dbconfigs"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/vtenv"
"vitess.io/vitess/go/vt/vttablet/tabletserver/connpool"
Expand All @@ -51,6 +50,7 @@ import (

binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
querypb "vitess.io/vitess/go/vt/proto/query"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
)

const baseShowTablesWithSizesPattern = `SELECT t\.table_name.*SUM\(i\.file_size\).*`
Expand Down Expand Up @@ -124,8 +124,8 @@ func TestOpenAndReloadLegacy(t *testing.T) {
// Modify test_table_03
// Add test_table_04
// Drop msg
db.AddQueryPattern(baseShowTablesWithSizesPattern, &sqltypes.Result{
Fields: mysql.BaseShowTablesWithSizesFields,
db.AddQuery(mysql.BaseShowTables, &sqltypes.Result{
Fields: mysql.BaseShowTablesFields,
Rows: [][]sqltypes.Value{
mysql.BaseShowTablesWithSizesRow("test_table_01", false, ""),
mysql.BaseShowTablesWithSizesRow("test_table_02", false, ""),
Expand All @@ -134,16 +134,14 @@ func TestOpenAndReloadLegacy(t *testing.T) {
sqltypes.MakeTrusted(sqltypes.VarChar, []byte("BASE TABLE")), // table_type
sqltypes.MakeTrusted(sqltypes.Int64, []byte("1427325877")), // unix_timestamp(t.create_time)
sqltypes.MakeTrusted(sqltypes.VarChar, []byte("")), // table_comment
sqltypes.MakeTrusted(sqltypes.Int64, []byte("128")), // file_size
sqltypes.MakeTrusted(sqltypes.Int64, []byte("256")), // allocated_size
},
// test_table_04 will in spite of older timestamp because it doesn't exist yet.
mysql.BaseShowTablesWithSizesRow("test_table_04", false, ""),
mysql.BaseShowTablesWithSizesRow("seq", false, "vitess_sequence"),
mysql.BaseShowTablesRow("test_table_04", false, ""),
mysql.BaseShowTablesRow("seq", false, "vitess_sequence"),
},
})

db.AddRejectedQuery(mysql.BaseShowTables, fmt.Errorf("Reloading schema engine should query tables with size information"))
db.AddRejectedQuery(mysql.TablesWithSize57, fmt.Errorf("Reloading schema engine should query tables with size information"))

db.MockQueriesForTable("test_table_03", &sqltypes.Result{
Fields: []*querypb.Field{{
Expand Down Expand Up @@ -200,15 +198,6 @@ func TestOpenAndReloadLegacy(t *testing.T) {

assert.EqualValues(t, secondReadRowsValue, se.innoDbReadRowsCounter.Get())

want["seq"].FileSize = 100
want["seq"].AllocatedSize = 150

want["test_table_01"].FileSize = 100
want["test_table_01"].AllocatedSize = 150

want["test_table_02"].FileSize = 100
want["test_table_02"].AllocatedSize = 150

want["test_table_03"] = &Table{
Name: sqlparser.NewIdentifierCS("test_table_03"),
Fields: []*querypb.Field{{
Expand All @@ -221,21 +210,17 @@ func TestOpenAndReloadLegacy(t *testing.T) {
Name: "val",
Type: sqltypes.Int32,
}},
PKColumns: []int{0, 1},
CreateTime: 1427325877,
FileSize: 128,
AllocatedSize: 256,
PKColumns: []int{0, 1},
CreateTime: 1427325877,
}
want["test_table_04"] = &Table{
Name: sqlparser.NewIdentifierCS("test_table_04"),
Fields: []*querypb.Field{{
Name: "pk",
Type: sqltypes.Int32,
}},
PKColumns: []int{0},
CreateTime: 1427325875,
FileSize: 100,
AllocatedSize: 150,
PKColumns: []int{0},
CreateTime: 1427325875,
}
delete(want, "msg")
assert.Equal(t, want, se.GetSchema())
Expand Down Expand Up @@ -372,8 +357,6 @@ func TestOpenAndReload(t *testing.T) {
sqltypes.MakeTrusted(sqltypes.VarChar, []byte("BASE TABLE")), // table_type
sqltypes.MakeTrusted(sqltypes.Int64, []byte("1427325877")), // unix_timestamp(t.create_time)
sqltypes.MakeTrusted(sqltypes.VarChar, []byte("")), // table_comment
sqltypes.MakeTrusted(sqltypes.Int64, []byte("128")), // file_size
sqltypes.MakeTrusted(sqltypes.Int64, []byte("256")), // allocated_size
},
mysql.BaseShowTablesRow("test_table_04", false, ""),
mysql.BaseShowTablesRow("seq", false, "vitess_sequence"),
Expand All @@ -391,8 +374,6 @@ func TestOpenAndReload(t *testing.T) {
mysql.BaseInnoDBTableSizesRow("fakesqldb", "test_table_02"),
{
sqltypes.MakeTrusted(sqltypes.VarChar, []byte("fakesqldb/test_table_03")), // table_name
sqltypes.MakeTrusted(sqltypes.Int64, []byte("128")), // file_size
sqltypes.MakeTrusted(sqltypes.Int64, []byte("256")), // allocated_size
},
mysql.BaseInnoDBTableSizesRow("fakesqldb", "test_table_04"),
mysql.BaseInnoDBTableSizesRow("fakesqldb", "seq"),
Expand Down Expand Up @@ -455,15 +436,6 @@ func TestOpenAndReload(t *testing.T) {

assert.EqualValues(t, secondReadRowsValue, se.innoDbReadRowsCounter.Get())

want["seq"].FileSize = 100
want["seq"].AllocatedSize = 150

want["test_table_01"].FileSize = 100
want["test_table_01"].AllocatedSize = 150

want["test_table_02"].FileSize = 100
want["test_table_02"].AllocatedSize = 150

want["test_table_03"] = &Table{
Name: sqlparser.NewIdentifierCS("test_table_03"),
Fields: []*querypb.Field{{
Expand All @@ -476,21 +448,17 @@ func TestOpenAndReload(t *testing.T) {
Name: "val",
Type: sqltypes.Int32,
}},
PKColumns: []int{0, 1},
CreateTime: 1427325877,
FileSize: 128,
AllocatedSize: 256,
PKColumns: []int{0, 1},
CreateTime: 1427325877,
}
want["test_table_04"] = &Table{
Name: sqlparser.NewIdentifierCS("test_table_04"),
Fields: []*querypb.Field{{
Name: "pk",
Type: sqltypes.Int32,
}},
PKColumns: []int{0},
CreateTime: 1427325875,
FileSize: 100,
AllocatedSize: 150,
PKColumns: []int{0},
CreateTime: 1427325875,
}
delete(want, "msg")
assert.Equal(t, want, se.GetSchema())
Expand Down Expand Up @@ -646,31 +614,14 @@ func TestReloadWithSwappedTables(t *testing.T) {
err := se.Reload(context.Background())
require.NoError(t, err)

want["msg"].FileSize = 100
want["msg"].AllocatedSize = 150

want["seq"].FileSize = 100
want["seq"].AllocatedSize = 150

want["test_table_01"].FileSize = 100
want["test_table_01"].AllocatedSize = 150

want["test_table_02"].FileSize = 100
want["test_table_02"].AllocatedSize = 150

want["test_table_03"].FileSize = 100
want["test_table_03"].AllocatedSize = 150

want["test_table_04"] = &Table{
Name: sqlparser.NewIdentifierCS("test_table_04"),
Fields: []*querypb.Field{{
Name: "mypk",
Type: sqltypes.Int32,
}},
PKColumns: []int{0},
CreateTime: 1427325877,
FileSize: 128,
AllocatedSize: 256,
PKColumns: []int{0},
CreateTime: 1427325877,
}

mustMatch(t, want, se.GetSchema())
Expand All @@ -689,8 +640,6 @@ func TestReloadWithSwappedTables(t *testing.T) {
mysql.BaseInnoDBTableSizesRow("fakesqldb", "test_table_02"),
{
sqltypes.MakeTrusted(sqltypes.VarChar, []byte("fakesqldb/test_table_03")), // table_name
sqltypes.MakeTrusted(sqltypes.Int64, []byte("128")), // file_size
sqltypes.MakeTrusted(sqltypes.Int64, []byte("256")), // allocated_size
},
mysql.BaseInnoDBTableSizesRow("fakesqldb", "test_table_04"),
mysql.BaseInnoDBTableSizesRow("fakesqldb", "seq"),
Expand All @@ -709,8 +658,6 @@ func TestReloadWithSwappedTables(t *testing.T) {
sqltypes.MakeTrusted(sqltypes.VarChar, []byte("BASE TABLE")),
sqltypes.MakeTrusted(sqltypes.Int64, []byte("1427325877")), // unix_timestamp(create_time)
sqltypes.MakeTrusted(sqltypes.VarChar, []byte("")),
sqltypes.MakeTrusted(sqltypes.Int64, []byte("128")), // file_size
sqltypes.MakeTrusted(sqltypes.Int64, []byte("256")), // allocated_size
},
mysql.BaseShowTablesRow("test_table_04", false, ""),
mysql.BaseShowTablesRow("seq", false, "vitess_sequence"),
Expand Down Expand Up @@ -755,21 +702,17 @@ func TestReloadWithSwappedTables(t *testing.T) {
Name: "mypk",
Type: sqltypes.Int32,
}},
PKColumns: []int{0},
CreateTime: 1427325877,
FileSize: 128,
AllocatedSize: 256,
PKColumns: []int{0},
CreateTime: 1427325877,
}
want["test_table_04"] = &Table{
Name: sqlparser.NewIdentifierCS("test_table_04"),
Fields: []*querypb.Field{{
Name: "pk",
Type: sqltypes.Int32,
}},
PKColumns: []int{0},
CreateTime: 1427325875,
FileSize: 100,
AllocatedSize: 150,
PKColumns: []int{0},
CreateTime: 1427325875,
}
mustMatch(t, want, se.GetSchema())
}
Expand Down Expand Up @@ -1679,7 +1622,7 @@ func TestEngineReload(t *testing.T) {
// that it conforms to the intended/expected behavior in various scenarios.
// This more specifically tests the behavior of the function when the historian is
// disabled or otherwise unable to get a table schema for the given position. When it
// CAN, that is tested indepenently in the historian tests.
// CAN, that is tested independently in the historian tests.
//
// Runs with 5.7 env
func TestGetTableForPosLegacy(t *testing.T) {
Expand Down Expand Up @@ -1720,9 +1663,9 @@ func TestGetTableForPosLegacy(t *testing.T) {
db.AddQuery(fmt.Sprintf(readTableCreateTimes, sidecar.GetIdentifier()),
sqltypes.MakeTestResult(sqltypes.MakeTestFields("table_name|create_time", "varchar|int64")))
db.AddQuery(fmt.Sprintf(detectUdfChange, sidecar.GetIdentifier()), &sqltypes.Result{})
db.AddQueryPattern(baseShowTablesWithSizesPattern,
db.AddQuery(mysql.BaseShowTables,
&sqltypes.Result{
Fields: mysql.BaseShowTablesWithSizesFields,
Fields: mysql.BaseShowTablesFields,
RowsAffected: 0,
InsertID: 0,
Rows: [][]sqltypes.Value{
Expand All @@ -1731,8 +1674,6 @@ func TestGetTableForPosLegacy(t *testing.T) {
sqltypes.MakeTrusted(sqltypes.VarChar, []byte("BASE TABLE")), // table_type
sqltypes.MakeTrusted(sqltypes.Int64, []byte(fmt.Sprintf("%d", time.Now().Unix()-1000))), // unix_timestamp(t.create_time)
sqltypes.MakeTrusted(sqltypes.VarChar, []byte("")), // table_comment
sqltypes.MakeTrusted(sqltypes.Int64, []byte("128")), // file_size
sqltypes.MakeTrusted(sqltypes.Int64, []byte("256")), // allocated_size
},
},
SessionStateChanges: "",
Expand Down

0 comments on commit cfd4d29

Please sign in to comment.