> On Oct. 18, 2016, 7:28 p.m., Lefty Leverenz wrote: > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, lines 1766-1772 > > <https://reviews.apache.org/r/52923/diff/2/?file=1540463#file1540463line1766> > > > > 1. camel caps "zookeeper" -> "ZooKeeper" > > 2. all-caps "sql" -> "SQL" and final period > > 3. (to separate "SQL" from the commands) rephrase as "using the SQL > > commands UNLOCK TABLE, UNLOCK DATABASE." > > 4. are the UNLOCK commands part of ZooKeeper? then maybe you should say > > "the ZooKeeper commands" instead of "the SQL commands" > > > > Congrats on fixing some other typos before I saw them! > > Lefty Leverenz wrote: > Thanks Peter for nudging me to publish my comments. (Oops.)
Actually Zsombor pointed out the other typos for me, so forward your congratulation to him :) I made the last sentence (about removing the locks) more general, because it is a Zookeeper feature and if they change something I would not want to depend on them. At least that is what I think now :) - Peter ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52923/#review153057 ----------------------------------------------------------- On Oct. 19, 2016, 11:50 a.m., Peter Vary wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52923/ > ----------------------------------------------------------- > > (Updated Oct. 19, 2016, 11:50 a.m.) > > > Review request for hive, Ashutosh Chauhan, Marta Kuczora, Miklos Csanady, > namit jain, Sergio Pena, and Barna Zsombor Klara. > > > Bugs: HIVE-14979 > https://issues.apache.org/jira/browse/HIVE-14979 > > > Repository: hive-git > > > Description > ------- > > Adding a new configuration option to HiveConf to signal whether stale lock > removal is requested on startup. > Adding a new method to ZooKeeperHiveLockManager to remove stale locks > Modifying the HiveServer2 to instantiate a lock manager and call the new > method if defined by the configuration. > > Please take extra care when reviewing these: > - Modifying the lock fetching method to use the clientIp from the lock, and > not update with the current ip - Not sure why it was done before > - Instantiation of the lock manager - I might not chose the best method for it > > Open for any suggestions :) > > Thanks, > Peter > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 8ffae3b > > ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java > 14d0ef4 > > ql/src/test/org/apache/hadoop/hive/ql/lockmgr/zookeeper/TestZookeeperLockManager.java > 3f9926e > service/src/java/org/apache/hive/service/server/HiveServer2.java 590b1f3 > > Diff: https://reviews.apache.org/r/52923/diff/ > > > Testing > ------- > > Created 2 unit test cases: > - Removing own locks > - Not removing other server's locks > > Manually tested the Lock manager instantiation method on HiveServer2 > > > Thanks, > > Peter Vary > >
