hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sergey Shelukhin <ser...@hortonworks.com>
Subject Re: Review Request 62706: HIVE-17473 implement workload management pools
Date Wed, 18 Oct 2017 20:44:43 GMT


> On Oct. 17, 2017, 10:48 p.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/UserPoolMapping.java
> > Lines 70 (patched)
> > <https://reviews.apache.org/r/62706/diff/1/?file=1840587#file1840587line70>
> >
> >     nit: throw if defaultPoolName is still null here.

Why? I actually wonder how this is going to work w.r.t. Tez. I think default pool name might
just mean Tez.


> On Oct. 17, 2017, 10:48 p.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
> > Lines 137 (patched)
> > <https://reviews.apache.org/r/62706/diff/1/?file=1840588#file1840588line141>
> >
> >     should we do some validation here? To check for max depth of tree. May be we
shouldn't allow >2 depth. Should we also add limits to how wide the tree can be. Or max
children (cannot be greater than some fraction of executor count).

Validation will be performed in a separate jira on apply command, see comments


> On Oct. 17, 2017, 10:48 p.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
> > Line 126 (original), 225 (patched)
> > <https://reviews.apache.org/r/62706/diff/1/?file=1840588#file1840588line230>
> >
> >     does this launch all sessions synchronously? should we make sure the created
sessions are open and are not in accepted state?

This only adds sessions objects. I actually refactored this in the next patch to use a factory
instead of passing them in.
The sessions are started async, see TezSessionPool


> On Oct. 17, 2017, 10:48 p.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
> > Lines 329 (patched)
> > <https://reviews.apache.org/r/62706/diff/1/?file=1840588#file1840588line338>
> >
> >     isn't this equivalent to sessionsClaimed.acquier()? blocked until acquire

No, we also want to check internalVersion periodically, in case if new plan . Although in
the next patch it will be gone anyway.


- Sergey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62706/#review188406
-----------------------------------------------------------


On Sept. 30, 2017, 12:57 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62706/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2017, 12:57 a.m.)
> 
> 
> Review request for hive, Zhiyuan Yang and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPool.java 4f2997b95b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/UserPoolMapping.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 3f621271cc 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java 7adf895077

>   service/src/java/org/apache/hive/service/server/HiveServer2.java 5cb973ca95 
> 
> 
> Diff: https://reviews.apache.org/r/62706/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message