cmccabe commented on code in PR #13407:
URL: https://github.com/apache/kafka/pull/13407#discussion_r1173089106
##########
metadata/src/main/java/org/apache/kafka/metadata/migration/KRaftMigrationDriver.java:
##########
@@ -222,7 +245,7 @@ private boolean isValidStateChange(MigrationDriverState
newState) {
private void transitionTo(MigrationDriverState newState) {
if (!isValidStateChange(newState)) {
- log.error("Error transition in migration driver from {} to {}",
migrationState, newState);
+ log.error("Invalid transition in migration driver from {} to {}",
migrationState, newState);
Review Comment:
I'm confused about this function:
1. Shouldn't the transition to UNINITIALIZED being invalid be covered by
isValidStateChange returning false if you pass it newState = UNINITIALIZED ?
2. The switch statement below throws an exception on invalid state change,
whereas the initial check just logs a message at `error`. The exception seems
more reasonable?
3. the switch has a case for INACTIVE that just does nothing?
```
switch (newState) {
...
case INACTIVE:
// Any state can go to INACTIVE.
break;
}
```
Just leave out the case for `INACTIVE` since it's a no-op? (And maybe the
whole switch, see above.)
--
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
