Re: Re: ClassLoader created by BlobLibraryCacheManager is not using context classloader

2019-09-03 Thread Till Rohrmann
I see the problem Jan. What about the following proposal: Instead of using
the LocalEnvironment for local tests you always use the RemoteEnvironment
but when testing it locally you spin up a MiniCluster in the same process
and initialize the RemoteEnvironment with `MiniCluster#getRestAddress`.
That way you would always submit a jar with the generated classes and,
hence, not having to set the context class loader.

The contract of the LocalEnvironment is indeed that all classes it is
supposed t execute must be present when being started.

Cheers,
Till

On Tue, Sep 3, 2019 at 2:27 PM guaishushu1...@163.com <
guaishushu1...@163.com> wrote:

>
>
>
>
> guaishushu1...@163.com
>
> From: guaishushu1...@163.com
> Date: 2019-09-03 20:25
> To: dev
> Subject: Re: Re: ClassLoader created by BlobLibraryCacheManager is not
> using context classloader
>
>
>
>
> guaishushu1...@163.com
> From: guaishushu1...@163.com
> Date: 2019-09-03 20:23
> To: dev
> Subject: Re: Re: ClassLoader created by BlobLibraryCacheManager is not
> using context classloader
> guaishushu1...@163.com
> From: Jan Lukavský
> Date: 2019-09-03 20:17
> To: dev
> Subject: Re: ClassLoader created by BlobLibraryCacheManager is not using
> context classloader
> Hi Till,
> the use-case is pretty much simple - I have a REPL shell in groovy,
> which generates classes at runtime. The actual hierarchy is therefore
> system class loader -> application classloader -> repl classloader
> (GroovyClassLoader actually)
> now, when a terminal (sink) operation in the shell occurs, I'm able to
> build a jar, which I can submit to remote cluster (in distributed case).
> But - during testing -  I run the code using local flink. There is no
> (legal) way of adding this new runtime generated jar to local flink. As
> I said, I have a hackish solution which works on JDK <= 8, because it
> uses reflection to call addURL on the application classloader (and
> thefore "pretends", that the generated jar was there all the time from
> the JVM startup). This breaks on JDK >= 9. It might be possible to work
> around this somehow, but I think that the reason why LocalEnvironment is
> not having a way to add jars (as in case of RemoteEnvironment) is that
> is assumes, that you actually have all of the on classpath when using
> local runner. I think that this implies that it either has to use
> context classloader (to be able to work on top of any classloading user
> might have), or is wrong and would need be fixed, so that
> LocalEnvironment would accept files to "stage" - which would mean adding
> them to a class loader (probably URLClassLoader with the application
> class loader as parent).
> Or, would you see any other option?
> Jan
> On 9/3/19 2:00 PM, Till Rohrmann wrote:
> > Hi Jan,
> >
> > I've talked with Aljoscha and Stephan offline and we concluded that we
> > would like to avoid the usage of context class loaders if possible. The
> > reason for this is that using the context class loader can easily mess up
> > an otherwise clear class loader hierarchy which makes it hard to reason
> > about and to debug class loader issues.
> >
> > Given this, I think it would help to better understand the exact problem
> > you are trying to solve by using the context class loader. Usually the
> > usage of the context class loader points towards an API deficiency which
> we
> > might be able to address differently.
> >
> > Cheers,
> > Till
> >
> > On Mon, Sep 2, 2019 at 11:32 AM Aljoscha Krettek 
> > wrote:
> >
> >> I’m not saying we can’t change that code to use the context class
> loader.
> >> I’m just not sure whether this might break other things.
> >>
> >> Best,
> >> Aljoscha
> >>
> >>> On 2. Sep 2019, at 11:24, Jan Lukavský  wrote:
> >>>
> >>> Essentially, the class loader of Flink should be present in parent
> >> hierarchy of context class loader. If FlinkUserCodeClassLoader doesn't
> use
> >> context class loader, then it is actually impossible to use a hierarchy
> >> like this:
> >>>   system class loader -> application class loader -> user-defined class
> >> loader (defines some UDFs to be used in flink program)
> >>> Flink now uses the application class loader, and so the UDFs fail to
> >> deserialize on local flink, because the user-defined class loader is
> >> bypassed. Moreover, there is no way to add additional classpath elements
> >> for LocalEnvironment (as opposed to RemoteEnvironment). I'm able to hack
> >> this by calling addURL method on the application class loader (which

Re: Re: ClassLoader created by BlobLibraryCacheManager is not using context classloader

2019-09-03 Thread guaishushu1...@163.com




guaishushu1...@163.com
 
From: guaishushu1...@163.com
Date: 2019-09-03 20:25
To: dev
Subject: Re: Re: ClassLoader created by BlobLibraryCacheManager is not using 
context classloader
 
 
 
 
guaishushu1...@163.com
From: guaishushu1...@163.com
Date: 2019-09-03 20:23
To: dev
Subject: Re: Re: ClassLoader created by BlobLibraryCacheManager is not using 
context classloader
guaishushu1...@163.com
From: Jan Lukavský
Date: 2019-09-03 20:17
To: dev
Subject: Re: ClassLoader created by BlobLibraryCacheManager is not using 
context classloader
Hi Till,
the use-case is pretty much simple - I have a REPL shell in groovy, 
which generates classes at runtime. The actual hierarchy is therefore
system class loader -> application classloader -> repl classloader 
(GroovyClassLoader actually)
now, when a terminal (sink) operation in the shell occurs, I'm able to 
build a jar, which I can submit to remote cluster (in distributed case). 
But - during testing -  I run the code using local flink. There is no 
(legal) way of adding this new runtime generated jar to local flink. As 
I said, I have a hackish solution which works on JDK <= 8, because it 
uses reflection to call addURL on the application classloader (and 
thefore "pretends", that the generated jar was there all the time from 
the JVM startup). This breaks on JDK >= 9. It might be possible to work 
around this somehow, but I think that the reason why LocalEnvironment is 
not having a way to add jars (as in case of RemoteEnvironment) is that 
is assumes, that you actually have all of the on classpath when using 
local runner. I think that this implies that it either has to use 
context classloader (to be able to work on top of any classloading user 
might have), or is wrong and would need be fixed, so that 
LocalEnvironment would accept files to "stage" - which would mean adding 
them to a class loader (probably URLClassLoader with the application 
class loader as parent).
Or, would you see any other option?
Jan
On 9/3/19 2:00 PM, Till Rohrmann wrote:
> Hi Jan,
>
> I've talked with Aljoscha and Stephan offline and we concluded that we
> would like to avoid the usage of context class loaders if possible. The
> reason for this is that using the context class loader can easily mess up
> an otherwise clear class loader hierarchy which makes it hard to reason
> about and to debug class loader issues.
>
> Given this, I think it would help to better understand the exact problem
> you are trying to solve by using the context class loader. Usually the
> usage of the context class loader points towards an API deficiency which we
> might be able to address differently.
>
> Cheers,
> Till
>
> On Mon, Sep 2, 2019 at 11:32 AM Aljoscha Krettek 
> wrote:
>
>> I’m not saying we can’t change that code to use the context class loader.
>> I’m just not sure whether this might break other things.
>>
>> Best,
>> Aljoscha
>>
>>> On 2. Sep 2019, at 11:24, Jan Lukavský  wrote:
>>>
>>> Essentially, the class loader of Flink should be present in parent
>> hierarchy of context class loader. If FlinkUserCodeClassLoader doesn't use
>> context class loader, then it is actually impossible to use a hierarchy
>> like this:
>>>   system class loader -> application class loader -> user-defined class
>> loader (defines some UDFs to be used in flink program)
>>> Flink now uses the application class loader, and so the UDFs fail to
>> deserialize on local flink, because the user-defined class loader is
>> bypassed. Moreover, there is no way to add additional classpath elements
>> for LocalEnvironment (as opposed to RemoteEnvironment). I'm able to hack
>> this by calling addURL method on the application class loader (which is
>> terribly hackish), but that works only on JDK <= 8. No sensible workaround
>> is available for JDK >= 9.
>>> Alternative solution would be to enable adding jars to class loader when
>> using LocalEnvironment, but that looks a little odd.
>>> Jan
>>>
>>> On 9/2/19 11:02 AM, Aljoscha Krettek wrote:
>>>> Hi,
>>>>
>>>> I actually don’t know whether that change would be ok.
>> FlinkUserCodeClassLoader has taken
>> FlinkUserCodeClassLoader.class.getClassLoader() as the parent ClassLoader
>> before my change. See:
>> https://github.com/apache/flink/blob/release-1.2/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoader.java
>> <
>> https://github.com/apache/flink/blob/release-1.2/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoader.java
>>> .
>>>> I have the feeling that this might be on purpose because we want to
>&

Re: Re: ClassLoader created by BlobLibraryCacheManager is not using context classloader

2019-09-03 Thread guaishushu1...@163.com




guaishushu1...@163.com
 
From: guaishushu1...@163.com
Date: 2019-09-03 20:23
To: dev
Subject: Re: Re: ClassLoader created by BlobLibraryCacheManager is not using 
context classloader
 
 
 
 
guaishushu1...@163.com
From: Jan Lukavský
Date: 2019-09-03 20:17
To: dev
Subject: Re: ClassLoader created by BlobLibraryCacheManager is not using 
context classloader
Hi Till,
the use-case is pretty much simple - I have a REPL shell in groovy, 
which generates classes at runtime. The actual hierarchy is therefore
system class loader -> application classloader -> repl classloader 
(GroovyClassLoader actually)
now, when a terminal (sink) operation in the shell occurs, I'm able to 
build a jar, which I can submit to remote cluster (in distributed case). 
But - during testing -  I run the code using local flink. There is no 
(legal) way of adding this new runtime generated jar to local flink. As 
I said, I have a hackish solution which works on JDK <= 8, because it 
uses reflection to call addURL on the application classloader (and 
thefore "pretends", that the generated jar was there all the time from 
the JVM startup). This breaks on JDK >= 9. It might be possible to work 
around this somehow, but I think that the reason why LocalEnvironment is 
not having a way to add jars (as in case of RemoteEnvironment) is that 
is assumes, that you actually have all of the on classpath when using 
local runner. I think that this implies that it either has to use 
context classloader (to be able to work on top of any classloading user 
might have), or is wrong and would need be fixed, so that 
LocalEnvironment would accept files to "stage" - which would mean adding 
them to a class loader (probably URLClassLoader with the application 
class loader as parent).
Or, would you see any other option?
Jan
On 9/3/19 2:00 PM, Till Rohrmann wrote:
> Hi Jan,
>
> I've talked with Aljoscha and Stephan offline and we concluded that we
> would like to avoid the usage of context class loaders if possible. The
> reason for this is that using the context class loader can easily mess up
> an otherwise clear class loader hierarchy which makes it hard to reason
> about and to debug class loader issues.
>
> Given this, I think it would help to better understand the exact problem
> you are trying to solve by using the context class loader. Usually the
> usage of the context class loader points towards an API deficiency which we
> might be able to address differently.
>
> Cheers,
> Till
>
> On Mon, Sep 2, 2019 at 11:32 AM Aljoscha Krettek 
> wrote:
>
>> I’m not saying we can’t change that code to use the context class loader.
>> I’m just not sure whether this might break other things.
>>
>> Best,
>> Aljoscha
>>
>>> On 2. Sep 2019, at 11:24, Jan Lukavský  wrote:
>>>
>>> Essentially, the class loader of Flink should be present in parent
>> hierarchy of context class loader. If FlinkUserCodeClassLoader doesn't use
>> context class loader, then it is actually impossible to use a hierarchy
>> like this:
>>>   system class loader -> application class loader -> user-defined class
>> loader (defines some UDFs to be used in flink program)
>>> Flink now uses the application class loader, and so the UDFs fail to
>> deserialize on local flink, because the user-defined class loader is
>> bypassed. Moreover, there is no way to add additional classpath elements
>> for LocalEnvironment (as opposed to RemoteEnvironment). I'm able to hack
>> this by calling addURL method on the application class loader (which is
>> terribly hackish), but that works only on JDK <= 8. No sensible workaround
>> is available for JDK >= 9.
>>> Alternative solution would be to enable adding jars to class loader when
>> using LocalEnvironment, but that looks a little odd.
>>> Jan
>>>
>>> On 9/2/19 11:02 AM, Aljoscha Krettek wrote:
>>>> Hi,
>>>>
>>>> I actually don’t know whether that change would be ok.
>> FlinkUserCodeClassLoader has taken
>> FlinkUserCodeClassLoader.class.getClassLoader() as the parent ClassLoader
>> before my change. See:
>> https://github.com/apache/flink/blob/release-1.2/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoader.java
>> <
>> https://github.com/apache/flink/blob/release-1.2/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoader.java
>>> .
>>>> I have the feeling that this might be on purpose because we want to
>> have the ClassLoader of the Flink Framework components be the parent
>> ClassLoader, but I could be wrong. Maybe Stephan would be most appropriate
>> for answering this

Re: Re: ClassLoader created by BlobLibraryCacheManager is not using context classloader

2019-09-03 Thread guaishushu1...@163.com




guaishushu1...@163.com
 
From: Jan Lukavský
Date: 2019-09-03 20:17
To: dev
Subject: Re: ClassLoader created by BlobLibraryCacheManager is not using 
context classloader
Hi Till,
 
the use-case is pretty much simple - I have a REPL shell in groovy, 
which generates classes at runtime. The actual hierarchy is therefore
 
 system class loader -> application classloader -> repl classloader 
(GroovyClassLoader actually)
 
now, when a terminal (sink) operation in the shell occurs, I'm able to 
build a jar, which I can submit to remote cluster (in distributed case). 
But - during testing -  I run the code using local flink. There is no 
(legal) way of adding this new runtime generated jar to local flink. As 
I said, I have a hackish solution which works on JDK <= 8, because it 
uses reflection to call addURL on the application classloader (and 
thefore "pretends", that the generated jar was there all the time from 
the JVM startup). This breaks on JDK >= 9. It might be possible to work 
around this somehow, but I think that the reason why LocalEnvironment is 
not having a way to add jars (as in case of RemoteEnvironment) is that 
is assumes, that you actually have all of the on classpath when using 
local runner. I think that this implies that it either has to use 
context classloader (to be able to work on top of any classloading user 
might have), or is wrong and would need be fixed, so that 
LocalEnvironment would accept files to "stage" - which would mean adding 
them to a class loader (probably URLClassLoader with the application 
class loader as parent).
 
Or, would you see any other option?
 
Jan
 
 
On 9/3/19 2:00 PM, Till Rohrmann wrote:
> Hi Jan,
>
> I've talked with Aljoscha and Stephan offline and we concluded that we
> would like to avoid the usage of context class loaders if possible. The
> reason for this is that using the context class loader can easily mess up
> an otherwise clear class loader hierarchy which makes it hard to reason
> about and to debug class loader issues.
>
> Given this, I think it would help to better understand the exact problem
> you are trying to solve by using the context class loader. Usually the
> usage of the context class loader points towards an API deficiency which we
> might be able to address differently.
>
> Cheers,
> Till
>
> On Mon, Sep 2, 2019 at 11:32 AM Aljoscha Krettek 
> wrote:
>
>> I’m not saying we can’t change that code to use the context class loader.
>> I’m just not sure whether this might break other things.
>>
>> Best,
>> Aljoscha
>>
>>> On 2. Sep 2019, at 11:24, Jan Lukavský  wrote:
>>>
>>> Essentially, the class loader of Flink should be present in parent
>> hierarchy of context class loader. If FlinkUserCodeClassLoader doesn't use
>> context class loader, then it is actually impossible to use a hierarchy
>> like this:
>>>   system class loader -> application class loader -> user-defined class
>> loader (defines some UDFs to be used in flink program)
>>> Flink now uses the application class loader, and so the UDFs fail to
>> deserialize on local flink, because the user-defined class loader is
>> bypassed. Moreover, there is no way to add additional classpath elements
>> for LocalEnvironment (as opposed to RemoteEnvironment). I'm able to hack
>> this by calling addURL method on the application class loader (which is
>> terribly hackish), but that works only on JDK <= 8. No sensible workaround
>> is available for JDK >= 9.
>>> Alternative solution would be to enable adding jars to class loader when
>> using LocalEnvironment, but that looks a little odd.
>>> Jan
>>>
>>> On 9/2/19 11:02 AM, Aljoscha Krettek wrote:
>>>> Hi,
>>>>
>>>> I actually don’t know whether that change would be ok.
>> FlinkUserCodeClassLoader has taken
>> FlinkUserCodeClassLoader.class.getClassLoader() as the parent ClassLoader
>> before my change. See:
>> https://github.com/apache/flink/blob/release-1.2/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoader.java
>> <
>> https://github.com/apache/flink/blob/release-1.2/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoader.java
>>> .
>>>> I have the feeling that this might be on purpose because we want to
>> have the ClassLoader of the Flink Framework components be the parent
>> ClassLoader, but I could be wrong. Maybe Stephan would be most appropriate
>> for answering this.
>>>> Best,
>>>> Aljoscha
>>>>
>>>>> On 30. Aug 2019, at 16:28, Till Rohrmann  wrote:
>>>>>
>>>&g