ctubbsii commented on PR #3375: URL: https://github.com/apache/accumulo/pull/3375#issuecomment-1536200374
> If we take VFS out of the equation - if a user mistakenly specifies that classpath for a classloader context, then runs a scan on table that uses that context to load iterator classes, then they can take down a good portion or the entire cluster. However, I believe that the client makes a call to the Tablet Server to test that it can load the iterator classes. I stand by my statement in the previous discussion that a misconfigured classpath is indicative of a serious problem, and the risks of continuing to run under such circumstances are far higher than being offline temporarily. > I believe the issue here is the class can only be loaded by a portion of the TabletServers, due to VFS. Given that we have users using the VFSClassLoader, I think we should resurrect the configuration property. If we can somehow scope that to apply only to scan threads and only to classloading Errors, that might be appropriate. I strongly disagree. We already knew that we had users still using the VFSClassLoader when we made the previous change. Nothing has changed from then. All the options we previously discussed when thinking through a migration path for these users are still available. Resurrecting a configuration property like that is still putting the responsibility for handling these things *inside* Accumulo's code base, and that's retreating from the position we staked out when creating the ContextClassLoaderFactory SPI. The proper place for adding that complexity, trade-offs, configuration knobs, etc., for handling things differently than the default for class loading is inside the user's choice of class to implement ContextClassLoaderFactory, and *not* inside Accumulo. -- 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]
