> On Feb. 13, 2014, 10:54 p.m., Joel Koshy wrote: > > core/src/main/scala/kafka/utils/Utils.scala, line 545 > > <https://reviews.apache.org/r/16718/diff/6/?file=484571#file484571line545> > > > > Thanks for the comment. However, I still don't get it. Why are JSON > > strings > > relevant here? i.e., we are ultimately dealing with a Java regex in the > > TopicFilter. I'm sure your patch works (as verified by Jason), but I > > can't > > say I understand this. > > > > Joe Stein wrote: > It is because the regex is used from zookeeper after it is saved as JSON > and parsed as JSON for use. e.g. during rebalance from the > TopicCount.constructTopicCount > > If we set Whitelist("test-(?!bad\\b)[\\w]+") and then consume, when > consumer starts it has the regex correct and creates the PartitionTopicInfo > for the topic "test-good" which we expect (with an instantiated chunkQueue: > BlockingQueue[FetchedDataChunk] for the topic "test-good" and filters out the > "test-bad" topic properly)). > > The regex patern is then stored in Zookeeper as JSON. From there on the > regex is not correct when retrieved and as parsed. So it is missing from the > partitionMap in ConsumerFetcherThread (correctly so) but it is still in the > topicAndPartition of processPartitionData in ConsumerFetcherThread (because > of this issue). > > So you get the scenario of the ConsumerFetcherThread trying to enque > partitionData for the topic "test-bad" which it doesn't have an instantiated > chunkQueue: BlockingQueue[FetchedDataChunk] for the PartitionTopicInfo (nor > should it). >
Got it - thanks for the explanation. > On Feb. 13, 2014, 10:54 p.m., Joel Koshy wrote: > > core/src/test/scala/unit/kafka/consumer/TopicFilterTest.scala, line 51 > > <https://reviews.apache.org/r/16718/diff/6/?file=484572#file484572line51> > > > > Do we actually need this test? i.e., my thinking was that we could just > > add > > the escaped cases to the whitelist/blacklist tests above - including the > > specific failure case that Jason reported. > > > > Joe Stein wrote: > I thought it made sense since the issue was use of this function is where > the problem was stemming from True - although I felt it may be clearer to (implicitly) exercise that logic through real whitelist/blacklist tests such as the one that was reported in the ticket. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16718/#review34433 ----------------------------------------------------------- On Feb. 13, 2014, 8:24 p.m., Joe Stein wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16718/ > ----------------------------------------------------------- > > (Updated Feb. 13, 2014, 8:24 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1180 > https://issues.apache.org/jira/browse/KAFKA-1180 > > > Repository: kafka > > > Description > ------- > > KAFKA-1180 WhiteList topic filter gets a NullPointerException on complex > Regex and added Blacklist test that was missing > > > Diffs > ----- > > core/src/main/scala/kafka/consumer/TopicCount.scala > e33263378489f7cb5ba98476a8e6d65640130965 > core/src/main/scala/kafka/utils/Utils.scala > a89b0463685e6224d263bc9177075e1bb6b93d04 > core/src/test/scala/unit/kafka/consumer/TopicFilterTest.scala > cf2724bb68d39256f033687c25cde24c667c3d8d > > Diff: https://reviews.apache.org/r/16718/diff/ > > > Testing > ------- > > > Thanks, > > Joe Stein > >
