executor: load data statement, separate data preparing routine and commit routine #11533
Conversation
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
|
/run-all-tests |
crazycs520
reviewed
Jul 31, 2019
|
Could you do a test for many columns(such as 100 columns) ? |
en. will construct such testing scenarios， and profiling defailed cpu/memory usage |
|
/run-all-tests |
jackysp
reviewed
Aug 1, 2019
jackysp
reviewed
Aug 1, 2019
executor/load_data.go
Outdated
| // EnqOneTask feed one batch commit task to commit routine | ||
| func (e *LoadDataInfo) EnqOneTask() { | ||
| if e.curBatchCnt > 0 { | ||
| logutil.BgLogger().Info("tried to enqueue one task current len", zap.Uint64("len", e.curBatchCnt)) |
There was a problem hiding this comment.
We could make the committed batch size configurable. It is better to set it to debug level.
jackysp
reviewed
Aug 1, 2019
jackysp
reviewed
Aug 1, 2019
jackysp
reviewed
Aug 1, 2019
tiancaiamao
reviewed
Aug 2, 2019
executor/load_data.go
Outdated
| // InitQueues initialize task queue and error report queue | ||
| func (e *LoadDataInfo) InitQueues() { | ||
| e.commitTaskQueue = make(chan commitTask, 1000) | ||
| e.commitErrQueue = make(chan error, 5) |
server/conn.go
Outdated
| } | ||
| }() | ||
| // start commit worker routine | ||
| go loadDataInfo.CommitRoutine(ctx) |
There was a problem hiding this comment.
Why not make the load process into a separate goroutine and keep the commit process in this goroutine?
Codecov Report
@@ Coverage Diff @@
## master #11533 +/- ##
===========================================
Coverage 81.2905% 81.2905%
===========================================
Files 454 454
Lines 98720 98720
===========================================
Hits 80250 80250
Misses 12725 12725
Partials 5745 5745 |
|
/run-all-tests |
jackysp
reviewed
Aug 6, 2019
sessionctx/variable/tidb_vars.go
Outdated
| @@ -286,6 +286,9 @@ const ( | |||
|
|
|||
| // TiDBEnableNoopFuncs set true will enable using fake funcs(like get_lock release_lock) | |||
| TiDBEnableNoopFuncs = "tidb_enable_noop_functions" | |||
|
|
|||
| // TiDBLoadDataSeqProcess set true will make load data process and commit sequentially in one routine | |||
| TiDBLoadDataSeqProcess = "tidb_load_data_seq_process" | |||
ngaut
reviewed
Aug 21, 2019
executor/load_data.go
Outdated
| defer func() { | ||
| r := recover() | ||
| if r != nil { | ||
| buf := make([]byte, 4096) |
There was a problem hiding this comment.
Zap log library has a Stack function to get stack tracing.
ngaut
reviewed
Aug 21, 2019
executor/load_data.go
Outdated
| logutil.Logger(ctx).Error("commit error refresh", zap.Error(err)) | ||
| return err | ||
| } | ||
| // this only set in sequential mode, e.rows buffer will be reused in sequential mode |
ngaut
reviewed
Aug 21, 2019
executor/load_data.go
Outdated
| return err | ||
| } | ||
| } | ||
| logutil.Logger(ctx).Debug("one task enqueued ", zap.Int("current queue len", len(e.commitTaskQueue))) |
ngaut
reviewed
Aug 21, 2019
executor/load_data.go
Outdated
| break | ||
| } | ||
| tasks++ | ||
| logutil.Logger(ctx).Debug("commit one task finished", |
ngaut
reviewed
Aug 21, 2019
executor/load_data.go
Outdated
| zap.Int("pending tasks count", len(e.commitTaskQueue))) | ||
| } else { | ||
| end = true | ||
| logutil.Logger(ctx).Info("commit work all finished", |
ngaut
reviewed
Aug 21, 2019
server/conn.go
Outdated
| } | ||
| if err = loadDataInfo.Ctx.StmtCommit(); err != nil { | ||
| return nil, err | ||
| if enqTask { |
There was a problem hiding this comment.
We do not need to use enqTask argument.
ngaut
reviewed
Aug 21, 2019
server/conn.go
Outdated
| defer func() { | ||
| r := recover() | ||
| if r != nil { | ||
| buf := make([]byte, 4096) |
|
Please do not force push your commit. Just commit is fine. |
zz-jason
reviewed
Aug 28, 2019
executor/load_data.go
Outdated
|
|
||
| // these fields are used for pipeline data prepare and commit | ||
| commitTaskQueue chan CommitTask | ||
| QuitCommit chan struct{} |
There was a problem hiding this comment.
I think we can do it by only using one signal channel.
|
/run-unit-test |
jackysp
approved these changes
Sep 16, 2019
jackysp
added
the
status/can-merge
|
/run-all-tests |
|
@cfzjywxk merge failed. |
|
/rebuild |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What problem does this PR solve?
experiments diff on
#11517
What is changed and how it works?
load data statement
bench workload1
sysbench schema
1m rows = 185MB
10m rows = 1.9GB
batch size = 20k
batch size = 200k
batch size = 4k
data size = 10m
bench workload2
this workload task generator routine and commit routine cooperate well, producing and committing seem concurrently progressing. Master branch do all these things sequentially
column content = 99999999
100columns 1m rows = 852MB
500columns 1m rows = 4.2GB
500columns
bench workload3
wide table with 100 columns (all columns with BLOB or TEXT fields) , big column
work queue size 1000(max wait job numbers)
( txn limit 100MB)
schema like
100k rows = 1.2GB
some conclusions up to now
workload3 batch size = 5k and batch size = 1k have much different performance, batch size = 5kmany error logs in like
from monitor data on grafana, we see prewrite on kv is quit slow, the root cause should be the prewrite request is quite slow(32sec on 5k batch size)tikv/tikv#5279
has some explanations on prewrite slow on tikv executing workload3(big rowsize)
this could be solved by increase tikv raftstore.raft-max-inflight-msgs configuration(default -> 4096), allow more un ack raft messages in queue, after changing this config,
Check List
Tests
Code changes
Side effects
Related changes