Make flush create its own reference of MemTableListVersion #12996
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.
This extra ref is redundant now but we need it for when data replacement can happen. This PR also changes the contract for cancelling a FlushJob and adds some TODOs for the flush code path for implementing atomic replacement.
FlushJob's cancellation is for properly releasing the resources it is holding. In this PR one more resource namely the
MemTableListVersion
is added into FlushJob, so I want to make sure it's properly handled. Currently, the caller of FlushJob's public APIs use a side by side boolean to track the need for cancellation on their side. That is a contract that cannot be enforced. For example, flush_job_test.cc is not doing that at all. Besides, this is also a complicated contract. There is setting that boolean to true / false in some conditions, checking it in combination with returned Status etc. That makes the contract error prone. There is also no enforcement that resources are guaranteed to be cleaned up.In this PR, we added a required output variable
need_cleanup
to FlushJob's public APIs. The contract is: the user always need to callFlushJob::Cleanup
as long as that boolean is set to true. Although this is yet one more output argument,FlushJob::Run
already have a few other boolean output arguments of the same pattern so it's not too much extra cognitive burden. FlushJob maintains its own internal state about resource management, and a few extra guards are added in debug mode.Test plan:
Existing tests