You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Joel Koshy <jj...@gmail.com> on 2014/04/18 00:33:21 UTC
Review Request 20468: Add log cleaner metrics. (Cherry-picked from trunk)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20468/
-----------------------------------------------------------
Review request for kafka.
Bugs: KAFKA-1380
https://issues.apache.org/jira/browse/KAFKA-1380
Repository: kafka
Description
-------
KAFKA-1327 Add log cleaner metrics.
Diffs
-----
core/src/main/scala/kafka/log/Log.scala b3ab5220a66a2ae82084dad89877daf60f613e66
core/src/main/scala/kafka/log/LogCleaner.scala 312204c6ddd0cf46cd7349d79a43edec839bc361
core/src/main/scala/kafka/log/LogCleanerManager.scala 79e9d556d22c5ecd9be2439e5c54caad9b58735c
core/src/main/scala/kafka/utils/Throttler.scala c6c3c75ee8408ca81aeeb5846f7987a287b5a6e8
Diff: https://reviews.apache.org/r/20468/diff/
Testing
-------
Thanks,
Joel Koshy
Re: Review Request 20468: Add log cleaner metrics. (Cherry-picked from trunk)
Posted by Joel Koshy <jj...@gmail.com>.
> On April 19, 2014, 12:22 a.m., Jun Rao wrote:
> > core/src/main/scala/kafka/log/LogCleanerManager.scala, lines 83-84
> > <https://reviews.apache.org/r/20468/diff/1/?file=561812#file561812line83>
> >
> > It seems that we should set the ratio back to 0 if dirtyLogs is empty. Otherwise, the reported metrics will stay with the last updated value, which can be miss-leading.
Jun, I think you are right on this and the point below. I can follow-up on Monday
- Joel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20468/#review40838
-----------------------------------------------------------
On April 17, 2014, 10:33 p.m., Joel Koshy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20468/
> -----------------------------------------------------------
>
> (Updated April 17, 2014, 10:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1380
> https://issues.apache.org/jira/browse/KAFKA-1380
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1327 Add log cleaner metrics.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala b3ab5220a66a2ae82084dad89877daf60f613e66
> core/src/main/scala/kafka/log/LogCleaner.scala 312204c6ddd0cf46cd7349d79a43edec839bc361
> core/src/main/scala/kafka/log/LogCleanerManager.scala 79e9d556d22c5ecd9be2439e5c54caad9b58735c
> core/src/main/scala/kafka/utils/Throttler.scala c6c3c75ee8408ca81aeeb5846f7987a287b5a6e8
>
> Diff: https://reviews.apache.org/r/20468/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joel Koshy
>
>
Re: Review Request 20468: Add log cleaner metrics. (Cherry-picked from trunk)
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20468/#review40838
-----------------------------------------------------------
Ship it!
core/src/main/scala/kafka/log/LogCleanerManager.scala
<https://reviews.apache.org/r/20468/#comment74050>
It seems that we should set the ratio back to 0 if dirtyLogs is empty. Otherwise, the reported metrics will stay with the last updated value, which can be miss-leading.
- Jun Rao
On April 17, 2014, 10:33 p.m., Joel Koshy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20468/
> -----------------------------------------------------------
>
> (Updated April 17, 2014, 10:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1380
> https://issues.apache.org/jira/browse/KAFKA-1380
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1327 Add log cleaner metrics.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala b3ab5220a66a2ae82084dad89877daf60f613e66
> core/src/main/scala/kafka/log/LogCleaner.scala 312204c6ddd0cf46cd7349d79a43edec839bc361
> core/src/main/scala/kafka/log/LogCleanerManager.scala 79e9d556d22c5ecd9be2439e5c54caad9b58735c
> core/src/main/scala/kafka/utils/Throttler.scala c6c3c75ee8408ca81aeeb5846f7987a287b5a6e8
>
> Diff: https://reviews.apache.org/r/20468/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joel Koshy
>
>
Re: Review Request 20468: Add log cleaner metrics. (Cherry-picked from trunk)
Posted by Joel Koshy <jj...@gmail.com>.
> On April 19, 2014, 12:42 a.m., Jun Rao wrote:
> > Also, I think we need to make the following var volatile.
> >
> > CleanerStats.startTime, mapCompleteTime, endTime, bytesRead, bytesWritten, mapBytesRead, mapMessagesRead, messagesRead, messagesWritten
> >
> > LogCleanerManager.dirtiestLogCleanableRatio
> >
> > Jay,
> >
> > Could you double check?
I think only dirtiestLogCleanable should be volatile. The others are fine since lastCleanerStats is volatile and metrics are reported only off lastCleanerstats.
- Joel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20468/#review40840
-----------------------------------------------------------
On April 17, 2014, 10:33 p.m., Joel Koshy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20468/
> -----------------------------------------------------------
>
> (Updated April 17, 2014, 10:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1380
> https://issues.apache.org/jira/browse/KAFKA-1380
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1327 Add log cleaner metrics.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala b3ab5220a66a2ae82084dad89877daf60f613e66
> core/src/main/scala/kafka/log/LogCleaner.scala 312204c6ddd0cf46cd7349d79a43edec839bc361
> core/src/main/scala/kafka/log/LogCleanerManager.scala 79e9d556d22c5ecd9be2439e5c54caad9b58735c
> core/src/main/scala/kafka/utils/Throttler.scala c6c3c75ee8408ca81aeeb5846f7987a287b5a6e8
>
> Diff: https://reviews.apache.org/r/20468/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joel Koshy
>
>
Re: Review Request 20468: Add log cleaner metrics. (Cherry-picked from trunk)
Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20468/#review40840
-----------------------------------------------------------
Also, I think we need to make the following var volatile.
CleanerStats.startTime, mapCompleteTime, endTime, bytesRead, bytesWritten, mapBytesRead, mapMessagesRead, messagesRead, messagesWritten
LogCleanerManager.dirtiestLogCleanableRatio
Jay,
Could you double check?
- Jun Rao
On April 17, 2014, 10:33 p.m., Joel Koshy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20468/
> -----------------------------------------------------------
>
> (Updated April 17, 2014, 10:33 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1380
> https://issues.apache.org/jira/browse/KAFKA-1380
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1327 Add log cleaner metrics.
>
>
> Diffs
> -----
>
> core/src/main/scala/kafka/log/Log.scala b3ab5220a66a2ae82084dad89877daf60f613e66
> core/src/main/scala/kafka/log/LogCleaner.scala 312204c6ddd0cf46cd7349d79a43edec839bc361
> core/src/main/scala/kafka/log/LogCleanerManager.scala 79e9d556d22c5ecd9be2439e5c54caad9b58735c
> core/src/main/scala/kafka/utils/Throttler.scala c6c3c75ee8408ca81aeeb5846f7987a287b5a6e8
>
> Diff: https://reviews.apache.org/r/20468/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Joel Koshy
>
>