Skip to content

VTTablet schema reload: include stats only on periodic reload for all flavors. Default to "base table" query for the "with sizes" query for 5.7 only #17855

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rohit-nayak-ps merged 5 commits into from
Feb 25, 2025
Merged

VTTablet schema reload: include stats only on periodic reload for all flavors. Default to "base table" query for the "with sizes" query for 5.7 only #17855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8d63419
Use base tables query for with sizes query for 5.7 flavor. In reload …
rohit-nayak-ps Feb 23, 2025
ab01e9e
Fix TestOpenAndReloadLegacy
rohit-nayak-ps Feb 24, 2025
d269373
Fix failing tests
rohit-nayak-ps Feb 24, 2025
22c26e3
Remove debug logs
rohit-nayak-ps Feb 24, 2025
ec98a9d
Update stats on periodic reload
rohit-nayak-ps Feb 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion go/mysql/flavor_mysql.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading