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

ddl: check allocator's existence before allocate/rebase #18932

Merged
merged 4 commits into from
Aug 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions ddl/db_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2328,3 +2328,17 @@ func (s *testIntegrationSuite4) TestAlterIndexVisibility(c *C) {
tk.MustExec("alter table t3 alter index idx invisible")
tk.MustQuery(query).Check(testkit.Rows("idx NO"))
}

func (s *testIntegrationSuite7) TestAutoIncrementAllocator(c *C) {
tk := testkit.NewTestKit(c, s.store)
defer config.RestoreFunc()()
config.UpdateGlobal(func(conf *config.Config) {
conf.AlterPrimaryKey = false
})
tk.MustExec("drop database if exists test_create_table_option_auto_inc;")
tk.MustExec("create database test_create_table_option_auto_inc;")
tk.MustExec("use test_create_table_option_auto_inc;")

tk.MustExec("create table t (a bigint primary key) auto_increment = 10;")
tk.MustExec("alter table t auto_increment = 10;")
djshow832 marked this conversation as resolved.
Show resolved Hide resolved
}
2 changes: 1 addition & 1 deletion ddl/db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,10 @@ func testAddIndexWithPK(tk *testkit.TestKit, s *testSerialDBSuite, c *C) {
func (s *testSerialDBSuite) TestAddIndexWithPK(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use " + s.schemaName)
defer config.RestoreFunc()()
config.UpdateGlobal(func(conf *config.Config) {
conf.AlterPrimaryKey = false
})
defer config.RestoreFunc()()

testAddIndexWithPK(tk, s, c)
tk.MustExec("set @@tidb_enable_clustered_index = 1;")
Expand Down
45 changes: 19 additions & 26 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1688,14 +1688,16 @@ func (d *ddl) CreateTableWithInfo(
if tbInfo.AutoIncID > 1 {
// Default tableAutoIncID base is 0.
// If the first ID is expected to greater than 1, we need to do rebase.
if err = d.handleAutoIncID(tbInfo, schema.ID, autoid.RowIDAllocType); err != nil {
newEnd := tbInfo.AutoIncID - 1
if err = d.handleAutoIncID(tbInfo, schema.ID, newEnd, autoid.RowIDAllocType); err != nil {
return errors.Trace(err)
}
}
if tbInfo.AutoRandID > 1 {
// Default tableAutoRandID base is 0.
// If the first ID is expected to greater than 1, we need to do rebase.
err = d.handleAutoIncID(tbInfo, schema.ID, autoid.AutoRandomType)
newEnd := tbInfo.AutoRandID - 1
err = d.handleAutoIncID(tbInfo, schema.ID, newEnd, autoid.AutoRandomType)
}
}

Expand Down Expand Up @@ -1985,22 +1987,11 @@ func checkCharsetAndCollation(cs string, co string) error {

// handleAutoIncID handles auto_increment option in DDL. It creates a ID counter for the table and initiates the counter to a proper value.
// For example if the option sets auto_increment to 10. The counter will be set to 9. So the next allocated ID will be 10.
func (d *ddl) handleAutoIncID(tbInfo *model.TableInfo, schemaID int64, tp autoid.AllocatorType) error {
func (d *ddl) handleAutoIncID(tbInfo *model.TableInfo, schemaID int64, newEnd int64, tp autoid.AllocatorType) error {
allocs := autoid.NewAllocatorsFromTblInfo(d.store, schemaID, tbInfo)
tbInfo.State = model.StatePublic
tb, err := table.TableFromMeta(allocs, tbInfo)
if err != nil {
return errors.Trace(err)
}
// The operation of the minus 1 to make sure that the current value doesn't be used,
// the next Alloc operation will get this value.
// Its behavior is consistent with MySQL.
if tp == autoid.RowIDAllocType {
if err = tb.RebaseAutoID(nil, tbInfo.AutoIncID-1, false, tp); err != nil {
return errors.Trace(err)
}
} else {
if err = tb.RebaseAutoID(nil, tbInfo.AutoRandID-1, false, tp); err != nil {
if alloc := allocs.Get(tp); alloc != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

So the manual rebase will be just ignored when alloc==nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. This is consistent with MySQL.

err := alloc.Rebase(tbInfo.ID, newEnd, false)
if err != nil {
return errors.Trace(err)
}
}
Expand Down Expand Up @@ -2381,16 +2372,18 @@ func (d *ddl) RebaseAutoID(ctx sessionctx.Context, ident ast.Ident, newBase int6
actionType = model.ActionRebaseAutoID
}

autoID, err := t.Allocators(ctx).Get(tp).NextGlobalAutoID(t.Meta().ID)
if err != nil {
return errors.Trace(err)
if alloc := t.Allocators(ctx).Get(tp); alloc != nil {
autoID, err := alloc.NextGlobalAutoID(t.Meta().ID)
if err != nil {
return errors.Trace(err)
}
// If newBase < autoID, we need to do a rebase before returning.
// Assume there are 2 TiDB servers: TiDB-A with allocator range of 0 ~ 30000; TiDB-B with allocator range of 30001 ~ 60000.
// If the user sends SQL `alter table t1 auto_increment = 100` to TiDB-B,
// and TiDB-B finds 100 < 30001 but returns without any handling,
// then TiDB-A may still allocate 99 for auto_increment column. This doesn't make sense for the user.
newBase = mathutil.MaxInt64(newBase, autoID)
}
// If newBase < autoID, we need to do a rebase before returning.
// Assume there are 2 TiDB servers: TiDB-A with allocator range of 0 ~ 30000; TiDB-B with allocator range of 30001 ~ 60000.
// If the user sends SQL `alter table t1 auto_increment = 100` to TiDB-B,
// and TiDB-B finds 100 < 30001 but returns without any handling,
// then TiDB-A may still allocate 99 for auto_increment column. This doesn't make sense for the user.
newBase = mathutil.MaxInt64(newBase, autoID)
job := &model.Job{
SchemaID: schema.ID,
TableID: t.Meta().ID,
Expand Down
13 changes: 7 additions & 6 deletions ddl/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,12 +529,13 @@ func onRebaseAutoID(store kv.Storage, t *meta.Meta, job *model.Job, tp autoid.Al
job.State = model.JobStateCancelled
return ver, errors.Trace(err)
}
// The operation of the minus 1 to make sure that the current value doesn't be used,
// the next Alloc operation will get this value.
// Its behavior is consistent with MySQL.
err = tbl.RebaseAutoID(nil, newBase-1, false, tp)
if err != nil {
return ver, errors.Trace(err)
if alloc := tbl.Allocators(nil).Get(tp); alloc != nil {
// The next value to allocate is `newBase`.
newEnd := newBase - 1
err = alloc.Rebase(tblInfo.ID, newEnd, false)
if err != nil {
return ver, errors.Trace(err)
}
}
ver, err = updateVersionAndTableInfo(t, job, tblInfo, true)
if err != nil {
Expand Down