mynameborat commented on a change in pull request #1337: SAMZA-2498: [Job
coordinator isolation] Classloader isolation for SamzaApplication.describe
execution in job coordinator
URL: https://github.com/apache/samza/pull/1337#discussion_r401403476
##########
File path:
samza-core/src/main/java/org/apache/samza/classloader/IsolatingClassLoaderFactory.java
##########
@@ -121,31 +121,72 @@
* </li>
* </ol>
*/
- public ClassLoader buildClassLoader() {
- // start at the user.dir to find the resources for the classpaths
- File baseJobDirectory = new File(System.getProperty("user.dir"));
- File apiLibDirectory = libDirectory(new File(baseJobDirectory,
DependencyIsolationUtils.FRAMEWORK_API_DIRECTORY));
+ public ClassLoader buildMainClassLoader() {
+ File apiLibDirectory = buildApiLibDirectory();
LOG.info("Using API lib directory: {}", apiLibDirectory);
- File infrastructureLibDirectory =
- libDirectory(new File(baseJobDirectory,
DependencyIsolationUtils.FRAMEWORK_INFRASTRUCTURE_DIRECTORY));
+ File infrastructureLibDirectory = buildInfrastructureLibDirectory();
LOG.info("Using infrastructure lib directory: {}",
infrastructureLibDirectory);
- File applicationLibDirectory =
- libDirectory(new File(baseJobDirectory,
DependencyIsolationUtils.APPLICATION_DIRECTORY));
+ File applicationLibDirectory = buildApplicationLibDirectory();
LOG.info("Using application lib directory: {}", applicationLibDirectory);
ClassLoader apiClassLoader = buildApiClassLoader(apiLibDirectory);
ClassLoader applicationClassLoader =
buildApplicationClassLoader(applicationLibDirectory, apiLibDirectory,
apiClassLoader);
// the classloader to return is the one with the infrastructure classpath
- return buildInfrastructureClassLoader(infrastructureLibDirectory,
baseJobDirectory, apiLibDirectory, apiClassLoader,
+ return buildInfrastructureClassLoader(infrastructureLibDirectory,
apiLibDirectory, apiClassLoader,
applicationClassLoader);
}
+ /**
+ * Build a classloader for loading {@link
org.apache.samza.application.SamzaApplication} when in isolation mode.
+ *
+ * It is not sufficient to use the classloader from {@link
#buildMainClassLoader()} to load the
+ * {@link org.apache.samza.application.SamzaApplication}, because the
describe method might need to use classes in the
+ * framework infrastructure classloader (e.g. framework descriptors). The
classloader from
+ * {@link #buildMainClassLoader()} would not be able to delegate from the
application classloader to the framework
+ * infrastructure classloader.
+ *
+ * If the classloader from {@link #buildMainClassLoader()} is passed as the
{@code parentClassLoader}, then the
+ * describe method can use framework infrastructure components and
application-specific components.
+ *
+ * Implementation notes:
+ *
+ * This classloader will only directly load the {@link
org.apache.samza.application.SamzaApplication} class, and it
+ * will delegate all other classloading to the {@code parentClassLoader}.
This allows framework descriptors to be
+ * loaded from the framework infrastructure classloader. In addition, this
allows the application processing classes
+ * to be loaded from the initial application classloader when calling
describe. This is desirable because the core
+ * processing flows will delegate to the initial application classloader as
well, and it is necessary that the same
+ * application classloader is used for all classes in the application
processing flows.
+ *
+ * @param samzaApplicationClassName Class name of the {@link
org.apache.samza.application.SamzaApplication}
+ * @param parentClassLoader {@link ClassLoader} to delegate to. Should be
the {@link #buildMainClassLoader()}.
+ * @return {@link ClassLoader} to use for loading the {@link
org.apache.samza.application.SamzaApplication}
+ */
+ public ClassLoader buildSamzaApplicationClassLoader(String
samzaApplicationClassName,
+ ClassLoader parentClassLoader) {
+ File applicationLibDirectory = buildApplicationLibDirectory();
+ return LoaderBuilder.anIsolatingLoader()
+ // SamzaApplication is in the application classpath
+ .withClasspath(getClasspathAsURIs(applicationLibDirectory))
+ // getClasspathAsURIs should only return JARs within
applicationLibDirectory anyways, but doing it to be safe
+
.withOriginRestriction(OriginRestriction.denyByDefault().allowingDirectory(applicationLibDirectory,
false))
+ .withParentRelationship(DelegateRelationshipBuilder.builder()
+ .withDelegateClassLoader(parentClassLoader)
+ // almost everything should come from the parent classloader
+ .addDelegatePreferredClassPredicate(new GlobMatcher("*"))
+ // blacklisting the SamzaApplication class from the parent so that
it gets loaded by this classloader
+ .addBlacklistedClassPredicate(new
GlobMatcher(samzaApplicationClassName))
Review comment:
Is it possible that by only blacklisting `samzaApplicationClass`, we end up
class loading some of the dependencies of user code through parent class loader
assuming both parent and application class loader are capable of classing
loading that class?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
