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

*: Remove the check of initialized auto ID #4971

Merged
merged 3 commits into from
Nov 2, 2017
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
2 changes: 1 addition & 1 deletion ddl/column_change_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func getCurrentTable(d *ddl, schemaID, tableID int64) (table.Table, error) {
if err != nil {
return nil, errors.Trace(err)
}
alloc := autoid.NewAllocator(d.store, schemaID)
alloc := autoid.NewAllocator(d.store, 0, schemaID)
tbl, err := table.TableFromMeta(alloc, tblInfo)
if err != nil {
return nil, errors.Trace(err)
Expand Down
6 changes: 1 addition & 5 deletions ddl/ddl_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -795,11 +795,7 @@ func (d *ddl) CreateTable(ctx context.Context, ident ast.Ident, colDefs []*ast.C
// 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) error {
if tbInfo.OldSchemaID != 0 {
schemaID = tbInfo.OldSchemaID
}
alloc := autoid.NewAllocator(d.store, schemaID)

alloc := autoid.NewAllocator(d.store, tbInfo.OldSchemaID, schemaID)
tbInfo.State = model.StatePublic
tb, err := table.TableFromMeta(alloc, tbInfo)
if err != nil {
Expand Down
5 changes: 1 addition & 4 deletions ddl/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,7 @@ func (d *ddl) splitTableRegion(tableID int64) error {
}

func (d *ddl) getTable(schemaID int64, tblInfo *model.TableInfo) (table.Table, error) {
if tblInfo.OldSchemaID != 0 {
schemaID = tblInfo.OldSchemaID
}
alloc := autoid.NewAllocator(d.store, schemaID)
alloc := autoid.NewAllocator(d.store, tblInfo.OldSchemaID, schemaID)
tbl, err := table.TableFromMeta(alloc, tblInfo)
return tbl, errors.Trace(err)
}
Expand Down
2 changes: 1 addition & 1 deletion ddl/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func testGetTableWithError(d *ddl, schemaID, tableID int64) (table.Table, error)
if tblInfo == nil {
return nil, errors.New("table not found")
}
alloc := autoid.NewAllocator(d.store, schemaID)
alloc := autoid.NewAllocator(d.store, tblInfo.OldSchemaID, schemaID)
tbl, err := table.TableFromMeta(alloc, tblInfo)
if err != nil {
return nil, errors.Trace(err)
Expand Down
2 changes: 1 addition & 1 deletion executor/ddl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func (s *testSuite) TestRenameTable(c *C) {
tk.MustExec("insert rename2.t values ()")
tk.MustExec("rename table rename2.t to rename3.t")
tk.MustExec("insert rename3.t values ()")
tk.MustQuery("select * from rename3.t").Check(testkit.Rows("1", "2", "3"))
tk.MustQuery("select * from rename3.t").Check(testkit.Rows("1", "5001", "10001"))
Copy link
Member

Choose a reason for hiding this comment

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

Why this result changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because of the change at your second question.

tk.MustExec("drop database rename1")
tk.MustExec("drop database rename2")
tk.MustExec("drop database rename3")
Expand Down
12 changes: 3 additions & 9 deletions infoschema/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (b *Builder) ApplyDiff(m *meta.Meta, diff *model.SchemaDiff) ([]int64, erro
// We try to reuse the old allocator, so the cached auto ID can be reused.
var alloc autoid.Allocator
if tableIDIsValid(oldTableID) {
if oldTableID == newTableID {
if oldTableID == newTableID && diff.Type != model.ActionRenameTable {
Copy link
Member

Choose a reason for hiding this comment

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

We can still reuse the allocator for rename table, just add a method SetDBID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But, it needs to add an interface to Allocator.

Copy link
Member

Choose a reason for hiding this comment

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

Alright, skip auto ID when rename table is acceptable.

alloc, _ = b.is.AllocByID(oldTableID)
}
if diff.Type == model.ActionRenameTable {
Expand Down Expand Up @@ -162,10 +162,7 @@ func (b *Builder) applyCreateTable(m *meta.Meta, roDBInfo *model.DBInfo, tableID
}
if alloc == nil {
schemaID := roDBInfo.ID
if tblInfo.OldSchemaID != 0 {
schemaID = tblInfo.OldSchemaID
}
alloc = autoid.NewAllocator(b.handle.store, schemaID)
alloc = autoid.NewAllocator(b.handle.store, tblInfo.OldSchemaID, schemaID)
}
tbl, err := tables.TableFromMeta(alloc, tblInfo)
if err != nil {
Expand Down Expand Up @@ -267,10 +264,7 @@ func (b *Builder) createSchemaTablesForDB(di *model.DBInfo) error {
b.is.schemaMap[di.Name.L] = schTbls
for _, t := range di.Tables {
schemaID := di.ID
if t.OldSchemaID != 0 {
schemaID = t.OldSchemaID
}
alloc := autoid.NewAllocator(b.handle.store, schemaID)
alloc := autoid.NewAllocator(b.handle.store, t.OldSchemaID, schemaID)
var tbl table.Table
tbl, err := tables.TableFromMeta(alloc, t)
if err != nil {
Expand Down
25 changes: 17 additions & 8 deletions meta/autoid/autoid.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ type allocator struct {
base int64
end int64
store kv.Storage
dbID int64
// originalDBID saves original schemaID to keep autoID unchanged
// while renaming a table from one database to another.
originalDBID int64
// dbID is current database's ID.
dbID int64
}

// GetStep is only used by tests
Expand Down Expand Up @@ -97,7 +101,7 @@ func (alloc *allocator) Rebase(tableID, requiredBase int64, allocIDs bool) error
var newBase, newEnd int64
err := kv.RunInNewTxn(alloc.store, true, func(txn kv.Transaction) error {
m := meta.NewMeta(txn)
currentEnd, err1 := m.GetAutoTableID(alloc.dbID, tableID)
currentEnd, err1 := m.GetAutoTableID(alloc.originalDBID, tableID)
if err1 != nil {
return errors.Trace(err1)
}
Expand All @@ -117,7 +121,7 @@ func (alloc *allocator) Rebase(tableID, requiredBase int64, allocIDs bool) error
newBase = requiredBase
newEnd = requiredBase
}
_, err1 = m.GenAutoTableID(alloc.dbID, tableID, newEnd-currentEnd)
_, err1 = m.GenAutoTableID(alloc.originalDBID, alloc.dbID, tableID, newEnd-currentEnd)
return errors.Trace(err1)
})
if err != nil {
Expand All @@ -139,11 +143,11 @@ func (alloc *allocator) Alloc(tableID int64) (int64, error) {
err := kv.RunInNewTxn(alloc.store, true, func(txn kv.Transaction) error {
m := meta.NewMeta(txn)
var err1 error
newBase, err1 = m.GetAutoTableID(alloc.dbID, tableID)
newBase, err1 = m.GetAutoTableID(alloc.originalDBID, tableID)
if err1 != nil {
return errors.Trace(err1)
}
newEnd, err1 = m.GenAutoTableID(alloc.dbID, tableID, step)
newEnd, err1 = m.GenAutoTableID(alloc.originalDBID, alloc.dbID, tableID, step)
if err1 != nil {
return errors.Trace(err1)
}
Expand Down Expand Up @@ -208,10 +212,15 @@ func (alloc *memoryAllocator) Alloc(tableID int64) (int64, error) {
}

// NewAllocator returns a new auto increment id generator on the store.
func NewAllocator(store kv.Storage, dbID int64) Allocator {
func NewAllocator(store kv.Storage, originalDBID, dbID int64) Allocator {
// If original DB ID is zero, it means that the orignial DB ID is equal to the current DB ID.
if originalDBID == 0 {
originalDBID = dbID
}
return &allocator{
store: store,
dbID: dbID,
store: store,
originalDBID: originalDBID,
dbID: dbID,
}
}

Expand Down
14 changes: 7 additions & 7 deletions meta/autoid/autoid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (*testSuite) TestT(c *C) {
})
c.Assert(err, IsNil)

alloc := autoid.NewAllocator(store, 1)
alloc := autoid.NewAllocator(store, 0, 1)
c.Assert(alloc, NotNil)

id, err := alloc.Alloc(1)
Expand Down Expand Up @@ -91,25 +91,25 @@ func (*testSuite) TestT(c *C) {
c.Assert(err, IsNil)
c.Assert(id, Equals, int64(3011))

alloc = autoid.NewAllocator(store, 1)
alloc = autoid.NewAllocator(store, 0, 1)
c.Assert(alloc, NotNil)
id, err = alloc.Alloc(1)
c.Assert(err, IsNil)
c.Assert(id, Equals, int64(autoid.GetStep()+1))

alloc = autoid.NewAllocator(store, 1)
alloc = autoid.NewAllocator(store, 0, 1)
c.Assert(alloc, NotNil)
err = alloc.Rebase(2, int64(1), false)
c.Assert(err, IsNil)
id, err = alloc.Alloc(2)
c.Assert(err, IsNil)
c.Assert(id, Equals, int64(2))

alloc = autoid.NewAllocator(store, 1)
alloc = autoid.NewAllocator(store, 0, 1)
c.Assert(alloc, NotNil)
err = alloc.Rebase(3, int64(3210), false)
c.Assert(err, IsNil)
alloc = autoid.NewAllocator(store, 1)
alloc = autoid.NewAllocator(store, 0, 1)
c.Assert(alloc, NotNil)
err = alloc.Rebase(3, int64(3000), false)
c.Assert(err, IsNil)
Expand Down Expand Up @@ -153,7 +153,7 @@ func (*testSuite) TestConcurrentAlloc(c *C) {
errCh := make(chan error, count)

allocIDs := func() {
alloc := autoid.NewAllocator(store, dbID)
alloc := autoid.NewAllocator(store, 0, dbID)
for j := 0; j < int(autoid.GetStep())+5; j++ {
id, err1 := alloc.Alloc(tblID)
if err1 != nil {
Expand Down Expand Up @@ -207,7 +207,7 @@ func (*testSuite) TestRollbackAlloc(c *C) {
injectConf := new(kv.InjectionConfig)
injectConf.SetCommitError(errors.New("injected"))
injectedStore := kv.NewInjectedStore(store, injectConf)
alloc := autoid.NewAllocator(injectedStore, 1)
alloc := autoid.NewAllocator(injectedStore, 0, 1)
_, err = alloc.Alloc(2)
c.Assert(err, NotNil)
c.Assert(alloc.Base(), Equals, int64(0))
Expand Down
22 changes: 9 additions & 13 deletions meta/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,18 +141,20 @@ func (m *Meta) parseTableID(key string) (int64, error) {
}

// GenAutoTableID adds step to the auto ID of the table and returns the sum.
func (m *Meta) GenAutoTableID(dbID int64, tableID int64, step int64) (int64, error) {
func (m *Meta) GenAutoTableID(originalDBID, dbID, tableID, step int64) (int64, error) {
// Check if DB exists.
Copy link
Member

Choose a reason for hiding this comment

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

Do we also need to check if old db exists or not ?

Copy link
Member

Choose a reason for hiding this comment

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

The old db doesn't need to exist, it can be dropped.

dbKey := m.dbKey(dbID)
id, err := m.txn.HGet(dbKey, m.autoTableIDKey(tableID))
if err != nil {
if err := m.checkDBExists(dbKey); err != nil {
return 0, errors.Trace(err)
}
isNotExisting := len(id) == 0
if isNotExisting {
return 0, kv.ErrNotExist
// Check if table exists.
tableKey := m.tableKey(tableID)
if err := m.checkTableExists(dbKey, tableKey); err != nil {
return 0, errors.Trace(err)
}

return m.txn.HInc(dbKey, m.autoTableIDKey(tableID), step)
// Using original DB ID to generate auto ID.
return m.txn.HInc(m.dbKey(originalDBID), m.autoTableIDKey(tableID), step)
}

// GetAutoTableID gets current auto id with table id.
Expand Down Expand Up @@ -271,12 +273,6 @@ func (m *Meta) CreateTable(dbID int64, tableInfo *model.TableInfo) error {
return errors.Trace(err)
}

// Initial the auto ID key.
base := []byte(strconv.FormatInt(0, 10))
err = m.txn.HSet(dbKey, m.autoTableIDKey(tableInfo.ID), base)
if err != nil {
return errors.Trace(err)
}
return m.txn.HSet(dbKey, tableKey, data)
}

Expand Down
52 changes: 32 additions & 20 deletions meta/meta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (s *testSuite) TestMeta(c *C) {
err = t.CreateTable(1, tbInfo)
c.Assert(err, IsNil)

n, err = t.GenAutoTableID(1, 1, 10)
n, err = t.GenAutoTableID(1, 1, 1, 10)
c.Assert(err, IsNil)
c.Assert(n, Equals, int64(10))

Expand Down Expand Up @@ -135,7 +135,7 @@ func (s *testSuite) TestMeta(c *C) {
c.Assert(err, IsNil)
c.Assert(tables, DeepEquals, []*model.TableInfo{tbInfo, tbInfo2})
// Generate an auto id.
n, err = t.GenAutoTableID(1, 2, 10)
n, err = t.GenAutoTableID(1, 1, 2, 10)
c.Assert(err, IsNil)
c.Assert(n, Equals, int64(10))
// Make sure the auto id key-value entry is there.
Expand All @@ -145,6 +145,7 @@ func (s *testSuite) TestMeta(c *C) {

err = t.DropTable(1, tbInfo2, true)
c.Assert(err, IsNil)
tbInfo2.OldSchemaID = 1
// Make sure auto id key-value entry is gone.
n, err = t.GetAutoTableID(1, 2)
c.Assert(err, IsNil)
Expand All @@ -163,39 +164,50 @@ func (s *testSuite) TestMeta(c *C) {
// Create table.
err = t.CreateTable(1, tbInfo100)
c.Assert(err, IsNil)
// Update auto id.
n, err = t.GenAutoTableID(1, tid, 10)
// Update auto ID.
originalDBID := int64(1)
currentDBID := int64(1)
n, err = t.GenAutoTableID(originalDBID, currentDBID, tid, 10)
c.Assert(err, IsNil)
c.Assert(n, Equals, int64(10))
// Fail to update auto id.
// Fail to update auto ID.
// The table ID doesn't exist.
nonExistentID := int64(1234)
_, err = t.GenAutoTableID(1, nonExistentID, 10)
_, err = t.GenAutoTableID(originalDBID, currentDBID, nonExistentID, 10)
c.Assert(err, NotNil)
n, err = t.GetAutoTableID(1, tid)
// Fail to update auto ID.
// The current database ID doesn't exist.
currentDBID = nonExistentID
_, err = t.GenAutoTableID(originalDBID, currentDBID, tid, 10)
c.Assert(err, NotNil)
// Make sure that auto id key-value entry is still 10.
n, err = t.GetAutoTableID(originalDBID, tid)
c.Assert(err, IsNil)
c.Assert(n, Equals, int64(10))
// Succeed in generating auto ID.
// Drop the table from original DB, then create the current DB and create the tbInfo100 under the current DB.
tbInfo100.OldSchemaID = currentDBID
// Drop table without touch auto id key-value entry.
err = t.DropTable(1, tbInfo100, false)
c.Assert(err, IsNil)
// Make sure that auto id key-value entry is still there.
n, err = t.GetAutoTableID(1, tid)
err = t.DropTable(originalDBID, tbInfo100, false)
c.Assert(err, IsNil)
c.Assert(n, Equals, int64(10))
// Drop table with old schema ID.
err = t.CreateTable(1, tbInfo100)
dbInfo1234 := &model.DBInfo{
ID: currentDBID,
Name: model.NewCIStr("db_x"),
}
err = t.CreateDatabase(dbInfo1234)
c.Assert(err, IsNil)
n, err = t.GenAutoTableID(1, tid, 10)
err = t.CreateTable(currentDBID, tbInfo100)
c.Assert(err, IsNil)
c.Assert(n, Equals, int64(10))
tbInfo100.OldSchemaID = 1
err = t.DropTable(1, tbInfo100, true)
_, err = t.GenAutoTableID(originalDBID, currentDBID, tid, 10)
c.Assert(err, IsNil)
n, err = t.GetAutoTableID(1, tid)
n, err = t.GetAutoTableID(originalDBID, tid)
c.Assert(err, IsNil)
c.Assert(n, Equals, int64(0))
c.Assert(n, Equals, int64(20))

err = t.DropDatabase(1)
c.Assert(err, IsNil)
err = t.DropDatabase(currentDBID)
c.Assert(err, IsNil)

dbs, err = t.ListDatabases()
c.Assert(err, IsNil)
Expand Down
6 changes: 3 additions & 3 deletions statistics/histogram.go
Original file line number Diff line number Diff line change
Expand Up @@ -571,9 +571,9 @@ func (idx *Index) getRowCount(sc *variable.StatementContext, indexRanges []*type
}
if bytes.Equal(lb, rb) {
if !indexRange.LowExclude && !indexRange.HighExclude {
rowCount, err := idx.equalRowCount(sc, types.NewBytesDatum(lb))
if err != nil {
return 0, errors.Trace(err)
rowCount, err1 := idx.equalRowCount(sc, types.NewBytesDatum(lb))
if err1 != nil {
return 0, errors.Trace(err1)
}
totalCount += rowCount
}
Expand Down
2 changes: 1 addition & 1 deletion util/admin/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func (s *testSuite) TestGetHistoryDDLJobs(c *C) {

func (s *testSuite) TestScan(c *C) {
defer testleak.AfterTest(c)()
alloc := autoid.NewAllocator(s.store, s.dbInfo.ID)
alloc := autoid.NewAllocator(s.store, s.tbInfo.OldSchemaID, s.dbInfo.ID)
tb, err := tables.TableFromMeta(alloc, s.tbInfo)
c.Assert(err, IsNil)
indices := tb.Indices()
Expand Down