> On May 15, 2014, 8:47 p.m., Jay Kreps wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java,
> > line 210
> > <https://reviews.apache.org/r/21398/diff/4/?file=582079#file582079line210>
> >
> > Is Node.UNKNOWN better than null?
> >
> > Also, please avoid single line statements like
> >
> > if(x) y;
> >
The original reason that I thought to not use null is because due to Node.equal
function, null != null, and List.contains use equal function. But then when I
check ArrayList.contains implementation it is actually fine (it used indexOf
function):
-------------
public int indexOf(Object o) {
if (o == null) {
for (int i = 0; i < size; i++)
if (elementData[i]==null)
return i;
} else {
for (int i = 0; i < size; i++)
if (o.equals(elementData[i]))
return i;
}
return -1;
}
> On May 15, 2014, 8:47 p.m., Jay Kreps wrote:
> > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java,
> > line 211
> > <https://reviews.apache.org/r/21398/diff/4/?file=582079#file582079line211>
> >
> > Linear search doing a full .equals comparison on each Node object.
Get it, I think we can use a hashset in this case then.
- Guozhang
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21398/#review43157
-----------------------------------------------------------
On May 14, 2014, 11:28 p.m., Guozhang Wang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21398/
> -----------------------------------------------------------
>
> (Updated May 14, 2014, 11:28 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1445
> https://issues.apache.org/jira/browse/KAFKA-1445
>
>
> Repository: kafka
>
>
> Description
> -------
>
> 0. Add the partitionsForNode index in Cluster;\n 1. Ready would return a list
> of ready nodes instead of partitions;\n 2. Ready nodes may contain a Unknon
> node placeholder, and processReadNodes would force metadata update in this
> case;\n 3. Drain would take a list of nodes and drain the batches per node
> until the max request size is reached;\n 4. Collocate would not be just
> tranform batches per node into a producer request;\n 5. Corresponding unit
> test changes; \n 6. One minor compilation warning fix
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java
> a6423f4b37a57f0290e2048b764de1218470f4f7
> clients/src/main/java/org/apache/kafka/clients/producer/MockProducer.java
> 6a0f3b27f754d340aa133218204470a822d4d747
>
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Metadata.java
> f114ffd84c502b6c4070f77d2f6a47ef478b30aa
>
> clients/src/main/java/org/apache/kafka/clients/producer/internals/Partitioner.java
> fbb732a57522109ac0e0eaf5c87b50cbd3a7f767
>
> 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/main/java/org/apache/kafka/common/Cluster.java
> 426bd1eec708979149cbd6fa3959e6f9e73c7e0e
> clients/src/main/java/org/apache/kafka/common/Node.java
> 0e47ff3ff0e055823ec5a5aa4839d25b0fac8374
>
> 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/21398/diff/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Guozhang Wang
>
>
