Mailing-List: contact dev-help@hive.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hive.apache.org
Content-Type: multipart/alternative;
 boundary="===============3843057965943136436=="
MIME-Version: 1.0
Subject: Re: Review Request 62091: HIVE-17386 support LLAP workload management
 in HS2 (low level only)
From: Zhiyuan Yang <sjtu.yzy@gmail.com>
To: Zhiyuan Yang <sjtu.yzy@gmail.com>, Gunther Hagleitner <gunther@apache.com>,
 Siddharth Seth <sseth@apache.org>
Cc: Sergey Shelukhin <sergey@hortonworks.com>, hive <dev@hive.apache.org>
Date: Tue, 12 Sep 2017 18:56:53 -0000
Message-ID: <20170912185653.5242.79153@reviews-vm2.apache.org>
Auto-Submitted: auto-generated
Sender: Zhiyuan Yang <noreply@reviews.apache.org>
References: <20170905185300.63444.87829@reviews-vm2.apache.org>
In-Reply-To: <20170905185300.63444.87829@reviews-vm2.apache.org>
Reply-To: Zhiyuan Yang <sjtu.yzy@gmail.com>
archived-at: Tue, 12 Sep 2017 18:57:02 -0000

--===============3843057965943136436==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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


common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 2385-2386 (patched)
<https://reviews.apache.org/r/62091/#comment261351>

    Should mention setting this conf means enable workload management


ql/src/java/org/apache/hadoop/hive/ql/exec/tez/GuaranteedTasksAllocator.java
Lines 101-106 (patched)
<https://reviews.apache.org/r/62091/#comment261379>

    Why is this here given it's already a daemon thread


ql/src/java/org/apache/hadoop/hive/ql/exec/tez/GuaranteedTasksAllocator.java
Lines 147 (patched)
<https://reviews.apache.org/r/62091/#comment261433>

    Additional define statement will be better.


ql/src/java/org/apache/hadoop/hive/ql/exec/tez/GuaranteedTasksAllocator.java
Lines 191 (patched)
<https://reviews.apache.org/r/62091/#comment261361>

    How would AM registry help in AM recovery? If that's not the case, this piece means any update during AM failure & recovery will fail the session, which make AM recovery in vain.


ql/src/java/org/apache/hadoop/hive/ql/exec/tez/GuaranteedTasksAllocator.java
Lines 201-215 (patched)
<https://reviews.apache.org/r/62091/#comment261362>

    You are really determined to knock out that field...


ql/src/java/org/apache/hadoop/hive/ql/exec/tez/LlapPluginEndpointClientImpl.java
Lines 61 (patched)
<https://reviews.apache.org/r/62091/#comment261251>

    git apply complains
    
    HIVE-17386.02.patch:1162: trailing whitespace.
      }
    warning: 1 line adds whitespace errors.


ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java
Lines 220 (patched)
<https://reviews.apache.org/r/62091/#comment261507>

    Wrong log message


service/src/java/org/apache/hive/service/server/HiveServer2.java
Lines 169 (patched)
<https://reviews.apache.org/r/62091/#comment261514>

    Where is the code that really put this wm instance in use? Additional jira?


- Zhiyuan Yang


On Sept. 5, 2017, 6:52 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62091/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2017, 6:52 p.m.)
> 
> 
> Review request for hive, Zhiyuan Yang, Gunther Hagleitner, and Siddharth Seth.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 6de07d2e76 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java b3677322ca 
>   llap-client/src/java/org/apache/hadoop/hive/llap/tez/LlapProtocolClientProxy.java b6501842e8 
>   llap-client/src/test/org/apache/hadoop/hive/llap/TestAsyncPbRpcProxy.java 1c4f0e7a09 
>   llap-common/src/java/org/apache/hadoop/hive/llap/AsyncPbRpcProxy.java 7726794fea 
>   llap-common/src/java/org/apache/hadoop/hive/llap/impl/LlapPluginProtocolClientImpl.java 19e81e6fa5 
>   llap-common/src/java/org/apache/hadoop/hive/llap/impl/ProtobufProxy.java fa99536bea 
>   llap-common/src/protobuf/LlapPluginProtocol.proto 39349b119d 
>   llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java cf8bd469dc 
>   llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/endpoint/LlapPluginServerImpl.java f3c0d5213f 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java 93a36c612d 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/GuaranteedTasksAllocator.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/LlapPluginEndpointClient.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/LlapPluginEndpointClientImpl.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/QueryAllocationManager.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPool.java 4f58565a4c 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java 1f4705c083 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolSession.java 005eeedc02 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java fe5c6a1e45 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezTask.java f1f10286a3 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WmTezSession.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/TezJobMonitor.java 9e2846ca6c 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/LlapClusterStateForCompile.java 7a02a563e9 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/tez/SampleTezSessionState.java 973c0cc630 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestGuaranteedTaskAllocator.java PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestTezSessionPool.java d2b98c46ca 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java PRE-CREATION 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java e5f449122b 
> 
> 
> Diff: https://reviews.apache.org/r/62091/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


--===============3843057965943136436==--