patsonluk commented on code in PR #1143:
URL: https://github.com/apache/solr/pull/1143#discussion_r1009628884
##########
solr/core/src/java/org/apache/solr/cloud/ZkController.java:
##########
@@ -2899,6 +2920,23 @@ public NotInClusterStateException(ErrorCode code, String
msg) {
}
}
+ /**
+ * Thrown during pre-register process if the replica is found in
clusterstate but is inconsistent
+ * with current node name
+ */
+ public static class InconsistentClusterStateException extends SolrException {
Review Comment:
Since this is a non runtime exception, the caller should be forced to handle
such exception specifically or this should fall to any handling of unexpected
exceptions of existing logic? :)
In this case, the caller `preRegister` does already have handling for
general unexpected exception, so this is supposed to work as is - be wrapped at
`SolrException`, re-thrown and caught in `createFromDescriptor`, but unlike
`NotInClusterStateException`, which unload(delete) the physical directory, it
will just interrupt the core loading, wrapped and thrown again, and finally be
logged in `CoreContainer#load` without affecting other cores.
Such described flow is already the handling of any unexpected exception
which I find quite fitting for our scenario. And there's an added test case to
ensure a single `InconsistentClusterStateException ` should not affect loading
of other "healthy" cores.
Lemme know if you have any thoughts on different handling!
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
