This repository has been archived by the owner on Jul 24, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 102
Speed up DDL execution by send DDL concurrently #377
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
6af92ed
task, restore: send DDLs parallelly
e703513
restore: use moded id to index sessionpool
4aae824
Merge branch 'master' of https://github.com/pingcap/br into par-ddl
cecdece
store: return nil DB when no TiDB session provided
9c5ec46
*: use isolated DB instead of Session
9532496
*: fix some mis-pushed files :|
4cdb758
*: rename session pool to DB pool
2739874
Merge branch 'master' into par-ddl
YuJuncen 3e910ab
restore: rename some sessions to dbs
2025d2b
Merge branch 'par-ddl' of https://github.com/Yujuncen/br into par-ddl
991536b
Merge branch 'master' into par-ddl
YuJuncen 539ae21
restore: add some todos
1b4f774
Merge branch 'master' into par-ddl
YuJuncen cbf9957
Merge branch 'master' into par-ddl
3pointer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove
rc.db
, or make the DB pool[]*DB
a field ofrc *Client
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rc.db
still be used for other DDLs (e.g.CREATE DATABASE
,ALTER TABLE SET TIFLASH_REPLICA = x
, incremental DDL execution, etc.), if we want to remove it, we must make many changes. (BTW It's seems a little strange if these uses db likerc.dbs[n]
outsideGoCreateTables
, there is an array but we just need one of them at anytime, since those DDLs are executed sequential.).Maybe do that at another refactoring PR would be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could have a
rc.GetDB()
and rotate the db list on every call 🙃.anyway, as we decide to refactor in the future, please add a TODO on the
db
field.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But how
GetDB()
should be implemented? If we trivially returnrc.dbs[0]
, seems we cannot take any good from it... Because we still need to inject some 'id' to every call if we need to use other dbs (hence we can't remove the extra parameter ofcreateTable
), and we use just one db at once (hence we can't speed up execution outsiderestoreTables
).I think maybe we need farther discussion to make sure whether we need to prune the
db
field... 🤔Or any better idea if possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@YuJuncen round-robin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's a good idea, but seem here would still be some risk if we want to use this algorithm to allocate session for processes concurrently running, that is, when some session stuck:
Assume we have two sessions, and two processes.
Maybe this cannot provide more for us... Our questions still present: we cannot remove the extra parameter of
createTable
, and out ofcreateTable
, we cannot take good of multi-dbs too, since they are executed sequentially.