Core: Track & close FileIO used for remote scan planning#15439
Open
nastra wants to merge 1 commit intoapache:mainfrom
Open
Core: Track & close FileIO used for remote scan planning#15439nastra wants to merge 1 commit intoapache:mainfrom
nastra wants to merge 1 commit intoapache:mainfrom
Conversation
singhpk234
reviewed
Feb 25, 2026
| } | ||
|
|
||
| /** Cancels the plan on the server (if supported) and closes the plan-scoped FileIO */ | ||
| private void cleanupPlanResources() { |
Contributor
There was a problem hiding this comment.
may be reverse would be nice in a sense when i cancel plan then i call cleanUpResource, we need to kind of also check the close of the streaming iterator also cancels( which i think it already does) along with the closing the file IO
Contributor
Author
There was a problem hiding this comment.
we need to kind of also check the close of the streaming iterator also cancels( which i think it already does) along with the closing the file IO
Closing the fileIOForPlanId in
fileIOForPlanId only in the two other places when exceptions are happening.
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.
This uses a similar approach to what we do in the
RESTSessionCatalogwith theFileIOTrackerby wrapping theRESTTableScanin aWeakReferenceand close the attachedFileIOinstance when theRESTTableScanobject is garbage-collected. We don't really haveclose()methods on eitherTableorScan, so I can't think of an easier way to keep track ofFileIOinstances created insideRESTTableScanso that we properly close them before actually reaching the finalizer inS3FileIO, which would then complain as can be seen below:I have verified that this works in combination of #15368, where the
fileIOForPlanIdwasn't previously closed properly and was running into the WARN shown above.I'm currently checking to see how to add a unit test for this to
TestRESTScanPlanningbut I think this is non-trivial (similar to how it was non-trivial to test this behavior in #7487)