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

ddl: add syntax for setting the cache step of auto id explicitly. #15409

Merged
merged 14 commits into from
Apr 10, 2020

Conversation

AilinKid
Copy link
Contributor

@AilinKid AilinKid commented Mar 16, 2020

What problem does this PR solve?

That autoid allocator's cache step is quite big now, it will consume a lot of autoid when tidb restarts or crashes. For those scenes which treasure autoid heavily, it's not acceptable.

Parser link: #765

Problem Summary:

What is changed and how it works?

Add auto_increment_cache in create table / alter table statement and it will determine the allocator's cache step.

But there is an exception when a statement like an insert batch(insert into values()()...) which requires allocating consecutive N autoid in one statement, we will make sure that the custom cache step is adequate for it with if step < N, then step = min(2N, maxstep)

Attention please:
tidb handle share the same allocator with auto_increment, so here:
auto_increment_cache will take effect on handle if PKIshandle is false, or even there is no auto_increment column at all.

Related changes

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

Check List

Tests

  • Unit test
  • Integration test

Release note:

ddl: add syntax for setting the cache step of auto id explicitly.

@AilinKid AilinKid requested a review from a team as a code owner March 16, 2020 13:32
@ghost ghost requested review from wshwsh12 and XuHuaiyu and removed request for a team March 16, 2020 13:32
@AilinKid AilinKid requested review from tangenta and bb7133 and removed request for wshwsh12 and XuHuaiyu March 16, 2020 13:32
@github-actions github-actions bot added sig/sql-infra SIG: SQL Infra sig/execution SIG execution labels Mar 16, 2020
@AilinKid
Copy link
Contributor Author

This PR need parser merged first

@codecov
Copy link

codecov bot commented Mar 18, 2020

Codecov Report

Merging #15409 into master will not change coverage by %.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #15409   +/-   ##
===========================================
  Coverage   80.6188%   80.6188%           
===========================================
  Files           506        506           
  Lines        137226     137226           
===========================================
  Hits         110630     110630           
  Misses        18092      18092           
  Partials       8504       8504           

@Deardrops Deardrops self-requested a review March 18, 2020 03:14
@bb7133 bb7133 changed the title ddl: support auto increment cache ddl: add syntax for setting the cache step of auto id explicitly. Mar 18, 2020
ddl/table.go Outdated Show resolved Hide resolved
ddl/ddl_api.go Outdated Show resolved Hide resolved
executor/show.go Outdated Show resolved Hide resolved
meta/autoid/autoid.go Show resolved Hide resolved
@bb7133
Copy link
Member

bb7133 commented Mar 30, 2020

LGTM

@AilinKid AilinKid requested review from tangenta and Deardrops and removed request for tangenta April 3, 2020 06:23
ddl/table.go Outdated Show resolved Hide resolved
ddl/ddl_api.go Show resolved Hide resolved
Copy link
Contributor

@Deardrops Deardrops left a comment

Choose a reason for hiding this comment

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

LGTM

@AilinKid AilinKid added the require-LGT3 Indicates that the PR requires three LGTM. label Apr 7, 2020
@AilinKid AilinKid requested a review from djshow832 April 7, 2020 09:48
@AilinKid
Copy link
Contributor Author

/run-all-tests

@AilinKid
Copy link
Contributor Author

/run-sqllogic-test-2

@AilinKid
Copy link
Contributor Author

/run-all-tests

@AilinKid
Copy link
Contributor Author

/run-check_dev_2

@AilinKid AilinKid added status/all-tests-passed status/LGT3 The PR has already had 3 LGTM. labels Apr 10, 2020
@bb7133 bb7133 merged commit 1c73dec into pingcap:master Apr 10, 2020
sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Apr 10, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor

sre-bot commented Apr 10, 2020

cherry pick to release-2.1 in PR #16286

@sre-bot
Copy link
Contributor

sre-bot commented Apr 10, 2020

cherry pick to release-3.0 in PR #16287

@sre-bot
Copy link
Contributor

sre-bot commented Apr 10, 2020

cherry pick to release-3.1 in PR #16288

@sre-bot
Copy link
Contributor

sre-bot commented Apr 10, 2020

cherry pick to release-4.0 in PR #16289

AilinKid added a commit to sre-bot/tidb that referenced this pull request Apr 26, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
AilinKid added a commit to sre-bot/tidb that referenced this pull request Apr 28, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
require-LGT3 Indicates that the PR requires three LGTM. sig/execution SIG execution sig/sql-infra SIG: SQL Infra status/LGT3 The PR has already had 3 LGTM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants