Support Non-Blocking Manual Compactions (CompactRange) (#597)#656
Conversation
|
@udi-speedb please add example file how to use the new non blocking compaction feature |
|
please use port::Thread and not std::thread for spawning threads, so the on thread start callback will work on those threads |
Done. Please just note that I have made the ThreadWithCb::joinable() a const method (same as std::thread::joinable()) (I am calling this method from a const method of mine). |
udi-speedb
force-pushed
the
597-make-compactrange-non-blocking
branch
from
ad48fb2 to
9a686a1
Compare
Done |
udi-speedb
force-pushed
the
597-make-compactrange-non-blocking
branch
from
980510e to
2c93929
Compare
|
@udi-speedb add a unit test for closing the db before compact range has been completed |
As we have discussed, it will be the user's responsibility to wait for all non-blocking CompactRange() calls to have their respective callbacks called. Thereafter, the Close() shouldn't have any issues joining the internal threads. |
| using ValidateCompletionStatusFunc = | ||
| std::function<void(ROCKSDB_NAMESPACE::Status completion_status)>; | ||
|
|
||
| void DefaultCompletionStatusValidation(Status completion_status, |
There was a problem hiding this comment.
above this line you added the ROCKSDB_NAMESPACE:: , please use it here or remove it from the other places.
There was a problem hiding this comment.
I have removed in new code that I have added, however, there are many places in the db_compaction_test.cc that use "ROCKSDB_NAMESPACE::" which I will not touch to avoid unnecessary code changes that are irrelevant to this PR.
| bool exclusive_manual_compaction_; | ||
| }; | ||
|
|
||
| class DBCompactionTestWithParamWithMCC |
There was a problem hiding this comment.
Manual Compaction Control
| threads_.push_back(std::move(thread)); | ||
| } | ||
|
|
||
| void CompactRangeThreadsMngr::CleanupCompletedThreads() { |
There was a problem hiding this comment.
I think joinable() will always return true, because we have never called join().
|
|
||
| // Wait for all non-blocking manual compactions that may still be in progress. | ||
| // Do it only after cleaning up all compaction-related activity above. | ||
| compact_range_threads_mngr_.Shutdown(); |
| // The number of LockWAL called without matching UnlockWAL call. | ||
| // See also lock_wal_write_token_ | ||
| uint32_t lock_wal_count_; | ||
|
|
There was a problem hiding this comment.
maybe add a small comment here about the manual/range compaction thread manager
| } else { | ||
| fprintf(stderr, | ||
| "Non-Blocking CompactRange() Didn't Complete Successfuly in " | ||
| "Time: %d\n", |
There was a problem hiding this comment.
Maybe print the status message?
There was a problem hiding this comment.
This is std::future::future_status which is an enum, what status message should I print?
|
|
||
| // An optional completion callback to allow for non-blocking (async) operation | ||
| // Default: Empty (Blocking) | ||
| std::shared_ptr<CompactRangeCompletedCbIf> async_completion_cb; |
There was a problem hiding this comment.
async_completion_cb, maybe use another name, what if we will want to add Async PUT for example?
|
|
||
| ThreadWithCb() {} | ||
| bool joinable() { return thread_.joinable(); } | ||
| bool joinable() const { return thread_.joinable(); } |
There was a problem hiding this comment.
I have removed this file
ofriedma
left a comment
ofriedma
left a comment
There was a problem hiding this comment.
before merge please add comments
|
|
||
| } // namespace | ||
|
|
||
| class DBCompactionTestWithMCC : public DBCompactionTest, |
There was a problem hiding this comment.
Please add a note what is MCC.
| // The number of LockWAL called without matching UnlockWAL call. | ||
| // See also lock_wal_write_token_ | ||
| uint32_t lock_wal_count_; | ||
|
|
Add a new option to CompactRangeOptions, a callback object. By defaut, the callback is empty so CompactRange() will be blocking (as it is now). If the callback is set, the callback object's callback method will be called upon completion with the completion status.
udi-speedb
force-pushed
the
597-make-compactrange-non-blocking
branch
from
129e3d6 to
3cf02ac
Compare
Add a new option to CompactRangeOptions, a callback object. By default, the callback is empty so CompactRange() will be blocking (as it is now). If the callback is set, the callback object's callback method will be called upon completion with the completion status.
Add a new option to CompactRangeOptions, a callback object. By default, the callback is empty so CompactRange() will be blocking (as it is now). If the callback is set, the callback object's callback method will be called upon completion with the completion status.
3 participants
Add a new option to CompactRangeOptions, a callback object. By defaut, the callback is empty so CompactRange() will be blocking (as it is now). If the callback is set, the callback object's callback method will be called upon completion with the completion status.