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 63319: HIVE-17908: LLAP External client not correctly handling killTask for pending requests
Date Fri, 03 Nov 2017 20:12:41 GMT

-----------------------------------------------------------
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
> 
>


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