This repository has been archived by the owner on Dec 8, 2021. It is now read-only.
Support multi-engine per table (batching) #113
Merged
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
kennytm
added
status/WIP
|
Hi contributor, thanks for your PR. This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically. |
|
/run-all-tests |
kennytm
changed the title
kennytm
added
status/PTAL
lonng
reviewed
Jan 2, 2019
Removed `[tikv-importer] batch-size` to avoid confusion. Removed `[mydumper] min-region-size` since it is useless now.
kennytm
force-pushed
the
kennytm/batching
branch
from
bb752cc
to
54de1aa
Compare
|
/run-all-tests |
IANTHEREAL
reviewed
Jan 2, 2019
| uint32 status = 3; | ||
| int64 alloc_base = 4; | ||
| repeated EngineCheckpointModel engines = 6; |
There was a problem hiding this comment.
why not use map, a nature way
There was a problem hiding this comment.
Engine IDs are assigned sequentially, so making it an array is more natural I think (a map would be just mapping 0, 1, 2, 3, ... to engines).
There was a problem hiding this comment.
you must make sure Engine IDs are assigned sequentially, this logic should be written in design document
There was a problem hiding this comment.
OK, Updated the design doc.
| if err := engineRows.Scan(&engineID, &status); err != nil { | ||
| return errors.Trace(err) | ||
| } | ||
| for len(cp.Engines) <= engineID { |
| @@ -0,0 +1 @@ | |||
| create database cpeng; | |||
There was a problem hiding this comment.
Check-Point — Engines 🤔
| &value.Chunk.Offset, &value.Chunk.EndOffset, &value.Chunk.PrevRowIDMax, &value.Chunk.RowIDMax, | ||
| &kvcBytes, &kvcKVs, &kvcChecksum, | ||
| ); err != nil { | ||
| return errors.Trace(err) | ||
| } | ||
| value.Checksum = verify.MakeKVChecksum(kvcBytes, kvcKVs, kvcChecksum) | ||
| cp.Chunks = append(cp.Chunks, value) | ||
| cp.Engines[engineID].Chunks = append(cp.Engines[engineID].Chunks, value) |
There was a problem hiding this comment.
is chunks sorted as comment at L101?
There was a problem hiding this comment.
Yes... a subslice of a sorted slice is still sorted 😁
kennytm
force-pushed
the
kennytm/batching
branch
from
54de1aa
to
294f228
Compare
|
/run-all-tests |
lonng
suggested changes
Jan 3, 2019
lonng
reviewed
Jan 3, 2019
lonng
approved these changes
Jan 3, 2019
kennytm
force-pushed
the
kennytm/batching
branch
from
59f529c
to
a2274b8
Compare
kennytm
force-pushed
the
kennytm/batching
branch
from
a2274b8
to
92e8dad
Compare
kennytm
added
status/LGT1
|
LGTM |
lonng
added
status/LGT2
|
/run-all-tests |
|
LGTM |
IANTHEREAL
added
status/LGT2
|
I'll merge after confirming whether 10G is a good default size. Maybe this needs to be larger (the previous default was 500G). |
* mydump: non-uniform batch size * *: make the `batch-size-scale` configurable * *: implemented the optimized non-uniform strategy * tests: due to change of strategy, checkpoint_engines count becomes 4 again * mydump/region: slightly adjust the batch size computation * Use the exact result of 1/Beta(N, R) instead of an approximation * When the number of engines is small and the total engine size of the first (table-concurrency) batches exceed the table size, the last batch was truncated, and disrupt the pipeline. Now in these case we will reduce the batch size to avoid this disruption. * restore: log the SQL size and KV size of each engine for debugging * config: change default batch size and ratio given experiment result * config: added more explanation about batch-import-ratio
kennytm
removed
the
Should Update Docs
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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?
Implements RFC 3 (a.k.a. Batching).
What is changed and how it works?
Decoupled 1 table = 1 engine. Now one table can produce multiple engines, partitioned by a
batch-size, allowing import of table partially. See the design document above for details.Check List
Tests
Code changes
Side effects
Related changes
tidb-ansiblerepository