linliu-code opened a new pull request, #20062:
URL: https://github.com/apache/hudi/pull/20062
### Describe the issue this Pull Request addresses
Addresses the narrower of the two defects reported in #20061. It does
**not** fix the main leak
described there (a small fraction of embedded timeline services never have
`close()` attempted at
all) — that one is still unexplained, and this PR deliberately does not
claim it.
`TimelineService.close()`, `EmbeddedTimelineService.stopForBasePath()` and
`EmbeddedTimelineService.shutdownAllTimelineServers()` had no exception
handling. If
`requestHandler.stop()`, `app.stop()` or `fsViewsManager.close()` throws:
- `this.app = null` never runs, so the Javalin server stays both running and
referenced
- in `stopForBasePath`, `this.server = null` never runs either, so every
later call re-enters the
same branch and the instance can **never** be closed — there is no retry
path
- in `shutdownAllTimelineServers`, the throw escapes the `forEach`,
abandoning every remaining
server in the registry and skipping `RUNNING_SERVICES.clear()`
- nothing is logged, so none of this is observable
On a long-running driver this was measured as retained Javalin/Jetty servers
and their
`TimelineService-JettyScheduler` threads accumulating over weeks; see #20061
for the counts.
### Summary and Changelog
Guard each shutdown stage so one failure cannot skip the others, always
release the references,
and log a warning when a stage fails.
- `TimelineService.close()` — each of the three stages wrapped; `app`
released in a `finally`
- `EmbeddedTimelineService.stopForBasePath()` — `server` / `viewManager`
released in a `finally`
- `EmbeddedTimelineService.shutdownAllTimelineServers()` — the sweep
continues past a failing
server, and the metric is decremented in a `finally`
- `TestEmbeddedTimelineService` — two tests added
No code was copied from elsewhere.
### Impact
No public API change and no behaviour change on the success path. On the
failure path a close
that previously threw now logs a warning and completes, leaving the instance
releasable.
Note what this does **not** do: releasing the reference does not guarantee
the underlying Jetty
threads are reclaimed, since a server that failed to stop is still running.
The aim is to remove
the unrecoverable state and make the failure visible — which is also what is
needed to diagnose
the larger leak in #20061, because today that failure emits nothing at all.
### Risk Level
low
The success path is unchanged; the new code only executes where an exception
previously escaped
and aborted shutdown. Verified by reverting the two source files to `master`
and confirming both
new tests fail with the exception escaping at the two previously unguarded
lines, then confirming
all 7 tests in the class pass with the change. Checkstyle clean on both
modules.
### Documentation Update
none
### Contributor's checklist
- [x] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
- [x] CI passed
--
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]
