----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19929/#review42205 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java <https://reviews.apache.org/r/19929/#comment75952> hadoop Configuration deprecation feature should handle it. You don't have to explicitly check for both. Only checking fs.default.name is good enough. core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java <https://reviews.apache.org/r/19929/#comment75953> Just getAuthority() != null is enough as it contains host as well core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java <https://reviews.apache.org/r/19929/#comment75954> Since this is in a loop, we can have a map<String,FileSystem) of authority and filesystem and reuse. core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java <https://reviews.apache.org/r/19929/#comment75968> Can just do: Path pathToAdd = new Path(uri.normalize()); Services.get().get(HadoopAccessorService.class).addFileToClassPath(user, pathToAdd, conf); Make the below change to HadoopAccessorService.java: public void addFileToClassPath(String user, final Path file, final Configuration conf) throws IOException { ParamChecker.notEmpty(user, "user"); try { UserGroupInformation ugi = getUGI(user); ugi.doAs(new PrivilegedExceptionAction<Void>() { public Void run() throws Exception { Configuration defaultConf = new Configuration(); XConfiguration.copy(conf, defaultConf); //Doing this NOP add first to have the FS created and cached DistributedCache.addFileToClassPath(file, defaultConf); // Hadoop 0.20/1.x. if (defaultConf.get("mapred.job.classpath.files") != null) { // Duplicate hadoop 1.x code to workaround MAPREDUCE-2361 in Hadoop 0.20 // Refer OOZIE-1806. String filepath = file.toUri().getPath(); String classpath = conf.get("mapred.job.classpath.files"); conf.set("mapred.job.classpath.files", classpath == null ? filepath : classpath + System.getProperty("path.separator") + filepath); URI uri = file.getFileSystem(defaultConf).makeQualified(file).toUri(); DistributedCache.addCacheFile(uri, conf); } else { // Hadoop 0.23/2.x DistributedCache.addFileToClassPath(file, conf); } return null; } }); } catch (InterruptedException ex) { throw new IOException(ex); } } core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java <https://reviews.apache.org/r/19929/#comment75959> uri.normalize() core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java <https://reviews.apache.org/r/19929/#comment75973> Can be removed core/src/test/java/org/apache/oozie/action/hadoop/ActionExecutorTestCase.java <https://reviews.apache.org/r/19929/#comment75969> Can we do this only for TestJavaActionExecutor as it is not needed by other action executor test cases? Even though cluster setup is only done once for all tests, we will unnecessarily keep creating test dirs in both clusters. core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java <https://reviews.apache.org/r/19929/#comment75972> 3. Absolute (but not fully qualified) path located in the both filesystems Comment not in sync with actual test. job3.xml is a fully qualified path. Need to make it absolute and add another job4.xml for fully qualified path. core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java <https://reviews.apache.org/r/19929/#comment75977> Can you add a comment here and in 3.2.2.4 Syntax of WorkflowFunctionalSpecification.twiki, saying relative and absolute paths for job-xml point to the Namenode of the app path even if a different namenode is specified for the action. And that to point to a different namenode it should be fully qualified. core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java <https://reviews.apache.org/r/19929/#comment75978> This will not be ignored right if job3.xml was not fully qualified? Aren't absolute non-qualified paths picked from app path filesystem? core/src/test/java/org/apache/oozie/test/XFsTestCase.java <https://reviews.apache.org/r/19929/#comment75970> file system of the second cluster core/src/test/java/org/apache/oozie/test/XFsTestCase.java <https://reviews.apache.org/r/19929/#comment75971> return the FS test working directory in the second cluster - Rohini Palaniswamy On May 4, 2014, 7:05 p.m., Benjamin Zhitomirsky wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19929/ > ----------------------------------------------------------- > > (Updated May 4, 2014, 7:05 p.m.) > > > Review request for oozie. > > > Repository: oozie-git > > > Description > ------- > > When <name-node> element in Oozie workflow specifies a name node different > from the default one (specified in core-site.xml), the following > functionality doesn’t work properly: > ?Location of libraries specified via > oozie.service.WorkflowAppService.system.libpath. Oozie first (during launcher > configuration) tries to locate them using name node specified by the > <name-node> element, but later during job submission it expects this path to > be under the default Oozie name node > ?Processing of the job-xml element if job xml is specified via absolute path. > Oozie tries locate it under the default Oozie name node instead of the > name-node specified in action. > > Specifying non-default name node makes a lot of sense in Azure environment, > because it allows to submit the same job to different Hadoop clusters. > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java > 59ad143 > > core/src/test/java/org/apache/oozie/action/hadoop/ActionExecutorTestCase.java > bc2c1b6 > > core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java > 390ad3f > core/src/test/java/org/apache/oozie/test/XFsTestCase.java 18cb742 > core/src/test/java/org/apache/oozie/test/XTestCase.java 1536927 > > Diff: https://reviews.apache.org/r/19929/diff/ > > > Testing > ------- > > On deployed Hadoop cluster. Two tests were added. > > > Thanks, > > Benjamin Zhitomirsky > >
