----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48886/#review140263 -----------------------------------------------------------
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java (line 103) <https://reviews.apache.org/r/48886/#comment205615> is this for all fragments, or only external ones? Should say and handle appropriately in get if it's only for external. llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java (line 158) <https://reviews.apache.org/r/48886/#comment205619> hmm... was this patch updated? this can throw in many circumstances. Why do we have to do it like this anyway? It seems that the query stuff will repeatedly be cleaned up and recreated. llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java (line 217) <https://reviews.apache.org/r/48886/#comment205617> nit llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java (line 232) <https://reviews.apache.org/r/48886/#comment205620> can fragments start before the cleanip task runs and cause it to be invalid? I think it might make more sense to have delay before even considering the cleanup... llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java (line 248) <https://reviews.apache.org/r/48886/#comment205621> nit: constant? llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java (line 293) <https://reviews.apache.org/r/48886/#comment205623> I don't think wait on Future is a real method, it's just Object::wait; get() should be called. Also timeout might be helpful llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java (line 416) <https://reviews.apache.org/r/48886/#comment205624> nit: unneeded? llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java (line 421) <https://reviews.apache.org/r/48886/#comment205625> should this be checked during the attempt to get the child locks? - Sergey Shelukhin On June 17, 2016, 10:31 p.m., Jason Dere wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48886/ > ----------------------------------------------------------- > > (Updated June 17, 2016, 10:31 p.m.) > > > Review request for hive and Siddharth Seth. > > > Bugs: HIVE-14052 > https://issues.apache.org/jira/browse/HIVE-14052 > > > Repository: hive-git > > > Description > ------- > > Add a hook to call run QueryTracker.queryComplete if there are no more > fragments for this query. > This cleanup runs on delay and can be cancelled if another fragment request > comes in with the same query ID. > > > Diffs > ----- > > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java > ded84c1 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java > c7e9d32 > > llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/QueryTracker.java > a965872 > ql/src/java/org/apache/hadoop/hive/llap/LlapOutputFormatService.java > 825488f > ql/src/test/org/apache/hadoop/hive/llap/TestLlapOutputFormat.java 2288cd4 > > Diff: https://reviews.apache.org/r/48886/diff/ > > > Testing > ------- > > > Thanks, > > Jason Dere > >
