[GitHub] [kafka] andymg3 commented on pull request #13521: MINOR: Add tests verifying leadership during and after reassignments in KRaft mode

["andymg3 (via GitHub)" <gi...@apache.org>]

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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org