[jira] [Commented] (FLINK-34402) Class loading conflicts when using PowerMock in ITcase.
[ https://issues.apache.org/jira/browse/FLINK-34402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17815213#comment-17815213 ] yisha zhou commented on FLINK-34402: Thank you for your patience and time, [~xtsong] I'll try to find a better solution for integration test of SQL connectors and not to use PowerMock. > Class loading conflicts when using PowerMock in ITcase. > > > Key: FLINK-34402 > URL: https://issues.apache.org/jira/browse/FLINK-34402 > Project: Flink > Issue Type: Bug > Components: Runtime / Coordination >Affects Versions: 1.19.0 >Reporter: yisha zhou >Assignee: yisha zhou >Priority: Major > > Currently when no user jars exist, system classLoader will be used to load > classes as default. However, if we use powerMock to create some ITCases, the > framework will utilize JavassistMockClassLoader to load classes. Forcing the > use of the system classLoader can lead to class loading conflict issue. > Therefore we should use Thread.currentThread().getContextClassLoader() > instead of > ClassLoader.getSystemClassLoader() here. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (FLINK-34402) Class loading conflicts when using PowerMock in ITcase.
[ https://issues.apache.org/jira/browse/FLINK-34402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17815206#comment-17815206 ] Xintong Song commented on FLINK-34402: -- > Returning to the change, do you have any concern about the replacement? Replacing `ClassLoader.getSystemClassLoader()` with `Thread.currentThread().getContextClassLoader()` implies the assumption that `BlobLibraryCacheManager` should be called from a thread that is created by the system classloader, which is implicit and fragile. If later `BlobLibraryCacheManager` is called from another thread, the assumption can easily be overlooked and broken, leading to unpredictable behaviors. This may not be absolutely unaffordable, but compared to what we gain from the changes, I'd rather not to apply it. > Class loading conflicts when using PowerMock in ITcase. > > > Key: FLINK-34402 > URL: https://issues.apache.org/jira/browse/FLINK-34402 > Project: Flink > Issue Type: Bug > Components: Runtime / Coordination >Affects Versions: 1.19.0 >Reporter: yisha zhou >Assignee: yisha zhou >Priority: Major > > Currently when no user jars exist, system classLoader will be used to load > classes as default. However, if we use powerMock to create some ITCases, the > framework will utilize JavassistMockClassLoader to load classes. Forcing the > use of the system classLoader can lead to class loading conflict issue. > Therefore we should use Thread.currentThread().getContextClassLoader() > instead of > ClassLoader.getSystemClassLoader() here. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (FLINK-34402) Class loading conflicts when using PowerMock in ITcase.
[ https://issues.apache.org/jira/browse/FLINK-34402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17815199#comment-17815199 ] yisha zhou commented on FLINK-34402: [~martijnvisser] I totally agree with your opinion that we should not do extra work to support what community discourages. Do you have any idea how to check API calling times for client to external system in SQL connector? Or what's the best practice to do integration testing for a SQL connector? > Class loading conflicts when using PowerMock in ITcase. > > > Key: FLINK-34402 > URL: https://issues.apache.org/jira/browse/FLINK-34402 > Project: Flink > Issue Type: Bug > Components: Runtime / Coordination >Affects Versions: 1.19.0 >Reporter: yisha zhou >Assignee: yisha zhou >Priority: Major > > Currently when no user jars exist, system classLoader will be used to load > classes as default. However, if we use powerMock to create some ITCases, the > framework will utilize JavassistMockClassLoader to load classes. Forcing the > use of the system classLoader can lead to class loading conflict issue. > Therefore we should use Thread.currentThread().getContextClassLoader() > instead of > ClassLoader.getSystemClassLoader() here. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (FLINK-34402) Class loading conflicts when using PowerMock in ITcase.
[ https://issues.apache.org/jira/browse/FLINK-34402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17815180#comment-17815180 ] Martijn Visser commented on FLINK-34402: I don't think this is a change that Flink should incorporate like [~xtsong] stated: we don't want Mockito in the Flink code base and recommend against it. If you use that for your internal fork, then it's also up to you to solve any problems that might arise because you're using something that the Flink community discourages. > Class loading conflicts when using PowerMock in ITcase. > > > Key: FLINK-34402 > URL: https://issues.apache.org/jira/browse/FLINK-34402 > Project: Flink > Issue Type: Bug > Components: Runtime / Coordination >Affects Versions: 1.19.0 >Reporter: yisha zhou >Assignee: yisha zhou >Priority: Major > Fix For: 1.19.0 > > > Currently when no user jars exist, system classLoader will be used to load > classes as default. However, if we use powerMock to create some ITCases, the > framework will utilize JavassistMockClassLoader to load classes. Forcing the > use of the system classLoader can lead to class loading conflict issue. > Therefore we should use Thread.currentThread().getContextClassLoader() > instead of > ClassLoader.getSystemClassLoader() here. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (FLINK-34402) Class loading conflicts when using PowerMock in ITcase.
[ https://issues.apache.org/jira/browse/FLINK-34402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17815164#comment-17815164 ] yisha zhou commented on FLINK-34402: [~fangyong] May I ask for your opinion on discussion between me and Xintong? > Class loading conflicts when using PowerMock in ITcase. > > > Key: FLINK-34402 > URL: https://issues.apache.org/jira/browse/FLINK-34402 > Project: Flink > Issue Type: Bug > Components: Runtime / Coordination >Affects Versions: 1.19.0 >Reporter: yisha zhou >Assignee: yisha zhou >Priority: Major > Fix For: 1.19.0 > > > Currently when no user jars exist, system classLoader will be used to load > classes as default. However, if we use powerMock to create some ITCases, the > framework will utilize JavassistMockClassLoader to load classes. Forcing the > use of the system classLoader can lead to class loading conflict issue. > Therefore we should use Thread.currentThread().getContextClassLoader() > instead of > ClassLoader.getSystemClassLoader() here. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (FLINK-34402) Class loading conflicts when using PowerMock in ITcase.
[ https://issues.apache.org/jira/browse/FLINK-34402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17815162#comment-17815162 ] yisha zhou commented on FLINK-34402: [~xtsong] I cannot find other use case which the issue affects either. Only applying the changes to my internal fork is ok to me. However I get a minor concern, when I update my Flink version, I should copy this work version by version. If this change can be merged into the master, no copy work should be done. Returning to the change, do you have any concern about the replacement? > Class loading conflicts when using PowerMock in ITcase. > > > Key: FLINK-34402 > URL: https://issues.apache.org/jira/browse/FLINK-34402 > Project: Flink > Issue Type: Bug > Components: Runtime / Coordination >Affects Versions: 1.19.0 >Reporter: yisha zhou >Assignee: yisha zhou >Priority: Major > Fix For: 1.19.0 > > > Currently when no user jars exist, system classLoader will be used to load > classes as default. However, if we use powerMock to create some ITCases, the > framework will utilize JavassistMockClassLoader to load classes. Forcing the > use of the system classLoader can lead to class loading conflict issue. > Therefore we should use Thread.currentThread().getContextClassLoader() > instead of > ClassLoader.getSystemClassLoader() here. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (FLINK-34402) Class loading conflicts when using PowerMock in ITcase.
[ https://issues.apache.org/jira/browse/FLINK-34402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17815142#comment-17815142 ] Xintong Song commented on FLINK-34402: -- Thanks for the detailed information. I think this should not be a bug. The reported issue doesn't really affect any real production use case. And Flink is not designed to be executed with a PowerMockRunner or JavassistMockClassLoader. I'm also not familiar with other approaches for checking API calling times and query outputs, other than manually implementing them. But if the cases are only for internal usages in your company, you don't really need to follow the community code-style guides. My suggestion would be to apply the proposed changes only to your internal fork. WDYT? > Class loading conflicts when using PowerMock in ITcase. > > > Key: FLINK-34402 > URL: https://issues.apache.org/jira/browse/FLINK-34402 > Project: Flink > Issue Type: Bug > Components: Runtime / Coordination >Affects Versions: 1.19.0 >Reporter: yisha zhou >Assignee: yisha zhou >Priority: Major > Fix For: 1.19.0 > > > Currently when no user jars exist, system classLoader will be used to load > classes as default. However, if we use powerMock to create some ITCases, the > framework will utilize JavassistMockClassLoader to load classes. Forcing the > use of the system classLoader can lead to class loading conflict issue. > Therefore we should use Thread.currentThread().getContextClassLoader() > instead of > ClassLoader.getSystemClassLoader() here. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (FLINK-34402) Class loading conflicts when using PowerMock in ITcase.
[
https://issues.apache.org/jira/browse/FLINK-34402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17815132#comment-17815132
]
yisha zhou commented on FLINK-34402:
Hi [~xtsong] ,
For the first question, yes, it's an ITCase that I'm going to add. The code is
only for internal use in our company and not intended to be contributed to the
community. But I can provide the keys to reproduce the issue.
It's ITcase for Redis SQL Connector. I added a RunWith and PrepareForTest
annotation, and then created a StreamTableEnvironment and used it to execute
queries. Those queries would make mocked class to work and be checked.
{code:java}
@RunWith(PowerMockRunner.class)
@PrepareForTest(RedisClientTableUtils.class)
···
EnvironmentSettings envSettings =
EnvironmentSettings.newInstance().inStreamingMode().build();
tEnv = StreamTableEnvironment.create(env, envSettings);{code}
Then an exception is thrown:
{code:java}
Caused by: java.lang.RuntimeException:
org.apache.flink.runtime.client.JobInitializationException: Could not start the
JobMaster.
at org.apache.flink.util.ExceptionUtils.rethrow(ExceptionUtils.java:321)
at
org.apache.flink.util.function.FunctionUtils.lambda$uncheckedFunction$2(FunctionUtils.java:75)
at
java.util.concurrent.CompletableFuture.uniApply(CompletableFuture.java:602)
at
java.util.concurrent.CompletableFuture$UniApply.tryFire(CompletableFuture.java:577)
at
java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:443)
at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
at
java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
at
java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)
Caused by: org.apache.flink.runtime.client.JobInitializationException: Could
not start the JobMaster.
at
org.apache.flink.runtime.jobmaster.DefaultJobMasterServiceProcess.lambda$new$0(DefaultJobMasterServiceProcess.java:97)
at
java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:760)
at
java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:736)
at
java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:474)
at
java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1595)
at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
Caused by: java.util.concurrent.CompletionException:
java.lang.ClassCastException: org.apache.flink.api.common.ExecutionConfig
cannot be cast to org.apache.flink.api.common.ExecutionConfig
at
java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:273)
at
java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:280)
at
java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1592)
... 3 more
Caused by: java.lang.ClassCastException:
org.apache.flink.api.common.ExecutionConfig cannot be cast to
org.apache.flink.api.common.ExecutionConfig
at
org.apache.flink.runtime.scheduler.DefaultSchedulerFactory.createInstance(DefaultSchedulerFactory.java:98)
at
org.apache.flink.runtime.jobmaster.DefaultSlotPoolServiceSchedulerFactory.createScheduler(DefaultSlotPoolServiceSchedulerFactory.java:119)
at
org.apache.flink.runtime.jobmaster.JobMaster.createScheduler(JobMaster.java:403)
at org.apache.flink.runtime.jobmaster.JobMaster.(JobMaster.java:379)
at
org.apache.flink.runtime.jobmaster.factories.DefaultJobMasterServiceFactory.internalCreateJobMasterService(DefaultJobMasterServiceFactory.java:123)
at
org.apache.flink.runtime.jobmaster.factories.DefaultJobMasterServiceFactory.lambda$createJobMasterService$0(DefaultJobMasterServiceFactory.java:95)
at
org.apache.flink.util.function.FunctionUtils.lambda$uncheckedSupplier$4(FunctionUtils.java:112)
at
java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1590)
... 3 more {code}
For question 2, I found if no users jars exist the system classLoader will be
used in
[BlobLibraryCacheManager#242|https://github.com/apache/flink/blob/f1fba33d85a802b896170ff3cdb0107ee082c44a/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/BlobLibraryCacheManager.java#L242]
. The related issue is
[FLINK-32265|https://issues.apache.org/jira/browse/FLINK-32265]
The fixed code should be:
{code:java}
private UserCodeClassLoader getOrResolveClassLoader(
Collection libraries, Collection classPaths)
throws IOException {
synchronized (lockObject) {
verifyIsNotReleased();
if (resolvedClassLoader == null) {
[jira] [Commented] (FLINK-34402) Class loading conflicts when using PowerMock in ITcase.
[ https://issues.apache.org/jira/browse/FLINK-34402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17815096#comment-17815096 ] Xintong Song commented on FLINK-34402: -- Hi [~nilerzhou], The description of this issue is a bit unclear to me. Could you provide a bit more information? - In which ITCase did you run into the problem? If it's an ITCase that is not yet exist and you are planning to add, it would be helpful to also provide the codes so others can reproduce the issue. - Where exactly are you suggesting to replace `ClassLoader.getSystemClassLoader()` with `Thread.currentThread().getContextClassLoader()`? BTW, it is discouraged to use Mockito for testing. See the Code Style and Quality Guide for more details. https://flink.apache.org/how-to-contribute/code-style-and-quality-common/#avoid-mockito---use-reusable-test-implementations > Class loading conflicts when using PowerMock in ITcase. > > > Key: FLINK-34402 > URL: https://issues.apache.org/jira/browse/FLINK-34402 > Project: Flink > Issue Type: Bug > Components: Runtime / Coordination >Affects Versions: 1.19.0 >Reporter: yisha zhou >Assignee: yisha zhou >Priority: Major > Fix For: 1.19.0 > > > Currently when no user jars exist, system classLoader will be used to load > classes as default. However, if we use powerMock to create some ITCases, the > framework will utilize JavassistMockClassLoader to load classes. Forcing the > use of the system classLoader can lead to class loading conflict issue. > Therefore we should use Thread.currentThread().getContextClassLoader() > instead of > ClassLoader.getSystemClassLoader() here. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (FLINK-34402) Class loading conflicts when using PowerMock in ITcase.
[ https://issues.apache.org/jira/browse/FLINK-34402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17815094#comment-17815094 ] yisha zhou commented on FLINK-34402: [~zjureel] hi, could you please assign this task to me? I'd like to fix it. > Class loading conflicts when using PowerMock in ITcase. > > > Key: FLINK-34402 > URL: https://issues.apache.org/jira/browse/FLINK-34402 > Project: Flink > Issue Type: Bug > Components: Runtime / Coordination >Affects Versions: 1.19.0 >Reporter: yisha zhou >Priority: Major > Fix For: 1.19.0 > > > Currently when no user jars exist, system classLoader will be used to load > classes as default. However, if we use powerMock to create some ITCases, the > framework will utilize JavassistMockClassLoader to load classes. Forcing the > use of the system classLoader can lead to class loading conflict issue. > Therefore we should use Thread.currentThread().getContextClassLoader() > instead of > ClassLoader.getSystemClassLoader() here. -- This message was sent by Atlassian Jira (v8.20.10#820010)
