[
https://issues.apache.org/jira/browse/KAFKA-597?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13527084#comment-13527084
]
Joel Koshy commented on KAFKA-597:
----------------------------------
On daemon vs non-daemon and shutdown vs shutdownNow. I may be misunderstanding
the javadoc but I think: since the
default is now daemon=true and you switched to use shutdown, VM shutdown can
continue even in the middle of a
scheduled task like checkpointing high watermarks or cleaning up logs. i.e.,
there may be such scenarios where it makes
sense to make them non-daemon - i.e., set it as a non-daemon, and use shutdown
(not shutdownNow - or use
shutdownNow and handle InterruptedException properly in the task) to let them
finish gracefully. Otherwise (iiuc)
it seems if we call shutdown on the executor the VM could exit and simply kill
(i.e., abruptly terminate) any running
task that was started by the executor in one of the (daemon) threads from its
pool.
Minor comments:
- Line 81 of KafkaScheduler: closing brace is mis-aligned.
- The scaladoc on MockScheduler uses a non-existent schedule variant - i.e., I
think you intended to add a period < 0
no?
> Refactor KafkaScheduler
> -----------------------
>
> Key: KAFKA-597
> URL: https://issues.apache.org/jira/browse/KAFKA-597
> Project: Kafka
> Issue Type: Bug
> Affects Versions: 0.8.1
> Reporter: Jay Kreps
> Priority: Minor
> Attachments: KAFKA-597-v1.patch, KAFKA-597-v2.patch,
> KAFKA-597-v3.patch, KAFKA-597-v4.patch, KAFKA-597-v5.patch
>
>
> It would be nice to cleanup KafkaScheduler. Here is what I am thinking
> Extract the following interface:
> trait Scheduler {
> def startup()
> def schedule(fun: () => Unit, name: String, delayMs: Long = 0, periodMs:
> Long): Scheduled
> def shutdown(interrupt: Boolean = false)
> }
> class Scheduled {
> def lastExecution: Long
> def cancel()
> }
> We would have two implementations, KafkaScheduler and MockScheduler.
> KafkaScheduler would be a wrapper for ScheduledThreadPoolExecutor.
> MockScheduler would only allow manual time advancement rather than using the
> system clock, we would switch unit tests over to this.
> This change would be different from the existing scheduler in a the following
> ways:
> 1. Would not return a ScheduledFuture (since this is useless)
> 2. shutdown() would be a blocking call. The current shutdown calls, don't
> really do what people want.
> 3. We would remove the daemon thread flag, as I don't think it works.
> 4. It returns an object which let's you cancel the job or get the last
> execution time.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
