kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sriharsha Chintalapani" <har...@hortonworks.com>
Subject Re: Review Request 23474: Patch for KAFKA-1483
Date Thu, 17 Jul 2014 15:13:32 GMT


> On July 15, 2014, 5:51 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/server/ReplicaManager.scala, line 484
> > <https://reviews.apache.org/r/23474/diff/1/?file=631878#file631878line484>
> >
> >     I am wondering if this function will introduce more lock contention on leaderIsrUpdateLock?
Every leaderReplicaIfLocal call will need to hold on its read lock while other requests at
the same time may try to modify the leader option?
> 
> Sriharsha Chintalapani wrote:
>     It might increase lock contention but writes to change the leadership occurs not
so frequently so using readLock wouldn't be much of an issue I think. I also considered comparing
ReplicaManager.localBrokerId with Partition.leaderReplicaIdOpt without a lock this will present
an issue where the leader is changed and we might be comparing against old leader value.
>     Instead of using leaderReplicaIfLocal I can create another method "checkLeader" in
Partition which will return a boolean after obtaining a readLock on leaderIsrUpdateLock. Right
now there is unnecessary call going to getReplica.
> 
> Guozhang Wang wrote:
>     I think one optimization we can try at least is to grab the read lock before the
iteration to avoid grab/release on each partition during the iteration.
> 
> Jun Rao wrote:
>     The intention of maintaining a separate leaderPartitions is to make LeaderCount check
an O(1) operation, instead of O(n). However, UnderReplicatedPartitions is still an O(n) operation
and it's already paying the readLock overhead. So, making LeaderCount an O(n) operation with
the readLock overhead probably won't be too bad.

one approach to eliminate leaderIsrUpdateLock is to wrap allPartitions in another ReadWriteLock
and not use the partition.leaderIsrUpdateLock for leader check. Let me know if that makes
sense or if we are ok with the current approach. Thanks.


- Sriharsha


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


On July 16, 2014, 6:07 p.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23474/
> -----------------------------------------------------------
> 
> (Updated July 16, 2014, 6:07 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1483
>     https://issues.apache.org/jira/browse/KAFKA-1483
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1483. Split Brain about Leader Partitions.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/server/ReplicaManager.scala 6a56a772c134dbf1e70c1bfe067223009bfdbac8

> 
> Diff: https://reviews.apache.org/r/23474/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message