keith-turner commented on code in PR #3683:
URL: https://github.com/apache/accumulo/pull/3683#discussion_r1290684472
##########
core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java:
##########
@@ -75,22 +75,29 @@ static synchronized void resetContextFactoryForTests() {
@SuppressWarnings("deprecation")
public static ClassLoader getClassLoader(String context) {
- try {
- if (FACTORY.isValid(context)) {
- return FACTORY.getClassLoader(context);
- } else {
- return
org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader.getClassLoader();
- }
- } catch (RuntimeException e) {
+ if (context != null && !context.isEmpty()) {
+ return FACTORY.getClassLoader(context);
Review Comment:
I don't think it possible to avoid bad config being set. We could do
complex validation of any classloader config set via ZK that exmanines the new
config against all existing classloader config. However since the config could
be set in a file, its still possible to set bad config.
Its way out of scope of this PR which is focused on validation of a single
config, but maybe we could create something like the following for loading
table plugins. After creating the following utility method somewhere we could
then adapt code that instantiates classes to use the new utility method.
```java
class SomeUtilClass {
/**
* Creates a new instance of an object using a tables context
classloader retrying until the object is created.
* /
T newInstance(TableConfiguration config, Property prop, Class<T>
expectedType){
// TODO use retry
while(true){
try{
String context = config.getClassloaderContext();
String clazz = config.getClassName(prop);
return instantiateClass(context, clazz, expectedType);
} catch(Exception e){
// assume this is caused by bad classloader config and
retry
// TODO log message and sleep using retry
}
}
}
}
```
--
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]
