----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72499/#review220748 -----------------------------------------------------------
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java Lines 610 (patched) <https://reviews.apache.org/r/72499/#comment309420> What's the reason for time based expiry? Is it because UGI expires after 24 hrs? Else, I would have expected long living cache with blocking queue of bounded size. Queue should be bounded by number of executors anyways, since having more connections than executors probably won't be needed. llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java Lines 617 (patched) <https://reviews.apache.org/r/72499/#comment309419> LOG.debug llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java Lines 638 (patched) <https://reviews.apache.org/r/72499/#comment309423> Bound this queue by number of executors? llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java Lines 640 (patched) <https://reviews.apache.org/r/72499/#comment309421> LOG.debug llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java Lines 646-651 (patched) <https://reviews.apache.org/r/72499/#comment309422> Is this logic needed? You already have valueloader in get() which must return a ugi, so it cant be null. llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java Lines 663 (patched) <https://reviews.apache.org/r/72499/#comment309424> if its null, then its programming error. Better to not do this null check and offer without checking for null. - Ashutosh Chauhan On May 12, 2020, 12:06 p.m., Rajesh Balamohan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72499/ > ----------------------------------------------------------- > > (Updated May 12, 2020, 12:06 p.m.) > > > Review request for hive, Ashutosh Chauhan and Gopal V. > > > Bugs: HIVE-23446 > https://issues.apache.org/jira/browse/HIVE-23446 > > > Repository: hive-git > > > Description > ------- > > Currently UGI pool is maintained at QueryInfo level. However, when short > queries and lots of AMs are there, it ends missing IPC connection cache. Too > many connections are are also established. Patch tries to avoid that by > maintaining this at ContainerRunner level. It retains the current behaviour > of having multiple connection to same AM (otherwise can get bottlenecked on > single connection) > > > Diffs > ----- > > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java > 6a13b55e69 > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryInfo.java > 00fed15d2b > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java > eae8e08540 > > llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java > 50dec4759e > > > Diff: https://reviews.apache.org/r/72499/diff/1/ > > > Testing > ------- > > > Thanks, > > Rajesh Balamohan > >
