-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add batch split #6
Conversation
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
8bfcf46
to
27080ea
Compare
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
PTAL @tikv/pingcap-i18n @zhangjinpeng1987 @siddontang |
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
@lilin90 PTAL. |
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
aa2ea7c
to
300f8b3
Compare
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
700a811
to
095d561
Compare
PTAL again |
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
Signed-off-by: Connor1996 <zbk602423539@gmail.com>
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.
Rest LGTM, I think the writing is good :)
text/2018-10-25-batch-split.md
Outdated
|
||
## Implementation in TiKV | ||
|
||
### How to produce multiple split keys |
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.
Do not overspecify the details in the RFC as doing so can bury the reviewer in minutiae.
IMO, this section is too detailed and not necessary.
PTAL again @huachaohuang |
general logic of SizeChecker and KeysChecker are similar. The only difference | ||
between them is one splits Region based on size and the other splits Region | ||
based on the number of keys. So here we mainly describe the logic of | ||
SizeChecker: |
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.
Actually, I think the following two paragraphs are too verbose and not necessary.
These explanations are more suitable as comments in the code than in the RFC.
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.
IMO, the algorithm should discussed in RFC stage.
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
@Connor1996 Try to get another approval :) |
PTAL @zhangjinpeng1987 |
ac2e9aa
to
0109896
Compare
0109896
to
dec91b9
Compare
No description provided.