[ https://issues.apache.org/jira/browse/FLINK-23022?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17368252#comment-17368252 ]
Arvid Heise edited comment on FLINK-23022 at 6/23/21, 2:51 PM: --------------------------------------------------------------- Imho is the ticket correct and we should fix documentation or even the {{PluginClassLoader}}. {noformat} "A (semicolon-separated) list of patterns that specifies which classes should always be" + " resolved through the plugin parent ClassLoader first. A pattern is a simple prefix that is checked " + " against the fully qualified class name. This setting should generally not be modified. To add another " + " pattern we recommend to use \"plugin.classloader.parent-first-patterns.additional\" instead."{noformat} This is more or less c&p from user classloader. However, in {{PluginClassLoader}}, it's actually different: it gives a list of classes that can be accessed from inside the plugins. It's a poor man's solution for a proper SPI system. Ideally, we would have some {{@SPI}} annotation on all white-listed classes. The plugin system would collect them on start and then the {{PluginClassLoader#loadClass}} should work like this {noformat} if (className in spiClasses) { resolve from flinkClassLoader } else { resolve from pluginClassLoader }{noformat} This would avoid having flink classes being shaded into the plugin from being used and avoid access to anything internal. At some point I was thinking if we should just assume {{@SPI=@Public+@PublicEvolving+@Experimental}}. WDYT? [~chesnay] was (Author: arvid): Imho is the ticket correct and we should fix documentation or even the `PluginClassLoader`. {noformat} "A (semicolon-separated) list of patterns that specifies which classes should always be" + " resolved through the plugin parent ClassLoader first. A pattern is a simple prefix that is checked " + " against the fully qualified class name. This setting should generally not be modified. To add another " + " pattern we recommend to use \"plugin.classloader.parent-first-patterns.additional\" instead."{noformat} This is more or less c&p from user classloader. However, in `PluginClassLoader`, it's actually different: it gives a list of classes that can be accessed from inside the plugins. It's a poor man's solution for a proper SPI system. Ideally, we would have some `@SPI` annotation on all white-listed classes. The plugin system would collect them on start and then the `PluginClassLoader#loadClass` should work like this {noformat} if (className in spiClasses) { resolve from flinkClassLoader } else { resolve from pluginClassLoader }{noformat} This would avoid having flink classes being shaded into the plugin from being used and avoid access to anything internal. At some point I was thinking if we should just assume `@SPI`=`@Public`+`@PublicEvolving`+`@Experimental`. WDYT? [~chesnay] > Plugin classloader settings do not work as described > ---------------------------------------------------- > > Key: FLINK-23022 > URL: https://issues.apache.org/jira/browse/FLINK-23022 > Project: Flink > Issue Type: Bug > Components: API / Core > Reporter: Dawid Wysakowicz > Priority: Major > > The options {{plugin.classloader.parent-first-patterns}} are documented as > all patterns that are put there should be loaded from the Flink classloader > instead of the plugin classloader. > However, the way they work is that they define a set of patterns allowed to > be pulled from the parent classloader. The plugin classloader takes > precedence in all cases. And a class can be loaded from the parent > classloader only if it matches the pattern in one of the aforementioned > options. -- This message was sent by Atlassian Jira (v8.3.4#803005)