rhauch commented on a change in pull request #10549:
URL: https://github.com/apache/kafka/pull/10549#discussion_r616994286
##########
File path:
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java
##########
@@ -208,6 +220,21 @@ protected void initLoaders() {
// Finally add parent/system loader.
initPluginLoader(CLASSPATH_NAME);
addAllAliases();
+ reportPluginConflicts();
+ }
+
+ //visible for testing
+ Set<String> reportPluginConflicts() {
+ return allAddedPlugins.entrySet().stream().filter(e ->
e.getValue().size() > 1).map(e -> {
+ String pluginClassName = e.getKey();
+ PluginDesc<?> usedPluginDesc = usedPluginDesc(pluginClassName);
+ List<PluginDesc<?>> ignoredPlugins = new ArrayList<>(e.getValue());
+ ignoredPlugins.remove(usedPluginDesc);
+ log.error("Detected multiple plugins contain '{}'; using plugin {}
and ignoring {} plugins ({}). "
+ + "Check the installation and remove duplicate
plugins from all workers.",
Review comment:
I know I suggested this text, but reading it now makes me think that
users may not really know what "remove duplicate plugins" means. "Remove all of
the ones listed here?" No, we mean all but the one you really want to use. :-D
How about the following?
```suggestion
+ "Check the installation on all workers and if
possible remove all but one of these duplicated plugins.",
```
##########
File path:
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java
##########
@@ -208,6 +220,21 @@ protected void initLoaders() {
// Finally add parent/system loader.
initPluginLoader(CLASSPATH_NAME);
addAllAliases();
+ reportPluginConflicts();
+ }
+
+ //visible for testing
+ Set<String> reportPluginConflicts() {
+ return allAddedPlugins.entrySet().stream().filter(e ->
e.getValue().size() > 1).map(e -> {
+ String pluginClassName = e.getKey();
+ PluginDesc<?> usedPluginDesc = usedPluginDesc(pluginClassName);
+ List<PluginDesc<?>> ignoredPlugins = new ArrayList<>(e.getValue());
+ ignoredPlugins.remove(usedPluginDesc);
+ log.error("Detected multiple plugins contain '{}'; using plugin {}
and ignoring {} plugins ({}). "
Review comment:
Offline you suggested that we change this to warn, based on @C0urante's
earlier observation that sometimes the same converter might be included by
multiple plugins. This latter isn't an issue if it's the same converter version
in all of them.
So +1 to change this to warn:
```suggestion
log.warn("Detected multiple plugins contain '{}'; using plugin
{} and ignoring {} plugins ({}). "
```
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
