joshelser commented on a change in pull request #32:
URL: https://github.com/apache/hbase-filesystem/pull/32#discussion_r817299486
##########
File path:
hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/ZKTreeLockManager.java
##########
@@ -196,7 +197,7 @@ protected void readUnlock(Path p) throws IOException {
We need to protect this block against potential concurrent calls to close()
*/
@Override
- protected synchronized boolean writeLockAbove(Path p) throws IOException {
+ protected boolean writeLockAbove(Path p) throws IOException {
Review comment:
HBASE-24961 #14 introduced the synchronized here. I think Wellington was
fixing an issue where SecureBulkLoadManager was causing problems with the
`FileSystem.CACHE`. That one was caused by HBASE-23679 which I had tried to
fix. Need to make sure we have testing for bulk loading to make sure we don't
regress.
##########
File path:
hbase-oss/src/main/java/org/apache/hadoop/hbase/oss/sync/ZKTreeLockManager.java
##########
@@ -380,24 +382,23 @@ String describeLock(InterProcessReadWriteLock lock) {
return sb.toString();
}
- public synchronized Map<Path,InterProcessReadWriteLock>
getUnmodifiableCache() {
+ public Map<Path,InterProcessReadWriteLock> getUnmodifiableCache() {
return Collections.unmodifiableMap(lockCache);
}
- private synchronized InterProcessReadWriteLock get(Path path) throws
IOException {
- if (!lockCache.containsKey(path)) {
+ private InterProcessReadWriteLock get(Path path) {
+ return lockCache.computeIfAbsent(path, s -> {
Review comment:
This makes sense to me.
--
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: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
