fix(rest): release fetched FileScanTask state to prevent unbounded memory growth - #17691
Open
waterWang wants to merge 3 commits into
Open
fix(rest): release fetched FileScanTask state to prevent unbounded memory growth#17691waterWang wants to merge 3 commits into
waterWang wants to merge 3 commits into
Conversation
singhpk234
reviewed
Aug 18, 2026
singhpk234
left a comment
singhpk234
left a comment
Contributor
There was a problem hiding this comment.
Are you using this in production if yes in what catalog ?
Comment on lines
+132
to
+133
| * unbounded memory growth — the {@code FileScanTask} list is the dominant memory consumer and | ||
| * need not be retained once the client has received it (#17427). |
Contributor
There was a problem hiding this comment.
do we need to add this in java doc ?
Comment on lines
+144
to
+145
| * the last plan task in a chain is fetched, so completed plans don't accumulate indefinitely | ||
| * (#17427). |
Contributor
There was a problem hiding this comment.
do we need to add this in the java doc ?
Comment on lines
+153
to
+161
| int lastHyphen = planTaskKey.lastIndexOf('-'); | ||
| if (lastHyphen < 0) { | ||
| return; | ||
| } | ||
| int secondLastHyphen = planTaskKey.lastIndexOf('-', lastHyphen - 1); | ||
| if (secondLastHyphen < 0) { | ||
| return; | ||
| } | ||
| String planId = planTaskKey.substring(0, secondLastHyphen); |
Contributor
There was a problem hiding this comment.
why not have a regex ? and extract using regex ?
i also think we should add get planId from the planTaskKey in a seperate util method
| import org.junit.jupiter.api.Test; | ||
| import org.mockito.Mockito; | ||
|
|
||
| public class TestInMemoryPlanningState { |
Contributor
There was a problem hiding this comment.
is it possible to add this in existing test for this ?
| List<String> nextPlanTasks = IN_MEMORY_PLANNING_STATE.nextPlanTask(planTask); | ||
|
|
||
| // Release the fetched plan task's state to prevent unbounded memory growth. The FileScanTask | ||
| // list is the dominant memory consumer and need not be retained once served (#17427). When |
Contributor
There was a problem hiding this comment.
does this needs to be in this comments ?
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
2 participants
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.
Problem
The reference implementation of REST server-side scan planning keeps all planned
FileScanTaskobjects in the singletonInMemoryPlanningState, even after clients have successfully fetched every plan task. As a result, repeated successful scans cause retained heap usage to grow approximately linearly with the number of planned files.The state is released only when
cancelPlanningis called, but according to the REST Catalog OpenAPI specification, cancellation is not required after scan tasks have been fetched for every plan task.Fix
InMemoryPlanningState.releasePlanTask(planTaskKey)— removes the fetched plan task's file scan tasks and next-task link from the maps. Called after every successful fetch infetchScanTasks.InMemoryPlanningState.releaseAsyncPlanForTask(planTaskKey)— when the last plan task in a chain is fetched (nextPlanTasksis empty), also removes the async planning state for the owning plan.This mirrors the design intent of the REST Catalog API: successful fetch lifecycles must eventually release fetched task state without requiring an explicit cancellation request.
Testing
Unit tests cover:
releasePlanTaskremoves both file scan tasks and next-task linksreleaseAsyncPlanForTaskremoves async planning stateCloses #17427