jojochuang commented on code in PR #9273:
URL: https://github.com/apache/ozone/pull/9273#discussion_r2512301444
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java:
##########
@@ -282,9 +278,7 @@ private void submitSnapshotMoveDeletedKeys(SnapshotInfo
snapInfo,
.setSnapshotMoveTableKeysRequest(moveDeletedKeys)
.setClientId(clientId.toString())
.build();
- try (BootstrapStateHandler.Lock lock = getBootstrapStateLock().lock()) {
Review Comment:
lock removed here.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java:
##########
@@ -244,16 +242,14 @@ private void submitSnapshotPurgeRequest(List<String>
purgeSnapshotKeys) throws I
.setClientId(clientId.toString())
.build();
- try (BootstrapStateHandler.Lock lock = getBootstrapStateLock().lock())
{
Review Comment:
lock removed here.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java:
##########
@@ -418,19 +417,18 @@ private static PurgeKeysRequest.Builder
getPurgeKeysRequest(String snapTableKey,
return requestBuilder;
}
- private boolean submitPurgeRequest(String snapTableKey, boolean purgeSuccess,
- PurgeKeysRequest.Builder requestBuilder) {
+ private boolean submitPurgeRequest(boolean purgeSuccess,
PurgeKeysRequest.Builder requestBuilder) {
OzoneManagerProtocolProtos.OMRequest omRequest =
OzoneManagerProtocolProtos.OMRequest.newBuilder().setCmdType(OzoneManagerProtocolProtos.Type.PurgeKeys)
.setPurgeKeysRequest(requestBuilder.build()).setClientId(getClientId().toString()).build();
- try (Lock lock = snapTableKey != null ? getBootstrapStateLock().lock() :
null) {
+ try {
Review Comment:
lock removed here.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -489,7 +486,7 @@ private OzoneManagerProtocolProtos.OMResponse
submitPurgePaths(List<PurgePathReq
.build();
// Submit Purge paths request to OM. Acquire bootstrap lock when
processing deletes for snapshots.
- try (BootstrapStateHandler.Lock lock = snapTableKey != null ?
getBootstrapStateLock().lock() : null) {
+ try {
Review Comment:
lock removed here.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SstFilteringService.java:
##########
@@ -187,9 +188,9 @@ public BackgroundTaskResult call() throws Exception {
getColumnFamilyToKeyPrefixMap(ozoneManager.getMetadataManager(),
snapshotInfo.getVolumeName(),
snapshotInfo.getBucketName());
-
- try (
- UncheckedAutoCloseableSupplier<OmSnapshot>
snapshotMetadataReader =
+ // Acquire bootstrap lock before opening any snapshot.
Review Comment:
moved the bootstrap lock earlier
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lock/BootstrapStateHandler.java:
##########
@@ -17,28 +17,31 @@
package org.apache.hadoop.ozone.lock;
-import java.util.concurrent.Semaphore;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import org.apache.ratis.util.UncheckedAutoCloseable;
/** Bootstrap state handler interface. */
public interface BootstrapStateHandler {
Lock getBootstrapStateLock();
- /** Bootstrap state handler lock implementation. */
- class Lock implements AutoCloseable {
- private final Semaphore semaphore = new Semaphore(1);
+ /** Bootstrap state handler lock implementation. Should be always acquired
before opening any snapshot to avoid
+ * deadlocks*/
+ class Lock {
+ private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
- public Lock lock() throws InterruptedException {
- semaphore.acquire();
- return this;
+ private UncheckedAutoCloseable lock(boolean readLock) {
+ java.util.concurrent.locks.Lock lock = readLock ?
readWriteLock.readLock() : readWriteLock.writeLock();
+ lock.lock();
+ return lock::unlock;
Review Comment:
This line returns a method reference to unlock the acquired lock.
Specifically, it creates an UncheckedAutoCloseable instance that, when closed,
calls unlock() on the same lock, releasing it. This enables usage of the lock
in a try-with-resources style, ensuring the lock will be released when no
longer needed.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
