nateab commented on code in PR #27579:
URL: https://github.com/apache/flink/pull/27579#discussion_r3693896443
##########
flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/BlobLibraryCacheManager.java:
##########
@@ -238,25 +238,41 @@ private UserCodeClassLoader getOrResolveClassLoader(
verifyIsNotReleased();
if (resolvedClassLoader == null) {
- boolean systemClassLoader =
- wrapsSystemClassLoader && libraries.isEmpty() &&
classPaths.isEmpty();
- resolvedClassLoader =
- new ResolvedClassLoader(
- systemClassLoader
- ?
ClassLoader.getSystemClassLoader()
- : createUserCodeClassLoader(
- jobId, applicationId,
libraries, classPaths),
- libraries,
- classPaths,
- systemClassLoader);
+ resolvedClassLoader = createResolvedClassLoader(libraries,
classPaths);
} else {
- resolvedClassLoader.verifyClassLoader(libraries,
classPaths);
+ try {
+ resolvedClassLoader.verifyClassLoader(libraries,
classPaths);
+ } catch (IllegalStateException e) {
+ LOG.warn(
+ "Library cache entry for job {} has a
classloader resolved with different "
+ + "library BLOBs than requested. This
can happen during JobManager "
+ + "failover. Re-creating the
classloader with the new blob keys.",
+ jobId,
+ e);
Review Comment:
Agreed, and it was a real problem rather than a style one, verifyClassLoader
throws for both a library-set and a class-path mismatch, so the catch also
accepted genuinely different JARs. The four assertions I'd had to weaken were
exactly that case.
Fixed by removing the catch entirely: the comparison is now content-hash
based, so identical content matches and anything genuinely different still
throws. All four assertions are restored.
--
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]
