----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72276/#review220102 -----------------------------------------------------------
Looking pretty good overall, I just have a few questions/comments. itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniLlapArrow.java Lines 415 (patched) <https://reviews.apache.org/r/72276/#comment308408> Might be worth to extract the expected string as a constant? itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithServiceDiscovery.java Lines 94 (patched) <https://reviews.apache.org/r/72276/#comment308409> Can be private if not used elsewhere service/src/java/org/apache/hive/service/server/KillQueryImpl.java Line 150 (original), 176 (patched) <https://reviews.apache.org/r/72276/#comment308410> Shouldn't we return if there are no ops to kill? I think a subsequent killOperations() call here might throw an NPE. service/src/java/org/apache/hive/service/server/KillQueryZookeeperManager.java Lines 106 (patched) <https://reviews.apache.org/r/72276/#comment308411> nit: typo: namespace service/src/java/org/apache/hive/service/server/KillQueryZookeeperManager.java Lines 243 (patched) <https://reviews.apache.org/r/72276/#comment308412> I'm fine with not exposing these in HiveConf (that's already a monster) but we could at least extract these as constants in this class. service/src/java/org/apache/hive/service/server/KillQueryZookeeperManager.java Lines 449 (patched) <https://reviews.apache.org/r/72276/#comment308415> Shouldn't we clear the progress flag here? service/src/java/org/apache/hive/service/server/KillQueryZookeeperManager.java Lines 453 (patched) <https://reviews.apache.org/r/72276/#comment308414> this is a no-op here - Adam Szita On March 27, 2020, 10:08 a.m., Peter Varga wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72276/ > ----------------------------------------------------------- > > (Updated March 27, 2020, 10:08 a.m.) > > > Review request for hive and Adam Szita. > > > Repository: hive-git > > > Description > ------- > > KILL <queryId|querytag> command was implemented in: > > https://issues.apache.org/jira/browse/HIVE-17483 > https://issues.apache.org/jira/browse/HIVE-20549 > But it is not working in an environment where service discovery is enabled > and more than one HS2 instance is running (except for manually sending the > kill query to all HS2 instance). > > Solution: > > If a HS2 instance can't kill a query locally, it should post a kill query > request to the Zookeeper > Every HS2 should watch the Zookeeper for kill query requests and if its > running on that instance kill it > Authorization of kill query should work the same > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 34df01e60e > > itests/hive-unit/src/test/java/org/apache/hive/jdbc/BaseJdbcWithMiniLlap.java > 3973ec9270 > > itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniLlapArrow.java > 68a515ccbe > > itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithServiceDiscovery.java > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/service/cli/thrift/TestMiniHS2StateWithNoZookeeper.java > 99e681e5b2 > > itests/hive-unit/src/test/java/org/apache/hive/service/server/TestKillQueryZookeeperManager.java > PRE-CREATION > itests/util/src/main/java/org/apache/hive/jdbc/miniHS2/MiniHS2.java > 1b60a51ebd > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionState.java > 8becef1cd3 > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java > 9e497545b5 > service/src/java/org/apache/hive/service/cli/session/SessionManager.java > 277519cba5 > service/src/java/org/apache/hive/service/server/HiveServer2.java 181ea5d6d5 > service/src/java/org/apache/hive/service/server/KillQueryImpl.java > 883e32bd2e > > service/src/java/org/apache/hive/service/server/KillQueryZookeeperManager.java > PRE-CREATION > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/common/ZooKeeperHiveHelper.java > 71d8651712 > > > Diff: https://reviews.apache.org/r/72276/diff/1/ > > > Testing > ------- > > > Thanks, > > Peter Varga > >