Repository navigation
[core] Release lookup merge spills when closing sort readers - #10977
Draft
discivigour wants to merge 1 commit into
Draft
discivigour wants to merge 1 commit into
discivigour wants to merge 1 commit into
Conversation
Reset the merge wrapper when either sort reader closes so the last key releases its spill files. Preserve input close failures when resetting the wrapper also fails.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Purpose
A lookup merge can spill records for the current key to local disk. Moving to the next key resets the merge state and releases those files. If the reader closes while processing a key, or immediately after it returns the last key, the spill files can survive until the IO manager shuts down.
Reset the merge function wrapper when closing both the loser-tree and min-heap readers. If closing an input fails, still attempt the reset and preserve the input exception, with any reset failure attached as a suppressed exception.
This fix was split out of #10867 because existing lookup merge readers can also leave spill files behind.
Tests
SortMergeReaderCleanupTestreproduces the bug with both sort engines. It forces a real spill, closes the reader while the IO manager is still open, and verifies that the files have been removed and the returned value remains readable. A second case checks the exception and cleanup behavior when closing the input fails. All four parameterized cases failed before the fix and passed with it.