joshelser commented on a change in pull request #14:
URL: https://github.com/apache/hbase-filesystem/pull/14#discussion_r478729305
##########
File path:
hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/ZKTreeLockManager.java
##########
@@ -180,9 +196,12 @@ protected boolean writeLockAbove(Path p) throws
IOException {
LOG.debug("Checking for write lock above {}", p);
while (!p.isRoot()) {
p = p.getParent();
- if (isLocked(get(p).writeLock())) {
- LOG.debug("Parent write lock currently held: {}", p);
- return true;
+ //We need to protect this block against potential concurrent calls to
close()
+ synchronized (this) {
Review comment:
I look at this and wonder if it would just be better to mark
`writeLockAbove` as `synchronized`, just to avoid all of the lock acquisition
and dropping as we go up the directory stack...
```
/hbase/data/default/table/region/
/hbase/data/default/table/
/hbase/data/default/
/hbase/data/
/hbase/
```
I'm not sure if that's going to be a noticeable perf impact. I think if your
testing didn't show anything obvious, this is fine for now. Just wanted to
write it out.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
