dlmarion commented on code in PR #3678:
URL: https://github.com/apache/accumulo/pull/3678#discussion_r1285231628
##########
core/src/main/java/org/apache/accumulo/core/spi/common/ContextClassLoaderFactory.java:
##########
@@ -65,4 +65,15 @@ default void init(ContextClassLoaderEnvironment env) {}
* @return the class loader for the given contextName
*/
ClassLoader getClassLoader(String contextName);
+
+ /**
+ * Validate that the contextName is supported by the
ContextClassLoaderFactory implementation
+ *
+ * @param contextName the name of the context that represents a class loader
that is managed by
+ * this factory (can be null)
+ * @return true if valid, false otherwise
+ */
+ default boolean isValid(String contextName) {
+ return false;
Review Comment:
Based on my analysis in #3677 , an invalid context name could prevent any
classes from being loaded, which could lead to the minor compaction thread for
a tablet to die. From what I found, a minor compaction for a tablet will not
occur again if the minor compaction thread dies, meaning that it can't be
closed normally. I think the safest action when this occurs is a total restart
of Accumulo.
The changes in #3677 try to prevent the minor compaction thread from dying,
but there could be other areas within Accumulo where something similar happens
when an invalid context name is used. Given that, and that the recovery for
this situation is a total restart, I would suggest that `true` not be the
default answer. In fact, I don't think this should be a default method, I think
we should force all implementations to provide an answer.
--
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]
