vbhanuchander-lang commented on issue #17427: URL: https://github.com/apache/iceberg/issues/17427#issuecomment-5287770730
Confirmed against current main — this is a real leak on the spec-conformant happy path, not just a cache without eviction. `InMemoryPlanningState.fileScanTasksForPlanTask` (line 112) and `nextPlanTask` (line 121) are plain `get` calls. The only removals are in `cancelPlan` (lines 165-166) and the full `clear()`, and the class is a process-lifetime singleton, so every successful scan adds state that is never reclaimed. What makes it a defect rather than a design choice is the spec. `rest-catalog-open-api.yaml` states that cancellation "is not necessary after `fetchScanTasks` has been used to fetch scan tasks for each plan task". A client that behaves exactly as specified therefore never calls `cancelPlanning`, so the only code path that frees state is the one such a client never takes. There is already a complete fix in #17429 by @Solaris-star, which adds `releasePlanTask` and `releaseAsyncPlanForTask` and calls them from `fetchScanTasks`, with tests. It was **closed by its own author on 2026-08-12 after two weeks with no review** — not rejected on merit. I have commented there with the two points a reviewer would want covered (that fetch becomes non-idempotent, which is spec-defensible but should be stated; and that deriving the planId by counting hyphens in the key is fragile). I have not opened a competing PR. The efficient path here is for a committer to look at #17429. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
