vamossagar12 commented on PR #14051: URL: https://github.com/apache/kafka/pull/14051#issuecomment-1646758971
Actually now that I think of it, instead of null check, can we => 1) Remove the `canAddReplicaToIsr` call from [here](https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/cluster/Partition.scala#L956C56-L956C56)? The reason I say is that, IIUC it is trying to access the shared `remoteReplicasMap` w.o acquiring a lock and also when we do acquire the lock via the 3rd predicate, we invoke `needsExpandIsr` which anyways invokes `canAddReplicaToIsr`. So, as such what we get out of the second AND predicate is a pre-emptive check which is happening in a synchronized manner below. 2) Also add the condition to `canAddReplicaToIsr` to check if the replica is part of the `remoteReplicasMap` once we have acquired the lock and before we invoke `isReplicaIsrEligible`. Then we can look at the PartitionLockTest class to see if we can test this scenario. WDYT? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
