artemlivshits commented on code in PR #14593: URL: https://github.com/apache/kafka/pull/14593#discussion_r1370847603
########## metadata/src/main/java/org/apache/kafka/controller/PartitionChangeBuilder.java: ########## @@ -361,11 +410,12 @@ public Optional<ApiMessageAndVersion> build() { maybeUpdateRecordElr(record); - if (record.isr() == null && !targetIsr.isEmpty() && !targetIsr.equals(Replicas.toList(partition.isr))) { + if (record.isr() == null && (!targetIsr.isEmpty() || eligibleLeaderReplicasEnabled) && !targetIsr.equals(Replicas.toList(partition.isr))) { Review Comment: Should we add a comment here that we let ISR shrink to empty if we're using eligible leader replicas? ########## metadata/src/main/java/org/apache/kafka/controller/PartitionChangeBuilder.java: ########## @@ -93,6 +93,9 @@ public enum Election { private boolean eligibleLeaderReplicasEnabled; private int minISR; + // Whether allow electing last known leader when ELR is enabled. Note, the last known leader will be stored in the + // lastKnownElr field. + private boolean lastKnownLeaderEnabled = true; Review Comment: Should we name it something like `useLastKnownLeaderInBalancedRecovery`? -- 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