Clear stale data using DeleteFilesInRanges #2731
Conversation
src/util/rocksdb/mod.rs
Outdated
| for cf in db.cf_names() { | ||
| let mut delete_ranges = Vec::new(); | ||
| for range in ranges { | ||
| assert!(range.0 <= range.1); |
There was a problem hiding this comment.
I think we should skip it when range.start == range.end.
src/util/rocksdb/mod.rs
Outdated
| let handle = get_cf_handle(db, cf)?; | ||
| db.delete_file_in_range_cf(handle, start_key, &end)?; | ||
| db.delete_files_in_ranges_cf(handle, &delete_ranges, /* include_end */ false)?; |
There was a problem hiding this comment.
Please check if delete_ranges empty before involve delete_files_in_ranges_cf.
BTW, we only need to calculate ranges once and reuse it for each cf.
src/raftstore/store/store.rs
Outdated
| } | ||
|
|
||
| #[test] | ||
| fn test_clear_stale_data() { |
There was a problem hiding this comment.
Can you test it in integration tests?
There was a problem hiding this comment.
I need to create a fake raft store here, seems I can't do that in integration tests?
There was a problem hiding this comment.
There is a raftstore node in integration tests already. You can just new a single node cluster and do whatever you want.
src/util/rocksdb/mod.rs
Outdated
| )); | ||
| pub fn roughly_cleanup_ranges(db: &DB, ranges: &[(Vec<u8>, Vec<u8>)]) -> Result<(), String> { | ||
| let mut delete_ranges = Vec::new(); | ||
| for range in ranges { |
There was a problem hiding this comment.
Why not use (ref start_key, ref end_key) in ranges?
|
Any updates? |
|
Working on the test. |
|
@BusyJay PTAL |
|
/run-all-tests |
huachaohuang
dismissed stale reviews from BusyJay and zhangjinpeng87
via
ee6ed9f
|
/run-all-tests |
4 similar comments
|
/run-all-tests |
|
/run-all-tests |
|
/run-all-tests |
|
/run-all-tests |
|
/run-all-tests |
|
/run-all-tests |
|
/run-all-tests |
|
OMG, all checks have passed, anyone seeing this please approve and merge this for me :) |
Optimize
clear_stale_databy deleting files in multiple ranges at once.