andymg3 commented on PR #13521:
URL: https://github.com/apache/kafka/pull/13521#issuecomment-1505671376

   > Thanks for tackling this!
   > 
   > Question about the first test:
   > 
   > > First, it adds a test that makes sure we handle what happens when a 
reassignment completes and none of the new replicas can be made leader. Its 
important we dont keep an old replica as leader.
   > 
   > The reassignment won't complete until the new nodes are in the ISR, right? 
So I'm curious how this could happen in practice. I must be missing something...
   > 
   > Second test looks 👍
   
   
   Thanks for the review @cmccabe.
   
   "The reassignment won't complete until the new nodes are in the ISR, right? 
So I'm curious how this could happen in practice. I must be missing 
something..." So in practice this might be true due to the `IntPredicate` we 
supply as input to `PartitionChangeBuilder`. I'm guessing in practice its true 
if a replica has just been added to the ISR then `isAcceptableLeader` will 
return true. However, that doesnt seem to be a definitive contract from 
`PartitionChangeBuilder`s perspective so it seems like from a coding 
implementation perspective we should not assume `isAcceptableLeader` will 
always return true for newly added to the ISR replicas. Due to that the test 
seems valid. What do you think? 


-- 
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

Reply via email to