----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64222/#review192545 -----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java Lines 3055 (patched) <https://reviews.apache.org/r/64222/#comment270762> nit: should this be a single setting? none/json/text? itests/hive-unit/src/test/java/org/apache/hive/jdbc/AbstractJdbcTriggersTest.java Lines 162 (patched) <https://reviews.apache.org/r/64222/#comment270764> is there no better way to do this? itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestTriggersMoveWorkloadManager.java Lines 109 (patched) <https://reviews.apache.org/r/64222/#comment270765> nit: return event reports everything as nulls but kill event has the data. Should it be consistent (as nulls maybe)? ql/src/java/org/apache/hadoop/hive/ql/exec/tez/AmPluginNode.java Lines 35 (patched) <https://reviews.apache.org/r/64222/#comment270766> is this one useful? iirc it's some bogus job id created just for the plugin ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillMoveTriggerActionHandler.java Lines 45 (patched) <https://reviews.apache.org/r/64222/#comment270767> should the type of the map be changed instead? ql/src/java/org/apache/hadoop/hive/ql/exec/tez/KillMoveTriggerActionHandler.java Line 45 (original), 50 (patched) <https://reviews.apache.org/r/64222/#comment270768> does context need to be visible here? seems like smth that can be hidden inside WM ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java Lines 111 (patched) <https://reviews.apache.org/r/64222/#comment270770> 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 ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WMEvent.java Lines 27 (patched) <https://reviews.apache.org/r/64222/#comment270778> nit^2: should all these classes start with "Wm" not WM? to be consistent :) ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WmTezSession.java Lines 52 (patched) <https://reviews.apache.org/r/64222/#comment270779> is this useful to the user? it's some internal counter ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java Lines 1061 (patched) <https://reviews.apache.org/r/64222/#comment270784> 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? ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java Line 1255 (original), 1308 (patched) <https://reviews.apache.org/r/64222/#comment270786> would it be helpful to record how long get took? that would be queue+init time ql/src/java/org/apache/hadoop/hive/ql/wm/WMContext.java Lines 2 (patched) <https://reviews.apache.org/r/64222/#comment270788> is this just a move+json? ql/src/java/org/apache/hadoop/hive/ql/wm/WMContext.java Lines 166 (patched) <https://reviews.apache.org/r/64222/#comment270789> jira? - Sergey Shelukhin 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 > >
