Jackie-Jiang opened a new pull request, #19260:
URL: https://github.com/apache/pinot/pull/19260
## Summary
`PinotTaskManager` runs its per-table cron jobs on a Quartz scheduler of its
own, created in the constructor and
started in `init()`. Nothing ever shut it down.
This is easy to miss because there are two Quartz schedulers in play.
`PeriodicTaskScheduler` owns one and shuts it
down correctly in its `stop()`. The one `PinotTaskManager` owns is separate,
and `cleanUpTask()` — the only shutdown
hook it participates in — just cleans up the task generators. So it kept
firing after `stopPinotController()` had
closed the ZkClient through `_helixResourceManager.stop()`:
```
ERROR [JobRunShell] Job ... threw an unhandled Exception:
java.lang.IllegalStateException: ZkClient already closed!
at
org.apache.helix.zookeeper.zkclient.ZkClient.retryUntilConnected(ZkClient.java:2166)
...
```
Quartz's `SimpleThreadPool` workers are not daemon threads either, so they
outlived every controller stop. That
accumulates across controller restarts inside a single JVM, which is what
integration tests do.
## Approach
`stopScheduler()` is added as the explicit counterpart to `init()`, and
called from `stopPinotController()` right
after the periodic task scheduler stops — well before Helix is torn down.
It deliberately does *not* hang off `cleanUpTask()`, which would look like
the natural hook. `cleanUpTask()` is also
used as a plain "reset the generators" operation on a live controller — see
`RealtimeToOfflineSegmentsMinionClusterIntegrationTest` — so attaching an
irreversible scheduler shutdown to it would
be a trap for the next caller that does the same.
`shutdown(true)` waits for the jobs already in flight, so a task generation
in progress is not cut off midway. This
matches what `PeriodicTaskScheduler` does for its own scheduler.
`stopScheduler()` is a no-op when the cron scheduler is disabled, which is
the default
(`controller.task.scheduler.enabled`), so nothing changes for controllers
that never start one.
## Coverage
`PinotTaskManagerStatelessTest#testSchedulerShutDownWithController` starts a
controller with the scheduler enabled,
stops it, and asserts the scheduler is shut down. It fails on `master` and
passes with this change.
--
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]
