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




llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java
Lines 82 (patched)
<https://reviews.apache.org/r/63319/#comment267306>

    random is not thread safe



llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java
Lines 370 (patched)
<https://reviews.apache.org/r/63319/#comment267307>

    nit: save in ctor/init?



llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java
Lines 550 (patched)
<https://reviews.apache.org/r/63319/#comment267308>

    I don't quite understand the logic around semaphore. For now it's 
try-acquired on send and released on response, so it seems like for retries 
after the response (in the callback) it's unneeded, and out of bounds retries 
like this won't work because it's still acquired while the response is still 
pending, so tryacquire would return false. Perhaps a comment would be helpful 
on the field about the lifecycle for acquire/release


- Sergey Shelukhin


On Oct. 26, 2017, 2:42 a.m., Jason Dere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63319/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2017, 2:42 a.m.)
> 
> 
> Review request for hive and Siddharth Seth.
> 
> 
> Bugs: HIVE-17908
>     https://issues.apache.org/jira/browse/HIVE-17908
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> - Pending requests should retry if killTask received
> - Change retry delay to use exponential backoff
> 
> 
> Diffs
> -----
> 
>   
> llap-client/src/java/org/apache/hadoop/hive/llap/ext/LlapTaskUmbilicalExternalClient.java
>  aa94e54 
> 
> 
> Diff: https://reviews.apache.org/r/63319/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Dere
> 
>

Reply via email to