[jira] [Updated] (KAFKA-597) Refactor KafkaScheduler

2012-12-07 Thread Jay Kreps (JIRA)

 [ 
https://issues.apache.org/jira/browse/KAFKA-597?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jay Kreps updated KAFKA-597:


Attachment: KAFKA-597-v4.patch

Patch v4. 
- Rebased
- Makes use of thread factory
- Fixed broken scaladoc

 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


 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


[jira] [Updated] (KAFKA-597) Refactor KafkaScheduler

2012-12-07 Thread Jay Kreps (JIRA)

 [ 
https://issues.apache.org/jira/browse/KAFKA-597?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jay Kreps updated KAFKA-597:


Attachment: KAFKA-597-v5.patch

Thanks, new patch v5 addresses your comments:
- Improved javadoc
- This is actually good. I thought about it a bit and since I am making 
shutdown block the only time daemon vs non-daemon comes into play is if you 
don't call shutdown. If that is the case non-daemon threads will prevent 
garbage collection of the scheduler tasks and eventually block shutdown of the 
jvm, which seems unnecessary.
- The change to shutdownNow is not good. This will invoke interrupt on all 
threads, which is too aggressive. Better to let them finish. If we end up 
needing to schedule long-running tasks we can invent a new notification 
mechanism. I changed this so that we use normal shutdown instead.

 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


[jira] [Updated] (KAFKA-597) Refactor KafkaScheduler

2012-12-03 Thread Jay Kreps (JIRA)

 [ 
https://issues.apache.org/jira/browse/KAFKA-597?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jay Kreps updated KAFKA-597:


Attachment: KAFKA-597-v1.patch

This patch refactors the scheduler as described above except that I didn't 
implement task cancelation.

It also converts LogManagerTest to use the new MockScheduler.

 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


 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