search

azkaban / azkaban-plugins

Plugins for Azkaban.
https://azkaban.github.io
Apache License 2.0
130 stars 178 forks source link

Temporarily resolve Azkaban common classpath in all job type #274

Closed kunkun-tang closed 6 years ago

kunkun-tang commented 6 years ago

The merged pull request created a new az-core module, which should be the baseline foundation class for all other az modules. It caused a bug in AZ production cluster:

Exception in thread "main" java.lang.NoClassDefFoundError: azkaban/jobExecutor/AbstractJob at java.lang.ClassLoader.defineClass1(Native Method) at java.lang.ClassLoader.defineClass(ClassLoader.java:763) at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142) at java.net.URLClassLoader.defineClass(URLClassLoader.java:467) at java.net.URLClassLoader.access$100(URLClassLoader.java:73) at java.net.URLClassLoader$1.run(URLClassLoader.java:368) at java.net.URLClassLoader$1.run(URLClassLoader.java:362) at java.security.AccessController.doPrivileged(Native Method) at java.net.URLClassLoader.findClass(URLClassLoader.java:361) at java.lang.ClassLoader.loadClass(ClassLoader.java:424) at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331) at java.lang.ClassLoader.loadClass(ClassLoader.java:357)

The issue is that launched job doesn't specify azkaban-common classpath. The root cause comes from the old legacy codebase in Azkaban-plugins. Originally every jobtype tries to locate which jar includes class Props and include that jar as part of the classpath. However, the mentioned pull request broke it since Props is moved to az-core module now. So we locate another class in Azkaban-common and have the jar wrapping the class.

The solution is just temporary, and we should wholely refactor these class at some point in future.

burgerkingeater commented 6 years ago

I don't think it's an elegant fix. Can we put those classpath in global.properties? or at least construct it from JavaProcessJob.java which avoids the code duplication. Check JavaProcessJob#getClassPaths(). cc @HappyRay

kunkun-tang commented 6 years ago

@chengren311 Yes, it's not. Your concern is valid, but we probably should consider moving all those classpaths together at a future point and comprehensively test it.

burgerkingeater commented 6 years ago

nit: there's typo in your description: logacy