Various fixes to embedded gobblin to allow it to run on mr mode. #1344

ibuenros · 2016-10-26T16:22:28Z

Major changes:

Embedded Gobblin will automatically add the correct classpath to distribute cache for a basic Gobblin run, and allow users to add their own jars.
Correctly set sys config in embedded mode.
Mr mode will only upload jars to distributed cache when setting up the job. This avoids having to upload them if there are no work units. Additionally, mr mode can now reuse a common jar directory to avoid new uploads for every run.
bin/gobblin.sh shell script will automatically load Hadoop classpath if HADOOP_HOME is defined.

ibuenros · 2016-10-26T16:25:02Z

@chavdar can you review?

coveralls · 2016-10-26T17:00:28Z

Changes Unknown when pulling 286f539 on ibuenros:embedded-working into * on linkedin:master*.

coveralls · 2016-10-26T17:38:07Z

Changes Unknown when pulling 286f539 on ibuenros:embedded-working into * on linkedin:master*.

chavdar · 2016-10-29T04:52:27Z

gobblin-data-management/src/main/java/gobblin/data/management/copy/CopySource.java

@@ -140,6 +140,10 @@

      final FileSystem sourceFs = getSourceFileSystem(state);
      final FileSystem targetFs = getTargetFileSystem(state);
+
+      log.info(String.format("Identified source file system at %s and target file system at %s.",


log.info("Identified source file system at {} and target file system at {}., ...) is more efficient.

chavdar · 2016-10-29T04:58:36Z

gobblin-runtime/src/main/java/gobblin/runtime/job_exec/JobLauncherExecutionDriver.java

+     * uses {@link #_sysConfig}, which is only initialized when the user runs {@link #withSysConfig(Configurable)} after
+     * construction.
+     */
+    public Launcher initialize() {


I don't like initialize() methods. How about the getMetrics() method check if _metrics is null and set it on the fly?

chavdar · 2016-10-29T04:59:34Z

gobblin-runtime/src/main/java/gobblin/runtime/job_exec/JobLauncherExecutionDriver.java

@@ -358,6 +370,9 @@ public Launcher withSysConfig(Configurable sysConfig) {
    /** Parent Gobblin instance */
    public Launcher withGobblinInstanceEnvironment(GobblinInstanceEnvironment gobblinInstance) {
      _gobblinEnv = Optional.of(gobblinInstance);
+      if (!_sysConfig.isPresent()) {


getSysConfig() is already doing that through getDefaultSysConfig().

chavdar · 2016-10-29T05:16:54Z

gobblin-runtime/src/main/java/gobblin/runtime/mapreduce/MRJobLauncher.java

      throws IOException {
    TimingEvent distributedCacheSetupTimer =
        this.eventSubmitter.getTimingEvent(TimingEvent.RunJobTimings.MR_DISTRIBUTED_CACHE_SETUP);

-    Path jarFileDir = new Path(this.mrJobDir, JARS_DIR_NAME);
+    Path jarFileDir = this.jarsDir;


I think this is dangerous as jarsDir will be shared across different jobs/runs. This may cause jar with mismatched versions to be stored.

I think you have two options:

Use both a job-specific and global directory. Use the latter if the name and size match, otherwise upload to the job-specific dir.

Have a more complex layout for the jar cache which allows different versions to be stored (like the gradle cache).

Jars are added individually to distributed classpath, so the same directory can safely have multiple versions of the same jar.

I am not sure I understand. Aren't all embedded gobblin jobs re-use the same path?

Line 362 in MRJobLauncher is this: DistributedCache.addFileToClassPath(destJarFile, this.conf, this.fs); , so the jars get added individually to the distributed classpath, the directory is just a container for all jars.

Never mind. If the jars are already versioned, it should work.

ibuenros · 2016-10-31T17:09:54Z

@chavdar addressed comments.

ibuenros force-pushed the embedded-working branch from bdd9370 to 286f539 Compare October 26, 2016 16:24

ibuenros assigned ibuenros and chavdar and unassigned ibuenros Oct 26, 2016

ibuenros force-pushed the embedded-working branch 2 times, most recently from 111b1bc to 0d3bc00 Compare October 27, 2016 16:12

Various fixes to embedded gobblin to allow it to run on mr mode.

71dba84

ibuenros force-pushed the embedded-working branch from 0d3bc00 to 71dba84 Compare October 27, 2016 16:30

chavdar suggested changes Oct 29, 2016

View reviewed changes

Addressed comments.

3f844d3

chavdar approved these changes Oct 31, 2016

View reviewed changes

ibuenros merged commit 9f09668 into apache:master Oct 31, 2016

ibuenros deleted the embedded-working branch October 31, 2016 22:03

pono unassigned chavdar Jul 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various fixes to embedded gobblin to allow it to run on mr mode. #1344

Various fixes to embedded gobblin to allow it to run on mr mode. #1344

ibuenros commented Oct 26, 2016 •

edited

ibuenros commented Oct 26, 2016

coveralls commented Oct 26, 2016

coveralls commented Oct 26, 2016

chavdar Oct 29, 2016

chavdar Oct 29, 2016

chavdar Oct 29, 2016

chavdar Oct 29, 2016

ibuenros Oct 31, 2016

chavdar Oct 31, 2016

ibuenros Oct 31, 2016

chavdar Oct 31, 2016

ibuenros commented Oct 31, 2016

Various fixes to embedded gobblin to allow it to run on mr mode. #1344

Various fixes to embedded gobblin to allow it to run on mr mode. #1344

Conversation

ibuenros commented Oct 26, 2016 • edited

ibuenros commented Oct 26, 2016

coveralls commented Oct 26, 2016

coveralls commented Oct 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibuenros commented Oct 31, 2016

ibuenros commented Oct 26, 2016 •

edited