cmccabe commented on code in PR #18045:
URL: https://github.com/apache/kafka/pull/18045#discussion_r1870374978
##########
metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java:
##########
@@ -1422,11 +1422,17 @@ void handleBrokerInControlledShutdown(int brokerId,
long brokerEpoch, List<ApiMe
* @param records The record list to append to.
*/
void handleBrokerUncleanShutdown(int brokerId, List<ApiMessageAndVersion>
records) {
- if (!featureControl.metadataVersion().isElrSupported()) return;
- generateLeaderAndIsrUpdates("handleBrokerUncleanShutdown", NO_LEADER,
NO_LEADER, brokerId, records,
- brokersToIsrs.partitionsWithBrokerInIsr(brokerId));
- generateLeaderAndIsrUpdates("handleBrokerUncleanShutdown", NO_LEADER,
NO_LEADER, brokerId, records,
- brokersToElrs.partitionsWithBrokerInElr(brokerId));
+ if (!featureControl.metadataVersion().isElrSupported()) {
+ // ELR is not enabled, handle the unclean shutdown as if the
broker was fenced
+ generateLeaderAndIsrUpdates("handleBrokerUncleanShutdown",
brokerId, NO_LEADER, NO_LEADER, records,
Review Comment:
The issue here that I see is that you're potentially generating a lot of
records. That's OK but you will need to change this line in
`ClusterControlManager`
```
return ControllerResult.atomicOf(records, new
BrokerRegistrationReply(record.brokerEpoch()));
```
to this:
```
return ControllerResult.of(records, new
BrokerRegistrationReply(record.brokerEpoch()));
```
The reason is because we are generating more records than may fit in a
single batch.
It should be safe to do this *since the broker registration comes last*. If
we get halfway through removing the broker from its old ISRs and controller
failover hits, the new controller can finish the job just fine.
Please do add a comment explaining this logic and specifying that the
registration needs to come last, though.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
