smengcl opened a new pull request, #10723: URL: https://github.com/apache/ozone/pull/10723
Generated-by: Claude Code (Opus 4.8) ## What is the problem https://issues.apache.org/jira/browse/HDDS-15826 Recon serves disk-usage (`/du`), file-size-distribution, and directory-count views by walking the NSSummary directory tree. Three code paths do this walk recursively: `EntityHandler.getTotalFileSizeDist`, `EntityHandler.getTotalDirCount`, and `ReconUtils.gatherSubPaths` (used when gathering open-file subpaths). Each recurses over `NSSummary.getChildDir()` with no protection against a malformed tree. If the persisted NSSummary data is corrupted so that a directory lists itself, or one of its ancestors, as a child, the child set forms a cycle. The recursion then never terminates and Recon dies with a `StackOverflowError`. On the read path this surfaces as an HTTP 500 from `/du` and, in the worst case, takes the Recon process down. Because the corruption lives in persisted state, the failure repeats on every request until the underlying data is rebuilt. The recursion also had no upper bound on depth even for a non-cyclic but pathologically deep tree. ## What this changes Convert all three walks from recursion to iterative traversal backed by an explicit stack and a `visited` set. Each object is visited, and counted, at most once, so a self-referencing or cyclic child can no longer drive infinite recursion or inflate the result. For a well-formed tree the results are identical to before. Two smaller improvements ride along in the same paths: 1. `gatherSubPaths` previously fetched each node's NSSummary twice (once to null-check a child before descending, once when the child was itself expanded). It now fetches once, on pop, which halves the number of `getNSSummary` lookups on large trees. 2. De-duplicating a corrupted tree silently would hide a real data problem from an observability tool. Each walk now emits a single WARN per request, with the path or object context, the first time it detects a repeated reference, so operators can see that NSSummary data is corrupted rather than just receiving a quietly wrong number. ## What this does not change This is read-side hardening only. It does not attempt to prevent the corruption at write time, and it does not add response paging or a size limit to `/du`. Bounding the `/du` response for very large (but well-formed) buckets is a separate concern and is left as a follow-up. ## How it was tested - New unit test `TestEntityHandlerCycleGuard` covers a self-referencing directory, a multi-node cycle with a back edge and a self loop, and a clean tree (to confirm unchanged counts) for both `EntityHandler` walks. Each case runs under a timeout so a regression back to infinite recursion fails fast. - `TestReconUtils` gains a cyclic-tree case for `gatherSubPaths`. - All new tests use mocks, so they add negligible CI time. - Existing `TestNSSummaryEndpointWithFSO` (28 tests) passes unchanged, confirming clean-tree behavior is preserved end to end. - `checkstyle.sh` for the recon module is clean. -- 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]
