splett2 commented on code in PR #15702: URL: https://github.com/apache/kafka/pull/15702#discussion_r1568084297
########## metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java: ########## @@ -2117,6 +2117,20 @@ ListPartitionReassignmentsResponseData listPartitionReassignments( return response; } + ControllerResult<Void> getPartitionElrUpdatesForConfigChanges(Optional<String> topicName) { + if (!isElrEnabled()) return ControllerResult.of(Collections.emptyList(), null); + + List<ApiMessageAndVersion> records = new ArrayList<>(); + if (topicName.isPresent()) { + generateLeaderAndIsrUpdates("handleMinIsrUpdate", NO_LEADER, NO_LEADER, NO_LEADER, records, + brokersToElrs.partitionsWithElr(topicsByName.get(topicName.get()))); + } else { + generateLeaderAndIsrUpdates("handleMinIsrUpdate", NO_LEADER, NO_LEADER, NO_LEADER, records, + brokersToElrs.partitionsWithElr()); + } Review Comment: I haven't been following along too closely. Is my understanding correct that we would only expect to generate partition change records that clear the ELR when the min ISR config decreases? When the configured topic ISR increases, it would not be safe to include more replicas in the ELR, since they may not have the HWM. If my understanding is correct, should we have tests to verify that: 1. When the configured topic ISR decreases, we generate the expected partition change record events. 2. When the configured topic ISR increases, we do not generate any partition change record events. -- 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