compaction: add a new option 'check_range_overlap_on_bottom_level' for manual compact#421
Conversation
Signed-off-by: glorv <glorvs@163.com>
ti-chi-bot
bot
added
dco-signoff: yes
| } | ||
| } | ||
| if (!check_overlap_within_file) { | ||
| if (!check_overlap_within_file || |
There was a problem hiding this comment.
Why !check_overlap_within_file is not sufficient?
rocksdb/db/db_impl/db_impl_compaction_flush.cc
Lines 1225 to 1227 in 2b04d05
Seems that if there are partitioner boundaries within the target range, it will trigger a compaction, not trivial move (by setting overlap to the current level)
There was a problem hiding this comment.
Yes, you are right, it's better to just change the CanDoTrivialMove implement in the SSTPartitioner. No need to add a new option.
There was a problem hiding this comment.
But it seems CanDoTrivialMove can't tell whether we are at the bottommost level? It only sees the key range
There was a problem hiding this comment.
Yes, but I think we should force partition/manual compact the overlapped SST files in all the level, not only the bottom level. This approach should gain better compatibility. So reuse CanDoTrivialMove is enough here.
|
/hold |
ti-chi-bot
bot
added
the
do-not-merge/hold
| } | ||
| if (!check_overlap_within_file) { | ||
| if (!check_overlap_within_file || | ||
| (!overlap && options.bottom_level_check_range_overlap && |
There was a problem hiding this comment.
Could you add a comment to elaborate on why this can ensure the SST file in the bottom level with a huge range can be force partitioned with manual compaction.
Signed-off-by: glorv <glorvs@163.com>
|
@v01dstar @Connor1996 PTAL again. |
|
/unhold |
ti-chi-bot
bot
removed
the
do-not-merge/hold
include/rocksdb/options.h
Outdated
| // If set to true, it will check file range overlap instead of keys overlap | ||
| // for the bottom level. This is used in manual compact for SST ingestion | ||
| // scenario. | ||
| bool bottom_level_check_range_overlap = false; |
There was a problem hiding this comment.
I suggest renaming it to check_range_overlap_on_bottom_level to match the existing naming style.
|
@hhwyt: Your lgtm message is repeated, so it is ignored. DetailsIn 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 kubernetes-sigs/prow repository. |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
|
/hold |
ti-chi-bot
bot
added
the
do-not-merge/hold
Signed-off-by: glorv <glorvs@163.com>
…rocksdb into manual-compact-bottom-check
glorv
changed the title
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
|
/unhold |
ti-chi-bot
bot
removed
the
do-not-merge/hold
glorv
changed the title
ti-chi-bot
bot
added
needs-1-more-lgtm
|
/retest |
|
/run-test |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hbisheng, hhwyt, v01dstar The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…e_partition_range` (#19121) close #19106, close #19120 This PR include the new change in tikv/rocksdb#421 that introduce a new config `check_range_overlap_on_bottom_level`. By set this config to true, the manual compaction while use range overlap instead of key overlap on the bottom level for selecting candidate SST files. This can ensure the manual compact can split large range SST files at the bottom level which blocks SST ingesting to the bottom level. Signed-off-by: glorv <glorvs@163.com>
…e_partition_range` (#19121) (#19124) close #19106, close #19120 This PR include the new change in tikv/rocksdb#421 that introduce a new config `check_range_overlap_on_bottom_level`. By set this config to true, the manual compaction while use range overlap instead of key overlap on the bottom level for selecting candidate SST files. This can ensure the manual compact can split large range SST files at the bottom level which blocks SST ingesting to the bottom level. Signed-off-by: glorv <glorvs@163.com> Co-authored-by: glorv <glorvs@163.com>
5 participants
bottom_level_check_range_overlapis used for manual compact to ensure the SST file in the bottom level with a huge range can be force partitioned with manual compaction.