meta/autoid: fix AUTO_ID_CACHE 1 setting affect row id allocator perf…

…ormance (#39534)

close #39528

ddl/db_integration_test.go

@@ -3036,21 +3036,6 @@ func TestAutoIncrementForceAutoIDCache(t *testing.T) {
 		return gid
 	}
 
-	// Rebase _tidb_row_id.
-	tk.MustExec("create table t (a int) AUTO_ID_CACHE 1")
-	tk.MustExec("alter table t force auto_increment = 2;")
-	tk.MustExec("insert into t values (1),(2);")
-	tk.MustQuery("select a, _tidb_rowid from t;").Check(testkit.Rows("1 1", "2 2"))
-	// Cannot set next global ID to 0.
-	tk.MustExec("alter table t force auto_increment = 0;")
-	tk.MustExec("alter table t force auto_increment = 1;")
-	require.Equal(t, uint64(3), getNextGlobalID())
-	// inserting new rows can overwrite the existing data.
-	tk.MustExec("insert into t values (3);")
-	tk.MustExec("insert into t values (3);")
-	tk.MustQuery("select a, _tidb_rowid from t;").Check(testkit.Rows("1 1", "2 2", "3 3", "3 4"))
-	tk.MustExec("drop table if exists t;")
-
 	// When AUTO_ID_CACHE is 1, row id and auto increment id use separate allocator, so the behaviour differs.
 	// "Alter table t force auto_increment" has no effect on row id.
 	tk.MustExec("create table t (a int) AUTO_ID_CACHE 1")
@@ -3060,11 +3045,14 @@ func TestAutoIncrementForceAutoIDCache(t *testing.T) {
 	// Cannot set next global ID to 0.
 	tk.MustExec("alter table t force auto_increment = 0;")
 	tk.MustExec("alter table t force auto_increment = 1;")
-	require.Equal(t, uint64(3), getNextGlobalID())
+	tk.MustQuery("show table t next_row_id").Check(testkit.Rows(
+		"auto_inc_force t _tidb_rowid 5001 _TIDB_ROWID",
+	))
+
 	// inserting new rows can overwrite the existing data.
 	tk.MustExec("insert into t values (3);")
 	tk.MustExec("insert into t values (3);")
-	tk.MustQuery("select a, _tidb_rowid from t;").Check(testkit.Rows("1 1", "2 2", "3 3", "3 4"))
+	tk.MustQuery("select a, _tidb_rowid from t;").Check(testkit.Rows("1 1", "2 2", "3 5001", "3 5002"))
 	tk.MustExec("drop table if exists t;")
 
 	// Rebase auto_increment.

executor/autoidtest/autoid_test.go

@@ -749,3 +749,21 @@ func TestMockAutoIDServiceError(t *testing.T) {
 	// Cover a bug that the autoid client retry non-retryable errors forever cause dead loop.
 	tk.MustExecToErr("insert into t_mock_err values (),()") // mock error, instead of dead loop
 }
+
+func TestIssue39528(t *testing.T) {
+	// When AUTO_ID_CACHE is 1, it should not affect row id setting when autoid and rowid are separated.
+	store := testkit.CreateMockStore(t)
+	tk := testkit.NewTestKit(t, store)
+	tk.MustExec("use test;")
+	tk.MustExec("create table issue39528 (id int unsigned key nonclustered auto_increment) shard_row_id_bits=4 auto_id_cache 1;")
+	tk.MustExec("insert into issue39528 values ()")
+	tk.MustExec("insert into issue39528 values ()")
+
+	ctx := context.Background()
+	var codeRun bool
+	ctx = context.WithValue(ctx, "testIssue39528", &codeRun)
+	_, err := tk.ExecWithContext(ctx, "insert into issue39528 values ()")
+	require.NoError(t, err)
+	// Make sure the code does not visit tikv on allocate path.
+	require.False(t, codeRun)
+}

meta/autoid/autoid.go

@@ -616,10 +616,17 @@ func NewAllocator(store kv.Storage, dbID, tbID int64, isUnsigned bool,
 	}
 
 	// Use the MySQL compatible AUTO_INCREMENT mode.
-	if allocType == AutoIncrementType && alloc.customStep && alloc.step == 1 && alloc.tbVersion >= model.TableInfoVersion5 {
-		alloc1 := newSinglePointAlloc(store, dbID, tbID, isUnsigned)
-		if alloc1 != nil {
-			return alloc1
+	if alloc.customStep && alloc.step == 1 && alloc.tbVersion >= model.TableInfoVersion5 {
+		if allocType == AutoIncrementType {
+			alloc1 := newSinglePointAlloc(store, dbID, tbID, isUnsigned)
+			if alloc1 != nil {
+				return alloc1
+			}
+		} else if allocType == RowIDAllocType {
+			// Now that the autoid and rowid allocator are separated, the AUTO_ID_CACHE 1 setting should not make
+			// the rowid allocator do not use cache.
+			alloc.customStep = false
+			alloc.step = step
 		}
 	}
 
@@ -972,6 +979,11 @@ func (alloc *allocator) alloc4Unsigned(ctx context.Context, n uint64, increment,
 			}()
 		}
 
+		if codeRun := ctx.Value("testIssue39528"); codeRun != nil {
+			*(codeRun.(*bool)) = true
+			return 0, 0, errors.New("mock error for test")
+		}
+
 		ctx = kv.WithInternalSourceType(ctx, kv.InternalTxnMeta)
 		err := kv.RunInNewTxn(ctx, alloc.store, true, func(ctx context.Context, txn kv.Transaction) error {
 			if span := opentracing.SpanFromContext(ctx); span != nil && span.Tracer() != nil {