Re: Review Request 66075: HIVE-18962 add WM task state to Tez AM heartbeat

2018-03-15 Thread Jason Dere

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


Ship it!




Ship It!

- Jason Dere


On March 15, 2018, 8:53 p.m., Sergey Shelukhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66075/
> ---
> 
> (Updated March 15, 2018, 8:53 p.m.)
> 
> 
> Review request for hive and Jason Dere.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> see jira
> 
> 
> Diffs
> -
> 
>   
> llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java
>  d97b1567ea 
>   
> llap-common/src/java/org/apache/hadoop/hive/llap/protocol/LlapTaskUmbilicalProtocol.java
>  a2dca1b319 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/AMReporter.java 
> b784360b6c 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java
>  75d8577d6a 
>   
> llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestAMReporter.java
>  bde354649a 
>   
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java
>  a4bd4dfd88 
>   
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
>  66de3b805a 
>   
> llap-tez/src/test/org/apache/hadoop/hive/llap/tezplugins/TestLlapTaskSchedulerService.java
>  90b31e4fcd 
> 
> 
> Diff: https://reviews.apache.org/r/66075/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>



Re: Review Request 66075: HIVE-18962 add WM task state to Tez AM heartbeat

2018-03-15 Thread Sergey Shelukhin

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

(Updated March 15, 2018, 8:53 p.m.)


Review request for hive and Jason Dere.


Repository: hive-git


Description
---

see jira


Diffs (updated)
-

  
llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java
 d97b1567ea 
  
llap-common/src/java/org/apache/hadoop/hive/llap/protocol/LlapTaskUmbilicalProtocol.java
 a2dca1b319 
  llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/AMReporter.java 
b784360b6c 
  
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java
 75d8577d6a 
  
llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestAMReporter.java
 bde354649a 
  
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java
 a4bd4dfd88 
  
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
 66de3b805a 
  
llap-tez/src/test/org/apache/hadoop/hive/llap/tezplugins/TestLlapTaskSchedulerService.java
 90b31e4fcd 


Diff: https://reviews.apache.org/r/66075/diff/2/

Changes: https://reviews.apache.org/r/66075/diff/1-2/


Testing
---


Thanks,

Sergey Shelukhin



Re: Review Request 66075: HIVE-18962 add WM task state to Tez AM heartbeat

2018-03-15 Thread Sergey Shelukhin


> On March 15, 2018, 5:53 p.m., Jason Dere wrote:
> > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/AMReporter.java
> > Line 541 (original), 547 (patched)
> > 
> >
> > The previous implementation will lock the tasks member from any 
> > additional updates when getting the TaskSnapshot. The newer implementation 
> > could potentially allow another caller to update the tasks map - does that 
> > matter at all here?

Shouldn't matter; added a comment.


- Sergey


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


On March 14, 2018, 11:45 p.m., Sergey Shelukhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66075/
> ---
> 
> (Updated March 14, 2018, 11:45 p.m.)
> 
> 
> Review request for hive and Jason Dere.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> see jira
> 
> 
> Diffs
> -
> 
>   
> llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java
>  d97b1567ea 
>   
> llap-common/src/java/org/apache/hadoop/hive/llap/protocol/LlapTaskUmbilicalProtocol.java
>  a2dca1b319 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/AMReporter.java 
> b784360b6c 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java
>  75d8577d6a 
>   
> llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestAMReporter.java
>  bde354649a 
>   
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java
>  a4bd4dfd88 
>   
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
>  66de3b805a 
>   
> llap-tez/src/test/org/apache/hadoop/hive/llap/tezplugins/TestLlapTaskSchedulerService.java
>  90b31e4fcd 
> 
> 
> Diff: https://reviews.apache.org/r/66075/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>



Re: Review Request 66075: HIVE-18962 add WM task state to Tez AM heartbeat

2018-03-15 Thread Jason Dere

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




llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/AMReporter.java
Lines 412 (patched)


nit whitespace



llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/AMReporter.java
Line 541 (original), 547 (patched)


The previous implementation will lock the tasks member from any additional 
updates when getting the TaskSnapshot. The newer implementation could 
potentially allow another caller to update the tasks map - does that matter at 
all here?



llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/AMReporter.java
Lines 590 (patched)


nit


- Jason Dere


On March 14, 2018, 11:45 p.m., Sergey Shelukhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66075/
> ---
> 
> (Updated March 14, 2018, 11:45 p.m.)
> 
> 
> Review request for hive and Jason Dere.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> see jira
> 
> 
> Diffs
> -
> 
>   
> llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java
>  d97b1567ea 
>   
> llap-common/src/java/org/apache/hadoop/hive/llap/protocol/LlapTaskUmbilicalProtocol.java
>  a2dca1b319 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/AMReporter.java 
> b784360b6c 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java
>  75d8577d6a 
>   
> llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestAMReporter.java
>  bde354649a 
>   
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java
>  a4bd4dfd88 
>   
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
>  66de3b805a 
>   
> llap-tez/src/test/org/apache/hadoop/hive/llap/tezplugins/TestLlapTaskSchedulerService.java
>  90b31e4fcd 
> 
> 
> Diff: https://reviews.apache.org/r/66075/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>



Re: Review Request 66075: HIVE-18962 add WM task state to Tez AM heartbeat

2018-03-14 Thread Sergey Shelukhin

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




llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
Lines 3020 (patched)


bogus comment


- Sergey Shelukhin


On March 14, 2018, 11:45 p.m., Sergey Shelukhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66075/
> ---
> 
> (Updated March 14, 2018, 11:45 p.m.)
> 
> 
> Review request for hive and Jason Dere.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> see jira
> 
> 
> Diffs
> -
> 
>   
> llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java
>  d97b1567ea 
>   
> llap-common/src/java/org/apache/hadoop/hive/llap/protocol/LlapTaskUmbilicalProtocol.java
>  a2dca1b319 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/AMReporter.java 
> b784360b6c 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java
>  75d8577d6a 
>   
> llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestAMReporter.java
>  bde354649a 
>   
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java
>  a4bd4dfd88 
>   
> llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
>  66de3b805a 
>   
> llap-tez/src/test/org/apache/hadoop/hive/llap/tezplugins/TestLlapTaskSchedulerService.java
>  90b31e4fcd 
> 
> 
> Diff: https://reviews.apache.org/r/66075/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>



Review Request 66075: HIVE-18962 add WM task state to Tez AM heartbeat

2018-03-14 Thread Sergey Shelukhin

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

Review request for hive and Jason Dere.


Repository: hive-git


Description
---

see jira


Diffs
-

  
llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java
 d97b1567ea 
  
llap-common/src/java/org/apache/hadoop/hive/llap/protocol/LlapTaskUmbilicalProtocol.java
 a2dca1b319 
  llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/AMReporter.java 
b784360b6c 
  
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java
 75d8577d6a 
  
llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/comparator/TestAMReporter.java
 bde354649a 
  
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskCommunicator.java
 a4bd4dfd88 
  
llap-tez/src/java/org/apache/hadoop/hive/llap/tezplugins/LlapTaskSchedulerService.java
 66de3b805a 
  
llap-tez/src/test/org/apache/hadoop/hive/llap/tezplugins/TestLlapTaskSchedulerService.java
 90b31e4fcd 


Diff: https://reviews.apache.org/r/66075/diff/1/


Testing
---


Thanks,

Sergey Shelukhin