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

*: separate auto_increment ID allocator from _tidb_rowid allocator #20708

Closed
wants to merge 12 commits into from

Conversation

tangenta
Copy link
Contributor

@tangenta tangenta commented Oct 29, 2020

What problem does this PR solve?

Issue Number: close #982

Problem Summary: before this PR, auto_increment ID and _tidb_rowid share the same key-value. The problem is any action that affects auto_increment column also has an impact on _tidb_rowid. For example,

create table t (a bigint auto_increment unique key);
insert into t values (9223372036854775805); -- MaxInt64-2
insert into t values (1);
Query OK, 0 rows affected (0.00 sec)
Query OK, 0 rows affected (0.00 sec)
ERROR 1467 (HY000): Failed to read auto-increment value from storage engine

The first insert statement forces auto_increment ID rebase to 9223372036854775807. Even if the second insert statement does not trigger auto_increment ID allocation, TiDB throws an "Failed to read" error because the _tidb_rowid allocator is exhausted.

What is changed and how it works?

What's Changed:

TiDB behavior changes:

  • ALTER TABLE ... AUTO_INCREMENT = ? will not affect _tidb_rowid allocation anymore.
  • session variables @@auto_increment_increment and @@auto_increment_offset will not affect _tidb_rowid allocation anymore.
  • the function of AUTO_ID_CACHE keeps unchanged(AUTO_ID_CACHE still affects _tidb_rowid allocation).

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch ?

Check List

Tests

  • Unit test
  • Integration test

Side effects

  • [ATTENTION] Breaking backward compatibility

Release note

  • No release note

@tangenta tangenta requested a review from a team as a code owner October 29, 2020 05:47
@tangenta tangenta requested review from qw4990 and removed request for a team October 29, 2020 05:47
@github-actions github-actions bot added sig/sql-infra SIG: SQL Infra sig/execution SIG execution labels Oct 29, 2020
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

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

Rest LGTM

executor/executor.go Show resolved Hide resolved
@ti-chi-bot ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed sig/infra labels Feb 22, 2021
@ti-chi-bot ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 3, 2021
@tangenta
Copy link
Contributor Author

tangenta commented Mar 3, 2021

/run-all-test

@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2021
@ti-chi-bot
Copy link
Member

@tangenta: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tangenta
Copy link
Contributor Author

/hold because the need to design a complete test

@ti-chi-bot ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 15, 2021
@qw4990 qw4990 removed their request for review June 21, 2021 03:21
@zimulala zimulala self-requested a review August 9, 2021 06:43
@tangenta
Copy link
Contributor Author

The new implementation is #28448.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. sig/execution SIG execution sig/sql-infra SIG: SQL Infra size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TiDB should use separated Allocators for the auto_increment column value and the row ID
3 participants