> On May 3, 2017, 3:35 a.m., Rui Li wrote: > >
Xuefu, the patch looks good to me overall. Thanks for the work. Do you think we should add some negative test case for it? > On May 3, 2017, 3:35 a.m., Rui Li wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkTask.java > > Lines 132 (patched) > > <https://reviews.apache.org/r/58865/diff/3/?file=1705971#file1705971line132> > > > > I think the log is unnecessary because the failure should already be > > logged in the monitor > > Xuefu Zhang wrote: > This is not new code. Do you mean "LOG.info("Failed to submit Spark job " + sparkJobID);" is not new code? I don't find it in the current SparkTask.java. > On May 3, 2017, 3:35 a.m., Rui Li wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkTask.java > > Lines 135 (patched) > > <https://reviews.apache.org/r/58865/diff/3/?file=1705971#file1705971line135> > > > > Same as above. Can we consolidate the logs a bit? > > Xuefu Zhang wrote: > Jobmonitor prints it on console, while the log here is written to > hive.log. The console.printInfo method does both printing and logging: public void printInfo(String info, String detail, boolean isSilent) { if (!isSilent) { getInfoStream().println(info); } LOG.info(info + StringUtils.defaultString(detail)); } > On May 3, 2017, 3:35 a.m., Rui Li wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java > > Lines 104 (patched) > > <https://reviews.apache.org/r/58865/diff/3/?file=1705972#file1705972line104> > > > > Maybe I was being misleading. I mean we can compute the total task only > > once when the job first reaches RUNNING state, i.e. in the "if (!running)". > > At this point, the total count is determined and won't change. > > Xuefu Zhang wrote: > Yeah. However, I'd like to keep the state transition to running first > before breaking up and returning rc=4. In fact, if we lose the transition, > Hive actually goes into an instable state. What you said was what I tried in > first place. I see. Thanks for the explanation. - Rui ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58865/#review173689 ----------------------------------------------------------- On May 2, 2017, 6:49 p.m., Xuefu Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58865/ > ----------------------------------------------------------- > > (Updated May 2, 2017, 6:49 p.m.) > > > Review request for hive. > > > Bugs: HIVE-16552 > https://issues.apache.org/jira/browse/HIVE-16552 > > > Repository: hive-git > > > Description > ------- > > See JIRA description > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 84398c6 > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkTask.java 32a7730 > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java > dd73f3e > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/SparkJobMonitor.java > 0b224f2 > > > Diff: https://reviews.apache.org/r/58865/diff/3/ > > > Testing > ------- > > Test locally > > > Thanks, > > Xuefu Zhang > >
