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