[jira] [Commented] (KAFKA-385) RequestPurgatory enhancements - expire/checkSatisfy issue; add jmx beans

["Jun Rao (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/KAFKA-385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13431475#comment-13431475 ] 

Jun Rao commented on KAFKA-385:
-------------------------------

Thanks for the patch. Overall, it seems that metrics-core is easy to use. Some comments:

1. KafkaMetricsGroup.metricName: add a comment on the following statement and explain what it does
          actualPkg.replaceFirst("""\.""", ".%s.".format(ident))

2. Unused imports: KafkaApis, KafkaConfig

3. Pool.getAndMaybePut(): This seems to force the caller to create a new value object on each call and in most cases, the new object is not needed.

4. FetchRequestKey and ProduceRequestKey: can they be shared?

5. Is it better to put all metrics classes in one package metrics?

6. It's useful to have topic level stats. I am wondering if we can just keep stats at the global and the topic level, but not at topic/partition level. 

7. DelayedProducerRequestMetrics,DelayedFetchRequestMetrics : Can we name the stats consistently? Something like FetchSatisfiedTime and ProduceStatisfiedTime,  FetchSatisfiedRequest and ProduceStatisfiedRequest. Also, there is some overlap with the stats in BrokerTopicStat.

8. ProducerPerformance: Why forcing producer acks to be 2? Shouldn't that come from the command line?

9. The changes and the new files in config/: Are they needed?
                
> RequestPurgatory enhancements - expire/checkSatisfy issue; add jmx beans
> ------------------------------------------------------------------------
>
>                 Key: KAFKA-385
>                 URL: https://issues.apache.org/jira/browse/KAFKA-385
>             Project: Kafka
>          Issue Type: Bug
>            Reporter: Joel Koshy
>            Assignee: Joel Koshy
>             Fix For: 0.8
>
>         Attachments: KAFKA-385-v1.patch, example_dashboard.jpg, graphite_explorer.jpg
>
>
> As discussed in KAFKA-353:
> 1 - There is potential for a client-side race condition in the implementations of expire and checkSatisfied. We can just synchronize on the DelayedItem.
> 2 - Would be good to add jmx beans to facilitate monitoring RequestPurgatory stats.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira