-
Notifications
You must be signed in to change notification settings - Fork 85
Conversation
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.
Great! Please also update this part for WriteInsertInCsv
.
https://github.com/pingcap/dumpling/blob/master/v4/export/writer_util.go#L299
Done. unit test & integration test also passed. |
OK. Please sign the Contributor License Agreement. |
signed. |
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.
LGTM
/reward 600 |
This PR already closed! |
Already added manually. Please reward before merging. |
* fix for_select_default_busy
* fix for_select_default_busy
* fix for_select_default_busy
* fix for_select_default_busy
* fix for_select_default_busy
Issue Number: #126
What problem does this PR solve?
Related to #126.
Reduce the cost of
selectgo
inwriteInsert
method, which can improve 30%+ perfomance of dumping data.What is changed and how it works?
selectgo
runs only in the case of buffer Reseting.Check List
Tests
Unit test
Result:
? github.com/pingcap/dumpling/cmd/dumpling [no test files]
? github.com/pingcap/dumpling/v4/cli [no test files]
ok github.com/pingcap/dumpling/v4/export 0.145s coverage: 51.6% of statements
? github.com/pingcap/dumpling/v4/log [no test files]
Integration test
Result:
[2020/10/21 13:53:13.812 +08:00] [INFO] [main.go:253] ["dump data successfully, dumpling will exit now"]
Cleaning up test output dir: /tmp/dumpling_test_result/sql_res.views
Passed integration tests.
Manual test (add detailed scripts or steps below)
Before changing:
Costs:
Flamegraph:
After changing
Costs:
Flamegraph:
Side effects
Possible performance regression
Maybe little delay to handle ctx.Done()
Increased code complexity
None
Breaking backward compatibility
Related changes
Release note