> On March 24, 2015, 10:46 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/server/AbstractFetcherThread.scala, lines 81-86
> > <https://reviews.apache.org/r/31366/diff/2/?file=898415#file898415line81>
> >
> >     Jun has a comment about the case when all partitions gets inactive, 
> > which is common when the fetched broker has been just gone through leader 
> > migration.
> >     
> >     We can move the foreach statement before the if statement, and after 
> > foreach check if any partitions gets added, if not just backoff for 
> > fetchBackoffMs.
> 
> Sriharsha Chintalapani wrote:
>     Thanks for the review. Are you looking at something like this. This 
> wouldn't handle if we have partitionMap populated but all of them are 
> inactive.
>     
>           partitionMap.foreach {
>             case((topicAndPartition, partitionFetchState)) =>
>               if(partitionFetchState.isActive)
>                 fetchRequestBuilder.addFetch(topicAndPartition.topic, 
> topicAndPartition.partition,
>                   partitionFetchState.offset, fetchSize)
>           }
>           if (partitionMap.isEmpty)
>             partitionMapCond.await(fetchBackOffMs, TimeUnit.MILLISECONDS)
>     or do we want to check if all the currentParttions are inactive and than 
> backoff? that would be expensive to check if all the partitions or active or 
> not in dowork.

What I think is a bit different and maybe simpler:

For FetchRequestBuilder, in the build() call its requestMap will be cleared 
after the fetch request is created, so we can just add another function in 
FetchRequestBuilder return boolean indicating if its request map is empty. With 
this we can get rid of the allPartitionsInactive flag.


- Guozhang


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31366/#review77674
-----------------------------------------------------------


On April 4, 2015, 3:48 a.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31366/
> -----------------------------------------------------------
> 
> (Updated April 4, 2015, 3:48 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1461
>     https://issues.apache.org/jira/browse/KAFKA-1461
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1461. Replica fetcher thread does not implement any back-off behavior.
> 
> 
> KAFKA-1461. Replica fetcher thread does not implement any back-off behavior.
> 
> 
> KAFKA-1461. Replica fetcher thread does not implement any back-off behavior.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala 
> 5d5cf5897cc37b3595f14bfe9d7cde43456bcc4b 
>   core/src/main/scala/kafka/server/ReplicaFetcherThread.scala 
> 96faa7b4ed7c9ba8a3f6f9f114bd94e19b3a7ac0 
> 
> Diff: https://reviews.apache.org/r/31366/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>

Reply via email to