----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20050/#review40433 -----------------------------------------------------------
The first question is whether we should we centralize all the metrics into a ProducerMetrics class. Even if we do this, though, this patch seems to try to accomplish this with a class of all static methods/variables. This is definitely, definitely not the right way to go. Among other things I don't see how this would work if you have multiple producers in the same JVM. With respect to centralizing the metrics, I think this is not what we want to do. The whole point of this metrics design is to allow a cross-cutting concern like metrics to appear all over the code without tying everything together with a single class with all the metrics logic. In other words a lower-level package like Selector can have metrics without implicitly depending on the rest of the producer. I think there are other ways to solve the registration problem we were seeing... - Jay Kreps On April 11, 2014, 9:20 p.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20050/ > ----------------------------------------------------------- > > (Updated April 11, 2014, 9:20 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1359 > https://issues.apache.org/jira/browse/KAFKA-1359 > > > Repository: kafka > > > Description > ------- > > minor change in SenderTest > > > remove un-used imports > > > move registration to centralized ProducerMetrics; fixed a small bug in > SenderTest > > > KAFKA-1359.v2 > > > KAFKA-1359.v1 > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > a6423f4b37a57f0290e2048b764de1218470f4f7 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java > ffd13ff00ba0d0d969e2ed130e49053f02481b50 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > 855ae84f14aa91653b3fa855c2af40560323f42a > clients/src/main/java/org/apache/kafka/common/ProducerMetrics.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/network/Selector.java > 558f8b47638b354f9c1a30be5d45dc7b61131bea > clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java > a2b77226f8c58caf632a0f4665bd4e4cd93e643d > clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java > 5c5e3d40819e41cab7b52a0eeaee5f2e7317b7b3 > > Diff: https://reviews.apache.org/r/20050/diff/ > > > Testing > ------- > > > Thanks, > > Guozhang Wang > >
