> On May 14, 2020, 4:31 a.m., Ashutosh Chauhan wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java > > Lines 610 (patched) > > <https://reviews.apache.org/r/72499/diff/1/?file=2231454#file2231454line611> > > > > 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. > > Rajesh Balamohan wrote: > Since this is based on the AM. So if AM dies after sometime (due to > inactivity, as in no-DAG submissions), these UGIs will be auto purged after > 10 minutes.
In LLAP, AM doesn't die due to inactivity, its long living. It may die because of crash though. So, then should this expiry be longer. 3 hrs? > On May 14, 2020, 4:31 a.m., Ashutosh Chauhan wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java > > Lines 646-651 (patched) > > <https://reviews.apache.org/r/72499/diff/1/?file=2231454#file2231454line647> > > > > Is this logic needed? You already have valueloader in get() which must > > return a ugi, so it cant be null. > > Rajesh Balamohan wrote: > Yes, value loader is for initial miss. This is to avoid single connection > becoming a contention for AM communication. > https://issues.apache.org/jira/browse/HIVE-16634 Not sure I follow. Can you add comments in code to explain the need for this? > On May 14, 2020, 4:31 a.m., Ashutosh Chauhan wrote: > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java > > Lines 663 (patched) > > <https://reviews.apache.org/r/72499/diff/1/?file=2231454#file2231454line664> > > > > if its null, then its programming error. Better to not do this null > > check and offer without checking for null. better to throw NPE then to leak ugi failing to return to pool. - Ashutosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72499/#review220748 ----------------------------------------------------------- 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 > >
