virajjasani commented on code in PR #5603:
URL: https://github.com/apache/hbase/pull/5603#discussion_r1450806311
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/TakeSnapshotHandler.java:
##########
@@ -147,12 +159,16 @@ private TableDescriptor loadTableDescriptor() throws
IOException {
public TakeSnapshotHandler prepare() throws Exception {
super.prepare();
// after this, you should ensure to release this lock in case of exceptions
- this.tableLock.acquire();
- try {
- this.htd = loadTableDescriptor(); // check that .tableinfo is present
- } catch (Exception e) {
- this.tableLock.release();
- throw e;
+ if (this.tableLock.tryAcquire(this.lockAcquireTimeoutMs)) {
Review Comment:
We can still us rpcTimeout but i still think it would be good to have this
config as some downstreamers would like to timeout faster if snapshot could not
acquire lock in 10-15 sec, because it anyways indicate that we have RIT for
that much time (or some other procedure holding lock at table level).
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockManager.java:
##########
@@ -108,19 +108,10 @@ public MasterLock(final RegionInfo[] regionInfos, final
String description) {
this.description = description;
}
- /**
- * Acquire the lock, waiting indefinitely until the lock is released or
the thread is
- * interrupted.
- * @throws InterruptedException If current thread is interrupted while
waiting for the lock
- */
- public boolean acquire() throws InterruptedException {
- return tryAcquire(0);
- }
-
/**
* Acquire the lock within a wait time.
- * @param timeoutMs The maximum time (in milliseconds) to wait for the
lock, 0 to wait
Review Comment:
It seems reasonable to have this change, while it is only being used by
snapshot as of today. If it is 0, client indicates that it has 0ms as timeout.
Does that not sound right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
