> On Nov. 21, 2017, 4:09 a.m., Sergey Shelukhin wrote: > > llap-server/bin/llapDaemon.sh > > Line 116 (original), 116 (patched) > > <https://reviews.apache.org/r/63972/diff/1/?file=1897948#file1897948line116> > > > > what is this change for?
The process that needs to be run as launch_command (check templates.py) needs to be a foreground process. That's why I was asking offline if anything else other than this llap app-package uses this script. > On Nov. 21, 2017, 4:09 a.m., Sergey Shelukhin wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapOptionsProcessor.java > > Line 65 (original), 65 (patched) > > <https://reviews.apache.org/r/63972/diff/1/?file=1897951#file1897951line65> > > > > hmm... is it possible to keep old names as backward compat for scripts? > > or accept both names The packaging in 3.x has changed significantly and hence trying to do anything for the sake of backward compatibility won't be worthwhile. Also, since Slider is being deprecated and going forward the Apache project is being wind down, there is no point in keeping any traces of its name. All traces of Slider keyword will be completely removed once all features of the packaging will be migrated to YARN Services. > On Nov. 21, 2017, 4:09 a.m., Sergey Shelukhin wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapSliderUtils.java > > Line 47 (original), 46 (patched) > > <https://reviews.apache.org/r/63972/diff/1/?file=1897953#file1897953line54> > > > > is this still needed? Will be removed in the next pass once status and diagnostics are migrated. Its mentioned in the jira as well. > On Nov. 21, 2017, 4:09 a.m., Sergey Shelukhin wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapSliderUtils.java > > Lines 176 (patched) > > <https://reviews.apache.org/r/63972/diff/1/?file=1897953#file1897953line210> > > > > is it a good idea to ignore all exceptions? the old code used to ignore > > UnknownApp.. only Unfortunately the YARN Service API does not throw such an exception and hence it does not provide a differentiator. Since this is in start cluster, its okay to ignore and move on, since if destroy was not done successfully, start will fail. > On Nov. 21, 2017, 4:09 a.m., Sergey Shelukhin wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapSliderUtils.java > > Lines 182 (patched) > > <https://reviews.apache.org/r/63972/diff/1/?file=1897953#file1897953line216> > > > > should this be configurable? or at least a constant Every app-package can choose their own location where they upload the package. It doesn't has to be this specific path. As long as this same path is specified in Yarnfile (refer templates.py). The reason I did not create a constant is because there is no other refernce of this path in the Java land. The only other reference is from templates.py. Nevertheless I can create a constant for it in the next pass. > On Nov. 21, 2017, 4:09 a.m., Sergey Shelukhin wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapStatusServiceDriver.java > > Line 352 (original) > > <https://reviews.apache.org/r/63972/diff/1/?file=1897954#file1897954line365> > > > > where did the timeout logic go? I saw the code above that seems to fail > > immediately when the app is not running, but no timeout logic This method is in LlapSliderUtils and is being used from there. > On Nov. 21, 2017, 4:09 a.m., Sergey Shelukhin wrote: > > llap-server/src/main/resources/package.py > > Line 184 (original) > > <https://reviews.apache.org/r/63972/diff/1/?file=1897957#file1897957line184> > > > > hmm... that doesn't do anything anymore? I think at least the scripts > > would still be needed, right? nope, scripts are not needed anymore. > On Nov. 21, 2017, 4:09 a.m., Sergey Shelukhin wrote: > > llap-server/src/main/resources/templates.py > > Lines 43 (patched) > > <https://reviews.apache.org/r/63972/diff/1/?file=1897959#file1897959line133> > > > > how does it know what LLAP_DAEMON_OPTS is, and other stuff like > > HEAPSIZE? it doesn't seem to be mentioned elsewhere in the patch and > > doesn't seem to follow the convention (e.g. component name is LLAP without > > DAEMON). Just checking; it used to have a fancy name like site.global. ... YARN Services automatically takes care of setting the env variables defined in Yarnfile before calling the launch_command. - Gour ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63972/#review191563 ----------------------------------------------------------- On Nov. 21, 2017, 1:37 a.m., Gour Saha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63972/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2017, 1:37 a.m.) > > > Review request for hive and Sergey Shelukhin. > > > Bugs: HIVE-18037 > https://issues.apache.org/jira/browse/HIVE-18037 > > > Repository: hive-git > > > Description > ------- > > First phase of migration of slider based llap app-package to YARN Services in > Hadoop 3.x. There will be follow up changes to migrate status, log links, > diagnostics and completely eliminate Slider dependency. > > > Diffs > ----- > > bin/ext/llap.sh 0462d26 > binary-package-licenses/README ef127e3 > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java bd25bc7 > jdbc/pom.xml 8710a8b > > llap-client/src/java/org/apache/hadoop/hive/llap/registry/LlapServiceInstance.java > 30b1810 > llap-server/bin/llapDaemon.sh 4945473 > llap-server/changes_for_non_slider_install.txt ec20fe1 > llap-server/pom.xml 176110d > > llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapOptionsProcessor.java > d01598c > llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapServiceDriver.java > 5090be2 > llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapSliderUtils.java > a0af554 > > llap-server/src/java/org/apache/hadoop/hive/llap/cli/LlapStatusServiceDriver.java > 296a851 > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryInfo.java > d2e9396 > llap-server/src/main/resources/llap.py 26756ce > llap-server/src/main/resources/package.py 21c34e9 > llap-server/src/main/resources/params.py 8972ba1 > llap-server/src/main/resources/templates.py 3d747a2 > packaging/src/main/assembly/bin.xml 84686ee > > > Diff: https://reviews.apache.org/r/63972/diff/1/ > > > Testing > ------- > > Package created and successfully deployed in a Hadoop 3.0 cluster, using cmd > line shell script and programatically via Java APIs. > > > File Attachments > ---------------- > > HIVE-18037.001.patch > > https://reviews.apache.org/media/uploaded/files/2017/11/21/e0844c04-be9b-4334-80b0-bae05e9ed885__HIVE-18037.001.patch > > > Thanks, > > Gour Saha > >
