You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Xuefu Zhang <xz...@cloudera.com> on 2017/05/01 17:13:40 UTC
Re: Review Request 58865: HIVE-16552: Limit the number of tasks a
Spark job may contain
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58865/
-----------------------------------------------------------
(Updated May 1, 2017, 5:13 p.m.)
Review request for hive.
Changes
-------
Updated patch to reflect Lefty's feedback.
Bugs: HIVE-16552
https://issues.apache.org/jira/browse/HIVE-16552
Repository: hive-git
Description
-------
See JIRA description
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d3ea824
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/2/
Changes: https://reviews.apache.org/r/58865/diff/1-2/
Testing
-------
Test locally
Thanks,
Xuefu Zhang
Re: Review Request 58865: HIVE-16552: Limit the number of tasks a
Spark job may contain
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58865/
-----------------------------------------------------------
(Updated May 3, 2017, 6:14 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 (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 84398c6
itests/src/test/resources/testconfiguration.properties 753f3a9
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
ql/src/test/queries/clientnegative/spark_job_max_tasks.q PRE-CREATION
ql/src/test/results/clientnegative/spark/spark_job_max_tasks.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/58865/diff/4/
Changes: https://reviews.apache.org/r/58865/diff/3-4/
Testing
-------
Test locally
Thanks,
Xuefu Zhang
Re: Review Request 58865: HIVE-16552: Limit the number of tasks a
Spark job may contain
Posted by Rui Li <ru...@intel.com>.
> 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
>
>
Re: Review Request 58865: HIVE-16552: Limit the number of tasks a
Spark job may contain
Posted by Xuefu Zhang <xz...@cloudera.com>.
> 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
This is not new code.
> 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?
Jobmonitor prints it on console, while the log here is written to hive.log.
> 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.
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.
- Xuefu
-----------------------------------------------------------
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
>
>
Re: Review Request 58865: HIVE-16552: Limit the number of tasks a
Spark job may contain
Posted by Rui Li <ru...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58865/#review173689
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkTask.java
Lines 132 (patched)
<https://reviews.apache.org/r/58865/#comment246728>
I think the log is unnecessary because the failure should already be logged in the monitor
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkTask.java
Lines 135 (patched)
<https://reviews.apache.org/r/58865/#comment246729>
Same as above. Can we consolidate the logs a bit?
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
Lines 104 (patched)
<https://reviews.apache.org/r/58865/#comment246731>
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.
- Rui Li
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
>
>
Re: Review Request 58865: HIVE-16552: Limit the number of tasks a
Spark job may contain
Posted by Xuefu Zhang <xz...@cloudera.com>.
-----------------------------------------------------------
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 (updated)
-----
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/
Changes: https://reviews.apache.org/r/58865/diff/2-3/
Testing
-------
Test locally
Thanks,
Xuefu Zhang
Re: Review Request 58865: HIVE-16552: Limit the number of tasks a
Spark job may contain
Posted by Chao Sun <ch...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58865/#review173407
-----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 3341 (patched)
<https://reviews.apache.org/r/58865/#comment246405>
nit: add new line after "may have."
nit: maximu -> maximum
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkTask.java
Lines 135 (patched)
<https://reviews.apache.org/r/58865/#comment246406>
Maybe also log the actual limit number?
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
Lines 106 (patched)
<https://reviews.apache.org/r/58865/#comment246407>
Maybe we should skip counting this if `sparkJobMaxTaskCount` is -1?
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
Lines 106 (patched)
<https://reviews.apache.org/r/58865/#comment246590>
Also, we don't need to compute this if `sparkJobMaxTaskCount` is -1.
- Chao Sun
On May 1, 2017, 5:13 p.m., Xuefu Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58865/
> -----------------------------------------------------------
>
> (Updated May 1, 2017, 5:13 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 d3ea824
> 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/2/
>
>
> Testing
> -------
>
> Test locally
>
>
> Thanks,
>
> Xuefu Zhang
>
>
Re: Review Request 58865: HIVE-16552: Limit the number of tasks a
Spark job may contain
Posted by Rui Li <ru...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58865/#review173556
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/SparkTask.java
Lines 135 (patched)
<https://reviews.apache.org/r/58865/#comment246543>
The log is incorrect because cancelling the job doesn't mean killing the application.
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/status/RemoteSparkJobMonitor.java
Lines 106 (patched)
<https://reviews.apache.org/r/58865/#comment246544>
I think the total task count needs only be computed once. It shouldn't change during the execution of the job, assuming we don't count failed/retried tasks.
- Rui Li
On May 1, 2017, 5:13 p.m., Xuefu Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58865/
> -----------------------------------------------------------
>
> (Updated May 1, 2017, 5:13 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 d3ea824
> 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/2/
>
>
> Testing
> -------
>
> Test locally
>
>
> Thanks,
>
> Xuefu Zhang
>
>