kevin-wu24 commented on code in PR #22191:
URL: https://github.com/apache/kafka/pull/22191#discussion_r3289887064
##########
tools/src/main/java/org/apache/kafka/tools/MetadataQuorumCommand.java:
##########
@@ -494,12 +503,56 @@ static void handleRemoveController(
throw new TerseException("Failed to parse
--controller-directory-id: " + e.getMessage());
}
if (!dryRun) {
- admin.removeRaftVoter(controllerId, directoryId).
- all().get();
+ removeRaftVoter(admin, controllerId, directoryId, unregister);
}
System.out.printf("%s KRaft controller %d with directory id %s%n",
dryRun ? "DRY RUN of removing " : "Removed ",
controllerId,
directoryId);
+ if (unregister) {
+ if (!dryRun) {
+ unregisterController(admin, controllerId);
+ }
+ System.out.printf("%s KRaft controller %d%n",
+ dryRun ? "DRY RUN of unregistering " : "Unregistered ",
+ controllerId);
+ }
+ }
+
+ private static void removeRaftVoter(
+ Admin admin,
+ int controllerId,
+ Uuid directoryId,
+ boolean unregister
+ ) throws TerseException, ExecutionException, InterruptedException {
+ try {
+ admin.removeRaftVoter(controllerId, directoryId).all().get();
+ } catch (ExecutionException e) {
+ Throwable cause = e.getCause();
+ if (unregister && (cause instanceof UnsupportedVersionException ||
+ cause instanceof VoterNotFoundException)) {
+ throw new TerseException("Failed to remove KRaft voter " +
controllerId
+ + ": " + cause.getMessage()
+ + ". To unregister the controller from the cluster, run "
+ + "`kafka-cluster.sh unregister-controller --controller-id
"
+ + controllerId + "`.");
+ }
+ throw e;
+ }
+ }
+
+ private static void unregisterController(Admin admin, int controllerId)
+ throws TerseException, InterruptedException {
+ try {
+ admin.unregisterController(controllerId).all().get();
+ } catch (ExecutionException e) {
+ Throwable cause = e.getCause();
+ throw new TerseException("Removed KRaft voter " + controllerId
+ + " but failed to unregister it: "
Review Comment:
We enter `MetadataQuorumCommand#unregisterController` only if
`removeRaftVoter` returns successfully first. I think it is okay to output what
happened to the user here. 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
