hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Vary <pv...@cloudera.com>
Subject Re: Review Request 52923: HIVE-14979 Removing stale Zookeeper locks at HiveServer2 initialization
Date Tue, 18 Oct 2016 09:23:44 GMT


> On Oct. 17, 2016, 12:47 p.m., Barna Zsombor Klara wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java,
line 648
> > <https://reviews.apache.org/r/52923/diff/1/?file=1539174#file1539174line648>
> >
> >     What is the reason for this null check?

In the original version when we read a lock from zookeeper we set the clinetIp of the lock
to the current server's clientIp.
If I leave it as it is, then we could not differentiate between the lock created by this server,
and locks created by other HS2 servers connected to this zookeeper instance.

I am not sure why we set clientIp at all, since it could come from the zookeeper server and
we use it only for display purposes (at least where I found reference for it). That is why
I raised the question in the review, and in the jira as well.


> On Oct. 17, 2016, 12:47 p.m., Barna Zsombor Klara wrote:
> > service/src/java/org/apache/hive/service/server/HiveServer2.java, line 671
> > <https://reviews.apache.org/r/52923/diff/1/?file=1539176#file1539176line671>
> >
> >     Any reason we are biased towards the ZooKeeperHiveLockManager? I would maybe
call the method releaseStaleLocks, put it into the HiveLockManager interface and then leave
it to the implementation to see if those are identified by IP and released per HS2 instance
or dealt with otherwise.

The HiveLockManager could be set by HiveConf. So I treat it as a public API, and I do not
want to change it between minor releases. It seems to me there should be a better solution
for these kind of problems - how to handle API changes, but this is another topic altogether
:)

The current implementations of HiveLockManager:
- ZookeeperHiveLockManager - we added the feature for that
- EmbeddedLockManager - using memory to store locks - not applicable
- DBLockManager - lock methods are throwing UnsupportedOperationException - not applicable


- Peter


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52923/#review152862
-----------------------------------------------------------


On Oct. 18, 2016, 9:23 a.m., Peter Vary wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52923/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2016, 9:23 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message