zentol commented on code in PR #22718:
URL: https://github.com/apache/flink/pull/22718#discussion_r1233971542
##########
flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/BlobLibraryCacheManager.java:
##########
@@ -391,6 +407,7 @@ private ResolvedClassLoader(
classPaths.add(url.toString());
}
this.libraries = new HashSet<>(requiredLibraries);
+ this.systemClassLoader = systemClassLoader;
Review Comment:
also rename to `wrapsSystemClassloader`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/execution/librarycache/TestingLibraryCacheManager.java:
##########
@@ -53,9 +55,11 @@ public static Builder newBuilder() {
public static final class Builder {
private Runnable shutdownRunnable = () -> {};
- private Function<JobID, LibraryCacheManager.ClassLoaderLease>
+ private BiFunction<JobID, Boolean,
LibraryCacheManager.ClassLoaderLease>
Review Comment:
revert?
##########
flink-runtime/src/test/java/org/apache/flink/runtime/execution/librarycache/BlobLibraryCacheManagerTest.java:
##########
@@ -478,15 +489,22 @@ public void
closingAllLeases_willReleaseUserCodeClassLoader() throws IOException
final LibraryCacheManager.ClassLoaderLease classLoaderLease2 =
libraryCacheManager.registerClassLoaderLease(jobId);
- classLoaderLease1.getOrResolveClassLoader(Collections.emptyList(),
Collections.emptyList());
+ UserCodeClassLoader userCodeClassLoader =
+ classLoaderLease1.getOrResolveClassLoader(
+ Collections.emptyList(), Collections.emptyList());
classLoaderLease1.release();
assertFalse(classLoader.isClosed());
classLoaderLease2.release();
- assertTrue(classLoader.isClosed());
+ if (!wrapsSystemClassLoader) {
Review Comment:
nit: invert condition; there's no need for negating things here
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
