SEZ9 commented on PR #11900:
URL: https://github.com/apache/seatunnel/pull/11900#issuecomment-5643229209
Quick status recap so the remaining asks are in one place:
- **F1 (scope / feature claim)** — Resolved. The PR title and Purpose
section now describe the listener-leak fix; nothing further needed.
- **F3 (test coverage for listener deregistration)** — Partially addressed.
`testShutdownRemovesListeners()` in `JobHistoryServiceTest.java` covers the
direct `shutdown()` path, but two gaps remain:
1. It does not drive `clearCoordinatorService()` with one of the preceding
closes throwing and assert the listeners are still removed — that is the exact
leak scenario this PR fixes.
2. It calls `shutdown()` on
`server.getCoordinatorService().getJobHistoryService()`, the shared
class-scoped instance the other tests in that file rely on, so later tests can
observe an already-torn-down service depending on execution order. Please use
an independently constructed `JobHistoryService` for this test instead.
Still outstanding: the throw-scenario test, the independent test instance,
and the overlap question with the related PR raised in the last round. Happy to
do a fresh pass once those land.
<!-- streview-comment:996 -->
--
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]
