----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36871/#review93333 -----------------------------------------------------------
clients/src/main/java/org/apache/kafka/clients/consumer/internals/SubscriptionState.java (line 87) <https://reviews.apache.org/r/36871/#comment147710> Sorry for being late to the party (I have been sick for 3 days now). Well, what I gonna say is more a musing than a a review comment: why not replace the vanilla (and non thread safe HashSet) from assignedPartitions, and similar fields, by a thread safe Set? This can be done either by letting assignedPartitions be ''CopyOnWriteArraySet'' (has to take care with the computational complexity of adding/removing in this collection) or ''ConcurrentSkipListSet''? IMHO, making a defensive copy here is nice for this particular case, but it still leave room for concurrent exceptions in the future. Just a musing though. I am fine with leaving as it-is now. :) - Edward Ribeiro On July 28, 2015, 4:17 p.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36871/ > ----------------------------------------------------------- > > (Updated July 28, 2015, 4:17 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2381 > https://issues.apache.org/jira/browse/KAFKA-2381 > > > Repository: kafka > > > Description > ------- > > Address review comment > > > Move closing of consumer to finally > > > Add a more specific unit test > > > KAFKA-2381: Possible ConcurrentModificationException while unsubscribing from > a topic in new consumer > > > Diffs > ----- > > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/SubscriptionState.java > 4d9a425201115a66b457b58d670992b279091f5a > > clients/src/test/java/org/apache/kafka/clients/consumer/internals/SubscriptionStateTest.java > 319751c374ccdc7e7d7d74bcd01bc279b1bdb26e > core/src/test/scala/integration/kafka/api/ConsumerTest.scala > 3eb5f95731a3f06f662b334ab2b3d0ad7fa9e1ca > > Diff: https://reviews.apache.org/r/36871/diff/ > > > Testing > ------- > > > Thanks, > > Ashish Singh > >
