You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by j....@gmail.com on 2017/11/16 22:00:09 UTC
Review Request 63890: HIVE-18025: Push resource plan changes to
tez/unmanaged sessions
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63890/
-----------------------------------------------------------
Review request for hive and Sergey Shelukhin.
Bugs: HIVE-18025
https://issues.apache.org/jira/browse/HIVE-18025
Repository: hive-git
Description
-------
HIVE-18025: Push resource plan changes to tez/unmanaged sessions
Diffs
-----
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersNoTezSessionPool.java bcce3dcf4b29cb0595d2b54cbf279066b23f4008
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersTezSessionPoolManager.java b377275fadc702fdacc83d085ed1874861a5a0b8
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java e7af5e004fb560b574b82f6d1b60517511802f37
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java 4e48f15e5fb749c4d1feb3fdd01bec216a0f1199
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolSession.java 769b24a35d905869b3571f61855471344a76e50d
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 1fe5859490c7bc00d0b91ad325dbb8d826761a62
ql/src/java/org/apache/hadoop/hive/ql/wm/MetastoreGlobalTriggersFetcher.java 87c007f69b8c2405e19f362d528141e0ffff0046
standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java 1d21908d4d7e8b07751eb2f75579112774deb561
Diff: https://reviews.apache.org/r/63890/diff/1/
Testing
-------
Thanks,
Prasanth_J
Re: Review Request 63890: HIVE-18025: Push resource plan changes to
tez/unmanaged sessions
Posted by j....@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63890/
-----------------------------------------------------------
(Updated Nov. 18, 2017, 12:58 a.m.)
Review request for hive and Sergey Shelukhin.
Changes
-------
Addressed review comments.
Bugs: HIVE-18025
https://issues.apache.org/jira/browse/HIVE-18025
Repository: hive-git
Description
-------
HIVE-18025: Push resource plan changes to tez/unmanaged sessions
Diffs (updated)
-----
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersNoTezSessionPool.java bcce3dc
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersTezSessionPoolManager.java b377275
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java e7af5e0
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java 4e48f15
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolSession.java 769b24a
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 3990f95
ql/src/java/org/apache/hadoop/hive/ql/wm/MetastoreGlobalTriggersFetcher.java 87c007f
service/src/java/org/apache/hive/service/server/HiveServer2.java 5a6d0c8
Diff: https://reviews.apache.org/r/63890/diff/3/
Changes: https://reviews.apache.org/r/63890/diff/2-3/
Testing
-------
Thanks,
Prasanth_J
Re: Review Request 63890: HIVE-18025: Push resource plan changes to
tez/unmanaged sessions
Posted by j....@gmail.com.
> On Nov. 17, 2017, 8:18 p.m., Sergey Shelukhin wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java
> > Lines 517 (patched)
> > <https://reviews.apache.org/r/63890/diff/2/?file=1896465#file1896465line524>
> >
> > this seems to get all triggers for the pools... I can file a follow-up jira, I think we need to make addition of triggers to Tez more explicit
Until we have explicit GLOBAL flag, this will get all triggers that is tied to default pool path.
> On Nov. 17, 2017, 8:18 p.m., Sergey Shelukhin wrote:
> > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java
> > Line 329 (original), 329 (patched)
> > <https://reviews.apache.org/r/63890/diff/2/?file=1896470#file1896470line329>
> >
> > this looks like a change to generated code without a thrift change. The null check (or isSet check?) should be in the caller
Oops. Didn't realize this is generated file. Moved it to caller.
- Prasanth_J
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63890/#review191387
-----------------------------------------------------------
On Nov. 17, 2017, 2:20 a.m., Prasanth_J wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63890/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2017, 2:20 a.m.)
>
>
> Review request for hive and Sergey Shelukhin.
>
>
> Bugs: HIVE-18025
> https://issues.apache.org/jira/browse/HIVE-18025
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-18025: Push resource plan changes to tez/unmanaged sessions
>
>
> Diffs
> -----
>
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersNoTezSessionPool.java bcce3dc
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersTezSessionPoolManager.java b377275
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java e7af5e0
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java 4e48f15
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolSession.java 769b24a
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 3990f95
> ql/src/java/org/apache/hadoop/hive/ql/wm/MetastoreGlobalTriggersFetcher.java 87c007f
> service/src/java/org/apache/hive/service/server/HiveServer2.java 5a6d0c8
> standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java 1d21908
>
>
> Diff: https://reviews.apache.org/r/63890/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Prasanth_J
>
>
Re: Review Request 63890: HIVE-18025: Push resource plan changes to
tez/unmanaged sessions
Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63890/#review191387
-----------------------------------------------------------
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersNoTezSessionPool.java
Line 59 (original), 58 (patched)
<https://reviews.apache.org/r/63890/#comment269182>
doesn't look like pool is used
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
Line 716 (original), 717 (patched)
<https://reviews.apache.org/r/63890/#comment269183>
nit: WorkloadManager.getInstance is enough to determine if it's enabled.
WM is initialized based on queue name only
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
Lines 743 (patched)
<https://reviews.apache.org/r/63890/#comment269184>
hmm... why is this an either or case? it's possible to have both in the same HS2. Should it call the else in both cases?
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java
Line 511 (original), 511 (patched)
<https://reviews.apache.org/r/63890/#comment269186>
openSessions appears to be synchronized elsewhere, accessing it here may not be thread safe. Might make sense to call this under the same sync block that makes the changes to the list.
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java
Lines 517 (patched)
<https://reviews.apache.org/r/63890/#comment269185>
this seems to get all triggers for the pools... I can file a follow-up jira, I think we need to make addition of triggers to Tez more explicit
service/src/java/org/apache/hive/service/server/HiveServer2.java
Line 218 (original), 220 (patched)
<https://reviews.apache.org/r/63890/#comment269187>
same about else
standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java
Line 329 (original), 329 (patched)
<https://reviews.apache.org/r/63890/#comment269189>
this looks like a change to generated code without a thrift change. The null check (or isSet check?) should be in the caller
- Sergey Shelukhin
On Nov. 17, 2017, 2:20 a.m., Prasanth_J wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63890/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2017, 2:20 a.m.)
>
>
> Review request for hive and Sergey Shelukhin.
>
>
> Bugs: HIVE-18025
> https://issues.apache.org/jira/browse/HIVE-18025
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-18025: Push resource plan changes to tez/unmanaged sessions
>
>
> Diffs
> -----
>
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersNoTezSessionPool.java bcce3dc
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersTezSessionPoolManager.java b377275
> ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java e7af5e0
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java 4e48f15
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolSession.java 769b24a
> ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 3990f95
> ql/src/java/org/apache/hadoop/hive/ql/wm/MetastoreGlobalTriggersFetcher.java 87c007f
> service/src/java/org/apache/hive/service/server/HiveServer2.java 5a6d0c8
> standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java 1d21908
>
>
> Diff: https://reviews.apache.org/r/63890/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Prasanth_J
>
>
Re: Review Request 63890: HIVE-18025: Push resource plan changes to
tez/unmanaged sessions
Posted by j....@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63890/
-----------------------------------------------------------
(Updated Nov. 17, 2017, 2:20 a.m.)
Review request for hive and Sergey Shelukhin.
Changes
-------
misc fixes and rebase
Bugs: HIVE-18025
https://issues.apache.org/jira/browse/HIVE-18025
Repository: hive-git
Description
-------
HIVE-18025: Push resource plan changes to tez/unmanaged sessions
Diffs (updated)
-----
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersNoTezSessionPool.java bcce3dc
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersTezSessionPoolManager.java b377275
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java e7af5e0
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java 4e48f15
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolSession.java 769b24a
ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 3990f95
ql/src/java/org/apache/hadoop/hive/ql/wm/MetastoreGlobalTriggersFetcher.java 87c007f
service/src/java/org/apache/hive/service/server/HiveServer2.java 5a6d0c8
standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java 1d21908
Diff: https://reviews.apache.org/r/63890/diff/2/
Changes: https://reviews.apache.org/r/63890/diff/1-2/
Testing
-------
Thanks,
Prasanth_J