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

meta/autoid: fix AUTO_ID_CACHE 1 setting affect row id allocator performance #39534

Merged
merged 26 commits into from
Dec 1, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
5e5abae
meta/autoid: fix AUTO_ID_CACHE 1 setting affect row id allocator perf…
tiancaiamao Dec 1, 2022
e13b800
fix a typo in comment
tiancaiamao Dec 1, 2022
25d8f02
fix a typo in comment
tiancaiamao Dec 1, 2022
a4a06ab
Merge branch 'master' into issue-39528
ti-chi-bot Dec 1, 2022
1c5388a
fix typo
tiancaiamao Dec 1, 2022
a5abf40
fix test case
tiancaiamao Dec 1, 2022
761585d
Merge branch 'master' into issue-39528
ti-chi-bot Dec 1, 2022
a487b53
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
294e5bc
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
cacad6b
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
d74c498
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
c397384
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
da35894
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
3938256
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
6548a79
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
032d322
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
3567a7e
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
2fa41ba
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
fe73086
Merge branch 'master' into issue-39528
tiancaiamao Dec 1, 2022
7f0bd83
Merge branch 'master' into issue-39528
ti-chi-bot Dec 1, 2022
eb54413
Merge branch 'master' into issue-39528
ti-chi-bot Dec 1, 2022
8a5caa9
Merge branch 'master' into issue-39528
ti-chi-bot Dec 1, 2022
c3dc7f0
Merge branch 'master' into issue-39528
ti-chi-bot Dec 1, 2022
31446a8
Merge branch 'master' into issue-39528
ti-chi-bot Dec 1, 2022
9318fe2
Merge branch 'master' into issue-39528
ti-chi-bot Dec 1, 2022
7aa366f
Merge branch 'master' into issue-39528
ti-chi-bot Dec 1, 2022
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
22 changes: 5 additions & 17 deletions ddl/db_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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;")

tiancaiamao marked this conversation as resolved.
Show resolved Hide resolved
// 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")
Expand All @@ -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(
tiancaiamao marked this conversation as resolved.
Show resolved Hide resolved
"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.
Expand Down
18 changes: 18 additions & 0 deletions executor/autoidtest/autoid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
20 changes: 16 additions & 4 deletions meta/autoid/autoid.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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 {
Expand Down