From 896afa74a1e56f5e2d8deb8c87d0ecdce14dfd3a Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Thu, 25 Apr 2024 14:39:19 +0800 Subject: [PATCH] autoid_service: fix non-continuous autoid for allocating N IDs (#52517) close pingcap/tidb#52465 --- pkg/autoid_service/autoid.go | 30 ++++--- .../integrationtest/r/executor/autoid.result | 80 +++++++++++++++++++ tests/integrationtest/t/executor/autoid.test | 24 ++++++ 3 files changed, 125 insertions(+), 9 deletions(-) diff --git a/pkg/autoid_service/autoid.go b/pkg/autoid_service/autoid.go index 87839c9173..5fa3ea1cff 100644 --- a/pkg/autoid_service/autoid.go +++ b/pkg/autoid_service/autoid.go @@ -73,11 +73,11 @@ func (alloc *autoIDValue) alloc4Unsigned(ctx context.Context, store kv.Storage, // calcNeededBatchSize calculates the total batch size needed. n1 := calcNeededBatchSize(alloc.base, int64(n), increment, offset, isUnsigned) - // The local rest is not enough for alloc, skip it. + // The local rest is not enough for alloc. if uint64(alloc.base)+uint64(n1) > uint64(alloc.end) || alloc.base == 0 { var newBase, newEnd int64 nextStep := int64(batch) - // Although it may skip a segment here, we still treat it as consumed. + fromBase := alloc.base ctx = kv.WithInternalSourceType(ctx, kv.InternalTxnMeta) err := kv.RunInNewTxn(ctx, store, true, func(_ context.Context, txn kv.Transaction) error { @@ -88,7 +88,12 @@ func (alloc *autoIDValue) alloc4Unsigned(ctx context.Context, store kv.Storage, return err1 } // calcNeededBatchSize calculates the total batch size needed on new base. - n1 = calcNeededBatchSize(newBase, int64(n), increment, offset, isUnsigned) + if alloc.base == 0 || newBase != alloc.end { + alloc.base = newBase + alloc.end = newBase + n1 = calcNeededBatchSize(newBase, int64(n), increment, offset, isUnsigned) + } + // Although the step is customized by user, we still need to make sure nextStep is big enough for insert batch. if nextStep < n1 { nextStep = n1 @@ -111,11 +116,11 @@ func (alloc *autoIDValue) alloc4Unsigned(ctx context.Context, store kv.Storage, zap.String("category", "autoid service"), zap.Int64("dbID", dbID), zap.Int64("tblID", tblID), - zap.Int64("from base", alloc.base), + zap.Int64("from base", fromBase), zap.Int64("from end", alloc.end), zap.Int64("to base", newBase), zap.Int64("to end", newEnd)) - alloc.base, alloc.end = newBase, newEnd + alloc.end = newEnd } min = alloc.base // Use uint64 n directly. @@ -142,11 +147,12 @@ func (alloc *autoIDValue) alloc4Signed(ctx context.Context, return 0, 0, errAutoincReadFailed } - // The local rest is not enough for allocN, skip it. + // The local rest is not enough for allocN. // If alloc.base is 0, the alloc may not be initialized, force fetch from remote. if alloc.base+n1 > alloc.end || alloc.base == 0 { var newBase, newEnd int64 nextStep := int64(batch) + fromBase := alloc.base ctx = kv.WithInternalSourceType(ctx, kv.InternalTxnMeta) err := kv.RunInNewTxn(ctx, store, true, func(_ context.Context, txn kv.Transaction) error { @@ -157,7 +163,13 @@ func (alloc *autoIDValue) alloc4Signed(ctx context.Context, return err1 } // calcNeededBatchSize calculates the total batch size needed on global base. - n1 = calcNeededBatchSize(newBase, int64(n), increment, offset, isUnsigned) + // alloc.base == 0 means uninitialized + // newBase != alloc.end means something abnormal, maybe transaction conflict and retry? + if alloc.base == 0 || newBase != alloc.end { + alloc.base = newBase + alloc.end = newBase + n1 = calcNeededBatchSize(newBase, int64(n), increment, offset, isUnsigned) + } // Although the step is customized by user, we still need to make sure nextStep is big enough for insert batch. if nextStep < n1 { nextStep = n1 @@ -180,11 +192,11 @@ func (alloc *autoIDValue) alloc4Signed(ctx context.Context, zap.String("category", "autoid service"), zap.Int64("dbID", dbID), zap.Int64("tblID", tblID), - zap.Int64("from base", alloc.base), + zap.Int64("from base", fromBase), zap.Int64("from end", alloc.end), zap.Int64("to base", newBase), zap.Int64("to end", newEnd)) - alloc.base, alloc.end = newBase, newEnd + alloc.end = newEnd } min = alloc.base alloc.base += n1 diff --git a/tests/integrationtest/r/executor/autoid.result b/tests/integrationtest/r/executor/autoid.result index c4b7ad3882..cc57ca2854 100644 --- a/tests/integrationtest/r/executor/autoid.result +++ b/tests/integrationtest/r/executor/autoid.result @@ -737,3 +737,83 @@ auto_increment_increment 65535 auto_increment_offset 65535 set auto_increment_offset = default; set auto_increment_increment = default; +drop table if exists issue52465; +create table issue52465 (id int primary key auto_increment, k int) AUTO_ID_CACHE=1; +insert into issue52465 (k) values (1); +insert into issue52465 values (3997, 2); +select * from issue52465 t; +id k +1 1 +3997 2 +insert into issue52465 (k) values (1),(2),(3),(4),(5),(6),(7),(8),(9),(10); +select * from issue52465; +id k +1 1 +3997 2 +3998 1 +3999 2 +4000 3 +4001 4 +4002 5 +4003 6 +4004 7 +4005 8 +4006 9 +4007 10 +insert into issue52465 (k) values (11); +select * from issue52465; +id k +1 1 +3997 2 +3998 1 +3999 2 +4000 3 +4001 4 +4002 5 +4003 6 +4004 7 +4005 8 +4006 9 +4007 10 +4008 11 +drop table issue52465; +drop table if exists issue52465; +create table issue52465 (id int unsigned primary key auto_increment, k int) AUTO_ID_CACHE=1; +insert into issue52465 (k) values (1); +insert into issue52465 values (3997, 2); +select * from issue52465 t; +id k +1 1 +3997 2 +insert into issue52465 (k) values (1),(2),(3),(4),(5),(6),(7),(8),(9),(10); +select * from issue52465; +id k +1 1 +3997 2 +3998 1 +3999 2 +4000 3 +4001 4 +4002 5 +4003 6 +4004 7 +4005 8 +4006 9 +4007 10 +insert into issue52465 (k) values (11); +select * from issue52465; +id k +1 1 +3997 2 +3998 1 +3999 2 +4000 3 +4001 4 +4002 5 +4003 6 +4004 7 +4005 8 +4006 9 +4007 10 +4008 11 +drop table issue52465; diff --git a/tests/integrationtest/t/executor/autoid.test b/tests/integrationtest/t/executor/autoid.test index 7adb3104ca..833de0f25a 100644 --- a/tests/integrationtest/t/executor/autoid.test +++ b/tests/integrationtest/t/executor/autoid.test @@ -483,3 +483,27 @@ show variables like 'auto_increment%'; set auto_increment_offset = default; set auto_increment_increment = default; + +## Test for issue 52465 +drop table if exists issue52465; +create table issue52465 (id int primary key auto_increment, k int) AUTO_ID_CACHE=1; +insert into issue52465 (k) values (1); +insert into issue52465 values (3997, 2); +select * from issue52465 t; +insert into issue52465 (k) values (1),(2),(3),(4),(5),(6),(7),(8),(9),(10); +select * from issue52465; +insert into issue52465 (k) values (11); +select * from issue52465; +drop table issue52465; + + +drop table if exists issue52465; +create table issue52465 (id int unsigned primary key auto_increment, k int) AUTO_ID_CACHE=1; +insert into issue52465 (k) values (1); +insert into issue52465 values (3997, 2); +select * from issue52465 t; +insert into issue52465 (k) values (1),(2),(3),(4),(5),(6),(7),(8),(9),(10); +select * from issue52465; +insert into issue52465 (k) values (11); +select * from issue52465; +drop table issue52465;