Skip to content
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

Only run sidecardb change detection on serving primary tablets #17051

Merged
merged 2 commits into from
Oct 24, 2024
Merged

Only run sidecardb change detection on serving primary tablets #17051

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
061d520
feat: only run sidecardb change detection on serving primary tablets
GuptaManan100 Oct 23, 2024
2ad880e
test: fix test expectations
GuptaManan100 Oct 23, 2024
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
4 changes: 2 additions & 2 deletions go/test/endtoend/vreplication/sidecardb_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func TestSidecarDB(t *testing.T) {

prs(t, keyspace, shard)
currentPrimary = tablet101
expectedChanges100 += numChanges
expectedChanges101 += numChanges
validateSidecarDBTables(t, tablet100, sidecarDBTables)
validateSidecarDBTables(t, tablet101, sidecarDBTables)
require.Equal(t, expectedChanges100, getNumExecutedDDLQueries(t, tablet100Port))
Expand All @@ -100,7 +100,7 @@ func TestSidecarDB(t *testing.T) {

t.Run("modify schema, prs, and self heal on new primary", func(t *testing.T) {
numChanges := modifySidecarDBSchema(t, vc, currentPrimary, ddls1)
expectedChanges101 += numChanges
expectedChanges100 += numChanges
prs(t, keyspace, shard)
// nolint
currentPrimary = tablet100
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 @@ -180,14 +180,15 @@ func (se *Engine) syncSidecarDB(ctx context.Context, conn *dbconnpool.DBConnecti
// EnsureConnectionAndDB ensures that we can connect to mysql.
// If tablet type is primary and there is no db, then the database is created.
// This function can be called before opening the Engine.
func (se *Engine) EnsureConnectionAndDB(tabletType topodatapb.TabletType) error {
func (se *Engine) EnsureConnectionAndDB(tabletType topodatapb.TabletType, serving bool) error {
ctx := tabletenv.LocalContext()
// We use AllPrivs since syncSidecarDB() might need to upgrade the schema
conn, err := dbconnpool.NewDBConnection(ctx, se.env.Config().DB.AllPrivsWithDB())
if err == nil {
se.dbCreationFailed = false
// upgrade sidecar db if required, for a tablet with an existing database
if tabletType == topodatapb.TabletType_PRIMARY {
// only run DDL updates when a PRIMARY is transitioning to serving state.
if tabletType == topodatapb.TabletType_PRIMARY && serving {
if err := se.syncSidecarDB(ctx, conn); err != nil {
conn.Close()
return err
Expand All @@ -196,7 +197,7 @@ func (se *Engine) EnsureConnectionAndDB(tabletType topodatapb.TabletType) error
conn.Close()
return nil
}
if tabletType != topodatapb.TabletType_PRIMARY {
if tabletType != topodatapb.TabletType_PRIMARY || !serving {
return err
}
if merr, isSQLErr := err.(*sqlerror.SQLError); !isSQLErr || merr.Num != sqlerror.ERBadDb {
Expand Down
18 changes: 17 additions & 1 deletion 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,6 +42,7 @@ 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 Down Expand Up @@ -95,6 +96,19 @@ func TestOpenAndReload(t *testing.T) {
assert.Equal(t, int64(0), se.tableFileSizeGauge.Counts()["msg"])
assert.Equal(t, int64(0), se.tableAllocatedSizeGauge.Counts()["msg"])

t.Run("EnsureConnectionAndDB", func(t *testing.T) {
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
// Verify that none of the following configurations run any schema change detection queries -
// 1. REPLICA serving
// 2. REPLICA non-serving
// 3. PRIMARY serving
err := se.EnsureConnectionAndDB(topodatapb.TabletType_REPLICA, true)
require.NoError(t, err)
err = se.EnsureConnectionAndDB(topodatapb.TabletType_PRIMARY, false)
require.NoError(t, err)
err = se.EnsureConnectionAndDB(topodatapb.TabletType_REPLICA, false)
require.NoError(t, err)
})

// Advance time some more.
db.AddQuery("select unix_timestamp()", sqltypes.MakeTestResult(sqltypes.MakeTestFields(
"t",
Expand Down Expand Up @@ -626,8 +640,10 @@ func newEngine(reloadTime time.Duration, idleTimeout time.Duration, schemaMaxAge
cfg.OlapReadPool.IdleTimeout = idleTimeout
cfg.TxPool.IdleTimeout = idleTimeout
cfg.SchemaVersionMaxAgeSeconds = schemaMaxAgeSeconds
dbConfigs := newDBConfigs(db)
cfg.DB = dbConfigs
se := NewEngine(tabletenv.NewEnv(vtenv.NewTestEnv(), cfg, "SchemaTest"))
se.InitDBConfig(newDBConfigs(db).DbaWithDB())
se.InitDBConfig(dbConfigs.DbaWithDB())
return se
}

Expand Down
14 changes: 7 additions & 7 deletions go/vt/vttablet/tabletserver/state_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ type stateManager struct {

type (
schemaEngine interface {
EnsureConnectionAndDB(topodatapb.TabletType) error
EnsureConnectionAndDB(topodatapb.TabletType, bool) error
Open() error
MakeNonPrimary()
MakePrimary(bool)
Expand Down Expand Up @@ -447,7 +447,7 @@ func (sm *stateManager) verifyTargetLocked(ctx context.Context, target *querypb.
func (sm *stateManager) servePrimary() error {
sm.watcher.Close()

if err := sm.connect(topodatapb.TabletType_PRIMARY); err != nil {
if err := sm.connect(topodatapb.TabletType_PRIMARY, true); err != nil {
return err
}

Expand Down Expand Up @@ -476,7 +476,7 @@ func (sm *stateManager) unservePrimary() error {

sm.watcher.Close()

if err := sm.connect(topodatapb.TabletType_PRIMARY); err != nil {
if err := sm.connect(topodatapb.TabletType_PRIMARY, false); err != nil {
return err
}

Expand All @@ -500,7 +500,7 @@ func (sm *stateManager) serveNonPrimary(wantTabletType topodatapb.TabletType) er
sm.se.MakeNonPrimary()
sm.hs.MakeNonPrimary()

if err := sm.connect(wantTabletType); err != nil {
if err := sm.connect(wantTabletType, true); err != nil {
return err
}

Expand All @@ -518,7 +518,7 @@ func (sm *stateManager) unserveNonPrimary(wantTabletType topodatapb.TabletType)
sm.se.MakeNonPrimary()
sm.hs.MakeNonPrimary()

if err := sm.connect(wantTabletType); err != nil {
if err := sm.connect(wantTabletType, false); err != nil {
return err
}

Expand All @@ -528,8 +528,8 @@ func (sm *stateManager) unserveNonPrimary(wantTabletType topodatapb.TabletType)
return nil
}

func (sm *stateManager) connect(tabletType topodatapb.TabletType) error {
if err := sm.se.EnsureConnectionAndDB(tabletType); err != nil {
func (sm *stateManager) connect(tabletType topodatapb.TabletType, serving bool) error {
if err := sm.se.EnsureConnectionAndDB(tabletType, serving); err != nil {
return err
}
if err := sm.se.Open(); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttablet/tabletserver/state_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@ type testSchemaEngine struct {
failMySQL bool
}

func (te *testSchemaEngine) EnsureConnectionAndDB(tabletType topodatapb.TabletType) error {
func (te *testSchemaEngine) EnsureConnectionAndDB(topodatapb.TabletType, bool) error {
if te.failMySQL {
te.failMySQL = false
return errors.New("intentional error")
Expand Down
Loading