kevinrr888 commented on issue #5601:
URL: https://github.com/apache/accumulo/issues/5601#issuecomment-2931428793
It seems like most (maybe all) instances are cases where it is believed we
have reached an impossible state, which seems appropriate to me that an
`AssertionError` is thrown (and thus the JVM is halted). If the impossible
occurs, it seems like it might be best that the JVM is halted and it is
immediately obvious something is very wrong. This kind of thing is more likely
to be overlooked/lost if a `RuntimeException` is thrown instead.
If we decide that `AssertionError` is fine in the situations where an
impossible state has somehow been reached, something that could be looked into
is if all uses of `AssertionError` are correct.
Could also see if there are more places where `AssertionError` can be used.
For example, in `CompactableImpl` noticed:
```
switch (kind) {
case SYSTEM: {
if (isCompactionStratConfigured) {
return Set.of();
}
return handleSystemCompaction(currFiles);
}
case SELECTOR:
// intentional fall through
case USER:
return handleUserSelectorCompaction(currFiles, kind);
case CHOP: {
return handleChopCompaction(currFiles);
}
default:
throw new AssertionError();
}
```
Switches like this are done a lot, and I know the default is not always to
throw `AssertionError`, when it maybe should be. `CompactionKind` is only
`SYSTEM`, `SELECTOR`, `USER`, `CHOP`, it is impossible that default is reached
unless `CompactionKind` is added to without considering this code, which it
needs to.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
