You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Manikumar Reddy O <ma...@gmail.com> on 2014/07/23 18:44:39 UTC

Review Request 23858: Patch for KAFKA-1544

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23858/
-----------------------------------------------------------

Review request for kafka.


Bugs: KAFKA-1544
    https://issues.apache.org/jira/browse/KAFKA-1544


Repository: kafka


Description
-------

Implemented Jun's solution to decrease LogCleaner shutdown time during shutdown


Diffs
-----

  core/src/main/scala/kafka/log/LogCleaner.scala 2faa196a4dc612bc634d5ff5f5f275d09073f13b 

Diff: https://reviews.apache.org/r/23858/diff/


Testing
-------


Thanks,

Manikumar Reddy O


Re: Review Request 23858: Patch for KAFKA-1544

Posted by Manikumar Reddy O <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23858/
-----------------------------------------------------------

(Updated July 24, 2014, 3:11 p.m.)


Review request for kafka.


Bugs: KAFKA-1544
    https://issues.apache.org/jira/browse/KAFKA-1544


Repository: kafka


Description (updated)
-------

Used CountDownLatch in CleanerThread to decrease LogCleaner shutdown time during shutdown


Diffs (updated)
-----

  core/src/main/scala/kafka/log/LogCleaner.scala 2faa196a4dc612bc634d5ff5f5f275d09073f13b 

Diff: https://reviews.apache.org/r/23858/diff/


Testing
-------


Thanks,

Manikumar Reddy O


Re: Review Request 23858: Patch for KAFKA-1544

Posted by Jay Kreps <bo...@gmail.com>.

> On July 23, 2014, 4:56 p.m., Jay Kreps wrote:
> > Is this better than using a condition? I was imagining replacing sleep with shutdownCondition.await(backoffMs, TimeUnit.MILLISECONDS) and then in shutdown() we call shutdownCondition.signal().

Actually a condition isn't quite right as there is a subtle race between signal and await. Better would be the same approach but using CountDownLatch.await(backoff, TimeUnit.MS)


- Jay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23858/#review48532
-----------------------------------------------------------


On July 23, 2014, 4:44 p.m., Manikumar Reddy O wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23858/
> -----------------------------------------------------------
> 
> (Updated July 23, 2014, 4:44 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1544
>     https://issues.apache.org/jira/browse/KAFKA-1544
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Implemented Jun's solution to decrease LogCleaner shutdown time during shutdown
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/log/LogCleaner.scala 2faa196a4dc612bc634d5ff5f5f275d09073f13b 
> 
> Diff: https://reviews.apache.org/r/23858/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>


Re: Review Request 23858: Patch for KAFKA-1544

Posted by Manikumar Reddy O <ma...@gmail.com>.

> On July 23, 2014, 4:56 p.m., Jay Kreps wrote:
> > Is this better than using a condition? I was imagining replacing sleep with shutdownCondition.await(backoffMs, TimeUnit.MILLISECONDS) and then in shutdown() we call shutdownCondition.signal().
> 
> Jay Kreps wrote:
>     Actually a condition isn't quite right as there is a subtle race between signal and await. Better would be the same approach but using CountDownLatch.await(backoff, TimeUnit.MS)

Ok..I got your point. I uploaded a new patch which uses CountDownLatch.  For this i need to override shutdown method in CleanerThread.


- Manikumar Reddy


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23858/#review48532
-----------------------------------------------------------


On July 24, 2014, 3:11 p.m., Manikumar Reddy O wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23858/
> -----------------------------------------------------------
> 
> (Updated July 24, 2014, 3:11 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1544
>     https://issues.apache.org/jira/browse/KAFKA-1544
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Used CountDownLatch in CleanerThread to decrease LogCleaner shutdown time during shutdown
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/log/LogCleaner.scala 2faa196a4dc612bc634d5ff5f5f275d09073f13b 
> 
> Diff: https://reviews.apache.org/r/23858/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>


Re: Review Request 23858: Patch for KAFKA-1544

Posted by Jay Kreps <bo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23858/#review48532
-----------------------------------------------------------


Is this better than using a condition? I was imagining replacing sleep with shutdownCondition.await(backoffMs, TimeUnit.MILLISECONDS) and then in shutdown() we call shutdownCondition.signal().

- Jay Kreps


On July 23, 2014, 4:44 p.m., Manikumar Reddy O wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23858/
> -----------------------------------------------------------
> 
> (Updated July 23, 2014, 4:44 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1544
>     https://issues.apache.org/jira/browse/KAFKA-1544
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Implemented Jun's solution to decrease LogCleaner shutdown time during shutdown
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/log/LogCleaner.scala 2faa196a4dc612bc634d5ff5f5f275d09073f13b 
> 
> Diff: https://reviews.apache.org/r/23858/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>