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: support batch create table #28763

Merged
merged 29 commits into from
Dec 31, 2021
Merged

ddl: support batch create table #28763

merged 29 commits into from
Dec 31, 2021

Conversation

xhebox
Copy link
Contributor

@xhebox xhebox commented Oct 12, 2021

What problem does this PR solve?

Issue Number: refer #30272

Problem Summary: add an internal batch create table api for br, all happens in one schema version.

Some notes on the behavior:

  1. This API does not set query since it is not from parser. One must set ctx.Value(sessionctx.QueryString) manually for displaying admin show ddl queries.
  2. admin show ddl jobs can not show multiple ids in tableID column, but will show comma separated list of table names in tableName column.

What is changed and how it works?

What's Changed:

  1. add CreateTablesWithInfo, the new batch api.
  2. Common part in CreateTableWithInfo and CreateTablesWithInfo are extracted.
  3. Common part in onCreateTable and onCreateTables are extracted.
  4. add simple unit tests in db_test.go and table_test.go.
  5. add Affected []HistoryInfoAffected in binlog.HistoryInfo for display multiple table names.
  6. When the job is cancelled, its arguments will be deleted. Since someone may issued a super large job that does not fit into the history queue.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

None

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Oct 12, 2021

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • AilinKid
  • wjhuang2016

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 12, 2021
infoschema/builder.go Outdated Show resolved Hide resolved
@fengou1
Copy link
Contributor

fengou1 commented Oct 18, 2021

/cc @wjhuang2016

ddl/ddl.go Outdated Show resolved Hide resolved
@fengou1
Copy link
Contributor

fengou1 commented Oct 19, 2021

/run-check_dev_2

1 similar comment
@fengou1
Copy link
Contributor

fengou1 commented Oct 19, 2021

/run-check_dev_2

@xhebox
Copy link
Contributor Author

xhebox commented Oct 19, 2021

/run-check_dev_2

It is a problem of the unit test code, should be fixed now.

ddl/ddl.go Outdated Show resolved Hide resolved
ddl/ddl_api.go Show resolved Hide resolved
ddl/ddl_api.go Show resolved Hide resolved
ddl/ddl_api.go Outdated Show resolved Hide resolved
ddl/ddl_api.go Show resolved Hide resolved
ddl/table.go Show resolved Hide resolved
@fengou1
Copy link
Contributor

fengou1 commented Oct 20, 2021

A integration test show the number of tables > 10000, the function call CreateTablesWithInfo got stuck. as private communication with xhebox, it looks CreateTablesWithInfo hit the raft entry max size limitation ( the sum of batch tables shall less than 500 if raft-entry-max-size = 8M (by default))

  1. This PR shall return a error when we hit the max raft limitation instead of stuck
  2. BR batch size set to 500 (possible restore table speed 50 per-seconds)

@fengou1
Copy link
Contributor

fengou1 commented Oct 20, 2021

BR batch size set to 500, restore successful. I think we are good to merge this PR to master.

@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 26, 2021
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 26, 2021
@ti-chi-bot ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 27, 2021
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 15, 2021
@fengou1
Copy link
Contributor

fengou1 commented Dec 21, 2021

/run-unit-test

@ti-chi-bot ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 22, 2021
@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 319fbda

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 31, 2021
@xhebox
Copy link
Contributor Author

xhebox commented Dec 31, 2021

/hold

@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 Dec 31, 2021
@xhebox
Copy link
Contributor Author

xhebox commented Dec 31, 2021

/unhold

@ti-chi-bot ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 31, 2021
@ti-chi-bot ti-chi-bot merged commit 4f30a14 into pingcap:master Dec 31, 2021
@xhebox xhebox added the needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. label Dec 31, 2021
@xhebox
Copy link
Contributor Author

xhebox commented Dec 31, 2021

/cherry-pick release-5.4

@ti-srebot
Copy link
Contributor

cherry pick to release-5.4 in PR #31234

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Dec 31, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
rebelice pushed a commit to TiInterstellar/tidb that referenced this pull request Jan 5, 2022
* topsql: make topsql enable only be controlled by pub/sub sink (pingcap#31209)

* ddl: support batch create table  (pingcap#28763)

* executor: fix data race in IndexMergeReaderExec (pingcap#31230)

close pingcap#31229

* server: filter the EOF error for normal closed at handshake  (pingcap#31081)

close pingcap#31063

* expression: change date add function return type (pingcap#28133)

close pingcap#27573

* support create interval partition

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* support create interval partition (support int/timestamp partition key)

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* parser: support alter table partitions move engine statement

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* support ddl operation

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* support interval partition manager

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* support interval partition manager handle job framwork

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* support auto create interval partition when insert meet no partition suitable error

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* fix bug

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* fix cancel job and load old job then continue to do

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* make partition readonly work(not allow to insert/update/delete)

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* add begin,end time in tables

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* tiny fix for auto create interval partition in concurrent case

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* init

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* init

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* todo: remove flag

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* fix dumpling

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* remove data in aws s3 when drop/truncate table/partition

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* make hello world work

Signed-off-by: crazycs520 <crazycs520@gmail.com>

* remove debug info

Signed-off-by: crazycs520 <crazycs520@gmail.com>

Co-authored-by: xhe <xw897002528@gmail.com>
Co-authored-by: guo-shaoge <shaoge1994@163.com>
Co-authored-by: knull-cn <hu__haifeng@163.com>
Co-authored-by: Meng Xin <tregoldmeng@gmail.com>
@fengou1 fengou1 removed the needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. label Mar 10, 2022
@xhebox xhebox deleted the batch_1 branch August 16, 2022 16:26
@xhebox xhebox restored the batch_1 branch August 16, 2022 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants