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.

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

domain: avoid check schema changed when change is tiflash replica status #13034

Merged
merged 10 commits into from
Nov 5, 2019
32 changes: 32 additions & 0 deletions ddl/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3331,6 +3331,38 @@ func (s *testDBSuite2) TestWriteLocal(c *C) {
tk2.MustExec("unlock tables")
}

func (s *testDBSuite2) TestSkipSchemaChecker(c *C) {
s.tk = testkit.NewTestKit(c, s.store)
tk := s.tk
tk.MustExec("use test")
tk.MustExec("drop table if exists t1")
defer tk.MustExec("drop table if exists t1")
tk.MustExec("create table t1 (a int)")
tk2 := testkit.NewTestKit(c, s.store)
tk2.MustExec("use test")

// Test skip schema checker for ActionSetTiFlashReplica.
tk.MustExec("begin")
tk.MustExec("insert into t1 set a=1;")
tk2.MustExec("alter table t1 set tiflash replica 2 location labels 'a','b';")
tk.MustExec("commit")

// Test skip schema checker for ActionUpdateTiFlashReplicaStatus.
tk.MustExec("begin")
tk.MustExec("insert into t1 set a=1;")
tb := testGetTableByName(c, s.tk.Se, "test", "t1")
err := domain.GetDomain(s.tk.Se).DDL().UpdateTableReplicaInfo(s.tk.Se, tb.Meta().ID, true)
c.Assert(err, IsNil)
tk.MustExec("commit")

// Test can't skip schema checker.
tk.MustExec("begin")
tk.MustExec("insert into t1 set a=1;")
tk2.MustExec("alter table t1 add column b int;")
_, err = tk.Exec("commit")
c.Assert(terror.ErrorEqual(domain.ErrInfoSchemaChanged, err), IsTrue)
}

func (s *testDBSuite2) TestLockTables(c *C) {
if israce.RaceEnabled {
c.Skip("skip race test")
Expand Down
11 changes: 11 additions & 0 deletions domain/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,12 +254,23 @@ func (do *Domain) tryLoadSchemaDiffs(m *meta.Meta, usedVersion, newVersion int64
if err != nil {
return false, nil, err
}
if canSkipSchemaCheckerDDL(diff.Type) {
continue
}
tblIDs = append(tblIDs, ids...)
}
builder.Build()
return true, tblIDs, nil
}

func canSkipSchemaCheckerDDL(tp model.ActionType) bool {
switch tp {
case model.ActionUpdateTiFlashReplicaStatus, model.ActionSetTiFlashReplica:
Copy link
Contributor

@AilinKid AilinKid Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By now two options here, how about use if A||B? maybe fast.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

em... I prefer switch😂

return true
}
return false
}

// InfoSchema gets information schema from domain.
func (do *Domain) InfoSchema() infoschema.InfoSchema {
return do.infoHandle.Get()
Expand Down