snehashisp commented on code in PR #16984:
URL: https://github.com/apache/kafka/pull/16984#discussion_r1871105860
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java:
##########
@@ -69,36 +70,109 @@ public DelegatingClassLoader() {
/**
* Retrieve the PluginClassLoader associated with a plugin class
+ *
* @param name The fully qualified class name of the plugin
* @return the PluginClassLoader that should be used to load this, or null
if the plugin is not isolated.
*/
// VisibleForTesting
- PluginClassLoader pluginClassLoader(String name) {
+ PluginClassLoader pluginClassLoader(String name, VersionRange range) {
if (!PluginUtils.shouldLoadInIsolation(name)) {
return null;
}
+
SortedMap<PluginDesc<?>, ClassLoader> inner = pluginLoaders.get(name);
if (inner == null) {
return null;
}
- ClassLoader pluginLoader = inner.get(inner.lastKey());
+
+
+ ClassLoader pluginLoader = findPluginLoader(inner, name, range);
return pluginLoader instanceof PluginClassLoader
- ? (PluginClassLoader) pluginLoader
- : null;
+ ? (PluginClassLoader) pluginLoader
+ : null;
+ }
+
+ PluginClassLoader pluginClassLoader(String name) {
+ return pluginClassLoader(name, null);
+ }
+
+ ClassLoader connectorLoader(String connectorClassOrAlias, VersionRange
range) throws ClassNotFoundException {
+ String fullName = aliases.getOrDefault(connectorClassOrAlias,
connectorClassOrAlias);
+ // if the plugin is not loaded via the plugin classloader, it might
still be available in the parent delegating
+ // classloader, in order to check if the version satisfies the
requirement we need to load the plugin class here
+ ClassLoader classLoader = loadVersionedPluginClass(fullName, range,
false).getClassLoader();
Review Comment:
Similar query here (w.r.t my previous comment). Since the class path plugins
are scanned and added to pluginLoaders dict, both should return the classpath
loader. Its only if for some reason, a plugin is found that was not scanned we
should get a different in the two methods.
I also saw the other comment in the other PR. I have actually replaced the
use of connector loader with fetching the connector and using
`getClass().getClassLoader()` as that avoids two calls. Now in both cases it
should return classpath loader for non-isolated plugins based on my
understanding. This would mean that instantiation plugins with these
classloaders will bypass the delegating loader, which does not look correct,
but that seems to be the current process. LMK if I am missing something here. I
can check if the class loader here is an instance of `PluginClassLoader` and
return the delegating classloader here if I am incorrect in my assumption.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
