----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21304/#review42734 -----------------------------------------------------------
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java <https://reviews.apache.org/r/21304/#comment76592> Does this need to be volatile? Do we actually need it? It is a little hard to know how to interpret it... clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java <https://reviews.apache.org/r/21304/#comment76593> It seems like this only works some of the time. Let's say you have two partitions F and N where F is full and N has a message but isn't full. If you happen to process F then N, N will carpool, but if you process N then F it won't. So you only get 50% of the carpooling you should (on avg). Perhaps I am misunderstanding? clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java <https://reviews.apache.org/r/21304/#comment76594> This linear search may be a bit problematic for large partition counts, no? Set will be slower in the common case but faster in the extreme case. Either way we should probably just keep the node id so the comparison is just of a boxed integer rather than a compound object. - Jay Kreps On May 10, 2014, 9:38 p.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21304/ > ----------------------------------------------------------- > > (Updated May 10, 2014, 9:38 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1445 > https://issues.apache.org/jira/browse/KAFKA-1445 > > > Repository: kafka > > > Description > ------- > > Mark a partition as ready if some other partitions with the same destination > is also ready so it can take a carpool; fix a unchecked or unsafe operations > warning > > > Diffs > ----- > > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java > 2d7e52d430fa267ee3689a06f8a621ce5dfd1e33 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > f0152fabbdd44e9f1a24291e84c17edf8379f4fc > > clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java > f37ab770b1794830154f9908a0156e7e99b4a458 > > clients/src/test/java/org/apache/kafka/common/utils/AbstractIteratorTest.java > 1df226606fad29da47d81d0b8ff36209c3536c06 > > Diff: https://reviews.apache.org/r/21304/diff/ > > > Testing > ------- > > > Thanks, > > Guozhang Wang > >
