You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "swamirishi (via GitHub)" <gi...@apache.org> on 2025/11/22 15:16:03 UTC
[PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager and simplify OmSnapshotLocalDataManager lock [ozone]
swamirishi opened a new pull request, #9353:
URL: https://github.com/apache/ozone/pull/9353
## What changes were proposed in this pull request?
Support full resource lock in PoolBasedHierarchicalLockManager and simplify OmSnapshotLocalDataManager lock to just acquire a full resource on SNAPSHOT_LOCAL_DATA resource.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13983
## How was this patch tested?
Added unit tests
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #9353:
URL: https://github.com/apache/ozone/pull/9353#discussion_r2561858615
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestPoolBasedHierarchicalResourceLockManager.java:
##########
@@ -158,7 +160,59 @@ public void testWriteLockExclusivity() throws Exception {
// Second thread tries to acquire write lock
CompletableFuture<Void> future2 = CompletableFuture.runAsync(() -> {
- try (HierarchicalResourceLock lock = lockManager.acquireWriteLock(DAGLeveledResource.SNAPSHOT_DB_LOCK, key)) {
+ try (HierarchicalResourceLock lock = lockManager.acquireWriteLock(SNAPSHOT_DB_LOCK, key)) {
+ secondLockAcquired.set(true);
+ latch2.countDown();
Review Comment:
latch2 isn't used (same below) elsewhere.
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #9353:
URL: https://github.com/apache/ozone/pull/9353#discussion_r2561949366
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestPoolBasedHierarchicalResourceLockManager.java:
##########
Review Comment:
```suggestion
latch2.await();
```
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9353:
URL: https://github.com/apache/ozone/pull/9353#discussion_r2561066004
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/PoolBasedHierarchicalResourceLockManager.java:
##########
@@ -144,6 +160,40 @@ public void close() {
this.lockPool.close();
}
+ /**
+ * The PoolBasedHierachicalResourceLock class implements the HierarchicalResourceLock
+ * and UncheckedAutoCloseable interfaces to manage hierarchical resource locks from
+ * a shared pool. It ensures proper lock acquisition and release during its lifecycle.
+ */
+ private final class PoolBasedHierachicalResourceLock implements HierarchicalResourceLock,
Review Comment:
done
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #9353:
URL: https://github.com/apache/ozone/pull/9353#discussion_r2558037546
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/PoolBasedHierarchicalResourceLockManager.java:
##########
@@ -144,6 +160,40 @@ public void close() {
this.lockPool.close();
}
+ /**
+ * The PoolBasedHierachicalResourceLock class implements the HierarchicalResourceLock
+ * and UncheckedAutoCloseable interfaces to manage hierarchical resource locks from
+ * a shared pool. It ensures proper lock acquisition and release during its lifecycle.
+ */
+ private final class PoolBasedHierachicalResourceLock implements HierarchicalResourceLock,
Review Comment:
typo PoolBasedHierarchicalResourceLock
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi merged PR #9353:
URL: https://github.com/apache/ozone/pull/9353
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9353:
URL: https://github.com/apache/ozone/pull/9353#issuecomment-3578431357
> Do we hav a specification for what happens when there's a key lock and a full lock for the same resource?
Added
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #9353:
URL: https://github.com/apache/ozone/pull/9353#issuecomment-3579417262
I meant to say that without looking at the code, it is unclear that acquiring a write lock on a key of a resource blocks another lock acquisition on the same resource, and vice versa.
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #9353:
URL: https://github.com/apache/ozone/pull/9353#discussion_r2561948714
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestPoolBasedHierarchicalResourceLockManager.java:
##########
@@ -158,7 +160,59 @@ public void testWriteLockExclusivity() throws Exception {
// Second thread tries to acquire write lock
CompletableFuture<Void> future2 = CompletableFuture.runAsync(() -> {
- try (HierarchicalResourceLock lock = lockManager.acquireWriteLock(DAGLeveledResource.SNAPSHOT_DB_LOCK, key)) {
+ try (HierarchicalResourceLock lock = lockManager.acquireWriteLock(SNAPSHOT_DB_LOCK, key)) {
+ secondLockAcquired.set(true);
+ latch2.countDown();
Review Comment:
yeah added a wait
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9353:
URL: https://github.com/apache/ozone/pull/9353#issuecomment-3580398631
> I meant to say that without looking at the code, it is unclear that acquiring a write lock on a key of a resource blocks another lock acquisition on the same resource, and vice versa.
>
> Other than that I think it's ready to go.
I am bringing in the same api which is present in the OzoneManagerLock interface
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9353:
URL: https://github.com/apache/ozone/pull/9353#issuecomment-3580744112
thank you @jojochuang for reviewing the patch
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #9353:
URL: https://github.com/apache/ozone/pull/9353#discussion_r2561052263
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/PoolBasedHierarchicalResourceLockManager.java:
##########
@@ -144,6 +160,40 @@ public void close() {
this.lockPool.close();
}
+ /**
+ * The PoolBasedHierachicalResourceLock class implements the HierarchicalResourceLock
+ * and UncheckedAutoCloseable interfaces to manage hierarchical resource locks from
+ * a shared pool. It ensures proper lock acquisition and release during its lifecycle.
+ */
+ private final class PoolBasedHierachicalResourceLock implements HierarchicalResourceLock,
Review Comment:
is this a duplicate by any chance? Seems quite similar to PoolBasedHierarchicalResourceKeyLock below
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestPoolBasedHierarchicalResourceLockManager.java:
##########
@@ -585,7 +648,8 @@ public void testDAGLockOrderAcquisition(DAGLeveledResource dagLeveledResource) t
String otherResourceName1 = otherResource.getName() + "key";
String otherResourceName2 = otherResource.getName() + "key";
String flatResourceName = dagLeveledResource.getName() + "key";
Review Comment:
String dagResourceName = dagLeveledResource.getName() + "key";
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-13983. Support full resource lock in PoolBasedHierarchicalLockManager [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #9353:
URL: https://github.com/apache/ozone/pull/9353#discussion_r2561090431
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/PoolBasedHierarchicalResourceLockManager.java:
##########
@@ -144,6 +160,40 @@ public void close() {
this.lockPool.close();
}
+ /**
+ * The PoolBasedHierachicalResourceLock class implements the HierarchicalResourceLock
+ * and UncheckedAutoCloseable interfaces to manage hierarchical resource locks from
+ * a shared pool. It ensures proper lock acquisition and release during its lifecycle.
+ */
+ private final class PoolBasedHierachicalResourceLock implements HierarchicalResourceLock,
Review Comment:
oh i see. One of which is the old implementation where it is locked by key. This one's locked on the whole resource.
--
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-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org