[jira] [Comment Edited] (FLINK-23022) Plugin classloader settings do not work as described

["Arvid Heise (Jira)" <ji...@apache.org>]

    [ 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)