load: restore schemas in parallel #1466
Conversation
|
/reward |
|
The reward invalid. |
|
/reward 600 |
|
This PR do not have any linked issue. DetailsTip : About issue link, there is a trace issue. Warning: None |
|
/reward 600 |
|
This PR's linked issue is not picked. |
|
/reward 600 |
|
Reward success. |
lance6716
added
needs-cherry-pick-release-2.0
There was a problem hiding this comment.
and you may take a look at https://pkg.go.dev/golang.org/x/sync/errgroup
|
@lance6716 PTAL anytime you have time :D |
|
|
Ok, I will provide some unit test(use cases) later. |
|
@lance6716 reply later... |
|
/run-all-tests |
|
@lance6716 PTAL |
👌 |
|
@GMHDBJD @lichunzhu PTAL |
lance6716
removed
the
needs-cherry-pick-release-2.0
|
/lgtm |
loader/loader.go
Outdated
| err = tblRestoreQueue.push(&restoreSchemaJob{ | ||
| loader: l, | ||
| session: dbSessionPool[dbSessionID], | ||
| database: db, | ||
| table: table, | ||
| filepath: schemaFile, | ||
| }) |
There was a problem hiding this comment.
It sames that there will be several jobs running at the same time and using the same mysql connection? If so, is there any problem with this situation?
There was a problem hiding this comment.
it's no problem for now, maybe use buffered channel implement connection pool is more perfect?
There was a problem hiding this comment.
Yes. Or we can start connections for each consumer thread. And close them after we stop the consumers. Like we did in dumpling.
Besides, sql.Rows will use sql.Conn exclusively. We'd better refine this part.
There was a problem hiding this comment.
Besides, sql.Rows will use sql.Conn exclusively. We'd better refine this part.
I don't get it 🤣 , would you help me understand that? thx!
There was a problem hiding this comment.
Besides, sql.Rows will use sql.Conn exclusively. We'd better refine this part.
go func() {
rows, _ := conn.QueryContext(ctx, "select * from pingcap.tb")
for rows.Next() {
time.Sleep(time.Minute)
}
}()
time.Sleep(time.Second)
rows, err := conn.QueryContext(ctx, "select * from pingcap.tb")
if err != nil {
return errors.Trace(err)
}
for rows.Next() {
}I mean this kind of code will cause an error.
[mysql] 2021/03/26 18:20:32 packets.go:446: busy bufferThere was a problem hiding this comment.
Besides, sql.Rows will use sql.Conn exclusively. We'd better refine this part.
go func() { rows, _ := conn.QueryContext(ctx, "select * from pingcap.tb") for rows.Next() { time.Sleep(time.Minute) } }() time.Sleep(time.Second) rows, err := conn.QueryContext(ctx, "select * from pingcap.tb") if err != nil { return errors.Trace(err) } for rows.Next() { }I mean this kind of code will cause an error.
[mysql] 2021/03/26 18:20:32 packets.go:446: busy buffer
Thanks a lot for your reminder! I don't even know that has such a deeply hidden problem. Fortunately, our modification will only perform DDL operations, so it will not trigger concurrent read buffer situations.
There was a problem hiding this comment.
this should has been fixed in aae003e @lichunzhu
|
@lichunzhu PTAL |
|
/lgtm |
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by writing |
ti-chi-bot
added
status/LGT2
lance6716
added
the
needs-cherry-pick-release-2.0
|
/merge |
|
This pull request has been accepted and is ready to merge. Commit hash: 8291b8c
|
|
/merge |
|
@hidehalo: In response to this:
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 ti-community-infra/tichi repository. |
|
/run-all-tests |
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
|
cherry pick to release-2.0 in PR #1544 |
ti-srebot
added
already-cherry-pick-2.0
What problem does this PR solve?
Issue Number: close #717
What is changed and how it works?
jobQueuedeal with task of restore schemasVERBOSEenvironment variable to support verbose mode for shell scriptsCheck List
Tests
run
many_tables(300*table) integration test on custom TiDB cluster with load balanceBenchmark results
Side effects
Related changes