> On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > > Lines 3055 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905198#file1905198line3055> > > > > nit: should this be a single setting? none/json/text?
Done. > On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersMoveWorkloadManager.java > > Lines 109 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905201#file1905201line109> > > > > nit: return event reports everything as nulls but kill event has the > > data. Should it be consistent (as nulls maybe)? Fixed. > On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/AmPluginNode.java > > Lines 35 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905204#file1905204line39> > > > > is this one useful? iirc it's some bogus job id created just for the > > plugin Not really. Removed. > On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillMoveTriggerActionHandler.java > > Lines 45 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905205#file1905205line47> > > > > should the type of the map be changed instead? Done. > On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillMoveTriggerActionHandler.java > > Line 45 (original), 50 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905205#file1905205line52> > > > > does context need to be visible here? seems like smth that can be > > hidden inside WM No. Reverted the visibility of KillQueryContext. > On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java > > Lines 111 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905207#file1905207line111> > > > > is there some way to have ignore by defualt? this is kind of annoying > > to add to every property. Also seems like isLegacy..., wmContext, etc are > > not covered Fixed. Only place that requires explict ignore (added comment for that). Removed from all other places. > On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WMEvent.java > > Lines 27 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905210#file1905210line27> > > > > nit^2: should all these classes start with "Wm" not WM? to be > > consistent :) Done > On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WmTezSession.java > > Lines 52 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905211#file1905211line52> > > > > is this useful to the user? it's some internal counter Actually not. Removed it. > On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java > > Lines 1061 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905212#file1905212line1074> > > > > hmm... any particular reason why this is here? if they are both put > > into the request at the same time, that means it could be put into the > > session there too, right? Good catch. This is not required since getSession already attaches WmContext to session we don't need this here. > On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java > > Line 1255 (original), 1308 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905212#file1905212line1321> > > > > would it be helpful to record how long get took? that would be > > queue+init time In the new format, also recording elapsed time for all events. > On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > ql/src/java/org/apache/hadoop/hive/ql/wm/WMContext.java > > Lines 2 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905219#file1905219line2> > > > > is this just a move+json? And generic events related stuffs. No longer kust triggers. > On Dec. 1, 2017, 9:29 p.m., Sergey Shelukhin wrote: > > ql/src/java/org/apache/hadoop/hive/ql/wm/WMContext.java > > Lines 166 (patched) > > <https://reviews.apache.org/r/64222/diff/2/?file=1905219#file1905219line166> > > > > jira? HIVE-18204 - Prasanth_J ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64222/#review192545 ----------------------------------------------------------- On Dec. 1, 2017, 12:21 a.m., Prasanth_J wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64222/ > ----------------------------------------------------------- > > (Updated Dec. 1, 2017, 12:21 a.m.) > > > Review request for hive and Sergey Shelukhin. > > > Bugs: HIVE-18088 > https://issues.apache.org/jira/browse/HIVE-18088 > > > Repository: hive-git > > > Description > ------- > > HIVE-18088: Add WM event traces at query level for debugging > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java ada2318 > itests/hive-unit/pom.xml ea5b7b9 > > itests/hive-unit/src/test/java/org/apache/hive/jdbc/AbstractJdbcTriggersTest.java > 235e6c3 > > itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersMoveWorkloadManager.java > a983855 > ql/src/java/org/apache/hadoop/hive/ql/Context.java 97b52b0 > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 389a1a6 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/AmPluginNode.java 0509cbc > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillMoveTriggerActionHandler.java > 94b189b > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillTriggerActionHandler.java > 8c60b6f > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java 6fa3724 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezTask.java af77f30 > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TriggerValidatorRunnable.java > 5821659 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WMEvent.java PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WmTezSession.java d61c531 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java ecdcf12 > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManagerFederation.java > 0a9fa72 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/PrintSummary.java > 5bb6bf1 > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/TezJobMonitor.java > 3dd4b31 > > ql/src/java/org/apache/hadoop/hive/ql/hooks/PostExecWMEventsSummaryPrinter.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/wm/Trigger.java e41b460 > ql/src/java/org/apache/hadoop/hive/ql/wm/TriggerContext.java 16072c3 > ql/src/java/org/apache/hadoop/hive/ql/wm/WMContext.java PRE-CREATION > ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java > 78df962 > > > Diff: https://reviews.apache.org/r/64222/diff/2/ > > > Testing > ------- > > > Thanks, > > Prasanth_J > >
