> On Oct. 18, 2013, 5:34 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/AbstractFetcherManager.scala, line 140
> > <https://reviews.apache.org/r/14730/diff/1/?file=366222#file366222line140>
> >
> >     We need to think through this change. What if a broker is restarted 
> > quickly with a new host/port, but same broker id? Can we guarantee that a 
> > new fetcher thread with the new host/port will be added if we just keep 
> > broker id in the fetcherThreadMap?

You are right, we need the host/port info as key here.


> On Oct. 18, 2013, 5:34 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/AbstractFetcherThread.scala, lines 190-194
> > <https://reviews.apache.org/r/14730/diff/1/?file=366223#file366223line190>
> >
> >     Not sure if doing this outside of the lock is safe since immediately 
> > after the check, new partitions can be added.
> >     
> >     Removing all partitions inside a lock should still be cheap.

Since addPartitions are only called by AbstractFetcherManager.addFetcher and 
removeFetcher, which is covered by mapLock, there will only be one add/remove 
operation happening at a time. So this would be fine.


- Guozhang


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


On Oct. 18, 2013, 2:15 a.m., Guozhang Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14730/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2013, 2:15 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1001
>     https://issues.apache.org/jira/browse/KAFKA-1001
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1001.v1.6
> 
> 
> KAFKA-1001.v1.5
> 
> 
> KAFKA-1001.v1
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/cluster/Partition.scala 
> 5ccecd179d33abfc14dcefc35dd68de7474c6978 
>   core/src/main/scala/kafka/common/ErrorMapping.scala 
> 153bc0b078d21200c02c47dd5ad9b7a7e3326ec4 
>   core/src/main/scala/kafka/consumer/ConsumerFetcherManager.scala 
> 566ca46d113ee7da4b38ee57302ba183b59ab5d6 
>   core/src/main/scala/kafka/consumer/ConsumerFetcherThread.scala 
> dda0a8f041f242bf8a501a8cbd2b9c0258323f96 
>   core/src/main/scala/kafka/log/LogManager.scala 
> 47197153c5d3797d2e2a2f9539d9cd55501468e3 
>   core/src/main/scala/kafka/server/AbstractFetcherManager.scala 
> 15b7bd31446ffb97b8ed0fa6461649a01d81c7e9 
>   core/src/main/scala/kafka/server/AbstractFetcherThread.scala 
> c64260f12bdd6b6c964875e1f3873156442e44e1 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 
> ee1cc0cf451b691eb91d9158ca765aeb60fc3dc8 
> 
> Diff: https://reviews.apache.org/r/14730/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guozhang Wang
> 
>

Reply via email to