You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by he...@apache.org on 2024/08/15 23:26:09 UTC
(ozone) branch master updated: HDDS-11137. Removed locks from SnapshotPurge and SnapshotSetProperty APIs. (#7018)
This is an automated email from the ASF dual-hosted git repository.
hemant pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new ad7b8db90b HDDS-11137. Removed locks from SnapshotPurge and SnapshotSetProperty APIs. (#7018)
ad7b8db90b is described below
commit ad7b8db90bda9fa0f83f4c096e958bf310315565
Author: Hemant Kumar <he...@apache.org>
AuthorDate: Thu Aug 15 16:26:03 2024 -0700
HDDS-11137. Removed locks from SnapshotPurge and SnapshotSetProperty APIs. (#7018)
---
.../request/snapshot/OMSnapshotPurgeRequest.java | 122 +++++----------------
.../snapshot/OMSnapshotSetPropertyRequest.java | 32 +-----
.../response/snapshot/OMSnapshotPurgeResponse.java | 2 +-
3 files changed, 33 insertions(+), 123 deletions(-)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java
index 29c7628e3c..2a9cfa6baf 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java
@@ -19,10 +19,7 @@
package org.apache.hadoop.ozone.om.request.snapshot;
-import org.apache.commons.lang3.tuple.Triple;
-import org.apache.hadoop.ozone.om.OMMetadataManager;
import org.apache.hadoop.ozone.om.OMMetrics;
-import org.apache.hadoop.ozone.om.exceptions.OMException;
import org.apache.ratis.server.protocol.TermIndex;
import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
@@ -44,15 +41,11 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
-import java.util.Set;
import java.util.UUID;
-import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.SNAPSHOT_LOCK;
-
/**
* Handles OMSnapshotPurge Request.
* This is an OM internal request. Does not need @RequireSnapshotFeatureState.
@@ -92,62 +85,34 @@ public class OMSnapshotPurgeRequest extends OMClientRequest {
new HashMap<>();
// Each snapshot purge operation does three things:
- // 1. Update the snapshot chain,
- // 2. Update the deep clean flag for the next active snapshot (So that it can be
+ // 1. Update the deep clean flag for the next active snapshot (So that it can be
// deep cleaned by the KeyDeletingService in the next run),
+ // 2. Update the snapshot chain,
// 3. Finally, purge the snapshot.
- // All of these steps have to be performed only when it acquires all the necessary
- // locks (lock on the snapshot to be purged, lock on the next active snapshot, and
- // lock on the next path and global previous snapshots). Ideally, there is no need
- // for locks for snapshot purge and can rely on OMStateMachine because OMStateMachine
- // is going to process each request sequentially.
- //
- // But there is a problem with that. After filtering unnecessary SST files for a snapshot,
- // SstFilteringService updates that snapshot's SstFilter flag. SstFilteringService cannot
- // use SetSnapshotProperty API because it runs on each OM independently and One OM does
- // not know if the snapshot has been filtered on the other OM in HA environment.
- //
- // If locks are not taken snapshot purge and SstFilteringService will cause a race condition
- // and override one's update with another.
+ // There is no need to take lock for snapshot purge as of now. We can simply rely on OMStateMachine
+ // because it executes transaction sequentially.
for (String snapTableKey : snapshotDbKeys) {
- // To acquire all the locks, a set is maintained which is keyed by snapshotTableKey.
- // snapshotTableKey is nothing but /volumeName/bucketName/snapshotName.
- // Once all the locks are acquired, it performs the three steps mentioned above and
- // release all the locks after that.
- Set<Triple<String, String, String>> lockSet = new HashSet<>(4, 1);
- try {
- if (omMetadataManager.getSnapshotInfoTable().get(snapTableKey) == null) {
- // Snapshot may have been purged in the previous iteration of SnapshotDeletingService.
- LOG.warn("The snapshot {} is not longer in snapshot table, It maybe removed in the previous " +
- "Snapshot purge request.", snapTableKey);
- continue;
- }
-
- acquireLock(lockSet, snapTableKey, omMetadataManager);
- SnapshotInfo fromSnapshot = omMetadataManager.getSnapshotInfoTable().get(snapTableKey);
-
- SnapshotInfo nextSnapshot =
- SnapshotUtils.getNextActiveSnapshot(fromSnapshot, snapshotChainManager, omSnapshotManager);
-
- if (nextSnapshot != null) {
- acquireLock(lockSet, nextSnapshot.getTableKey(), omMetadataManager);
- }
-
- // Update the chain first so that it has all the necessary locks before updating deep clean.
- updateSnapshotChainAndCache(lockSet, omMetadataManager, fromSnapshot, trxnLogIndex,
- updatedPathPreviousAndGlobalSnapshots);
- updateSnapshotInfoAndCache(nextSnapshot, omMetadataManager, trxnLogIndex, updatedSnapInfos);
- // Remove and close snapshot's RocksDB instance from SnapshotCache.
- omSnapshotManager.invalidateCacheEntry(fromSnapshot.getSnapshotId());
- // Update SnapshotInfoTable cache.
- omMetadataManager.getSnapshotInfoTable()
- .addCacheEntry(new CacheKey<>(fromSnapshot.getTableKey()), CacheValue.get(trxnLogIndex));
- } finally {
- for (Triple<String, String, String> lockKey: lockSet) {
- omMetadataManager.getLock()
- .releaseWriteLock(SNAPSHOT_LOCK, lockKey.getLeft(), lockKey.getMiddle(), lockKey.getRight());
- }
+ SnapshotInfo fromSnapshot = omMetadataManager.getSnapshotInfoTable().get(snapTableKey);
+ if (fromSnapshot == null) {
+ // Snapshot may have been purged in the previous iteration of SnapshotDeletingService.
+ LOG.warn("The snapshot {} is not longer in snapshot table, It maybe removed in the previous " +
+ "Snapshot purge request.", snapTableKey);
+ continue;
}
+
+ SnapshotInfo nextSnapshot =
+ SnapshotUtils.getNextActiveSnapshot(fromSnapshot, snapshotChainManager, omSnapshotManager);
+
+ // Step 1: Update the deep clean flag for the next active snapshot
+ updateSnapshotInfoAndCache(nextSnapshot, omMetadataManager, trxnLogIndex, updatedSnapInfos);
+ // Step 2: Update the snapshot chain.
+ updateSnapshotChainAndCache(omMetadataManager, fromSnapshot, trxnLogIndex,
+ updatedPathPreviousAndGlobalSnapshots);
+ // Remove and close snapshot's RocksDB instance from SnapshotCache.
+ omSnapshotManager.invalidateCacheEntry(fromSnapshot.getSnapshotId());
+ // Step 3: Purge the snapshot from SnapshotInfoTable cache.
+ omMetadataManager.getSnapshotInfoTable()
+ .addCacheEntry(new CacheKey<>(fromSnapshot.getTableKey()), CacheValue.get(trxnLogIndex));
}
omClientResponse = new OMSnapshotPurgeResponse(omResponse.build(),
@@ -168,41 +133,19 @@ public class OMSnapshotPurgeRequest extends OMClientRequest {
return omClientResponse;
}
- private void acquireLock(Set<Triple<String, String, String>> lockSet, String snapshotTableKey,
- OMMetadataManager omMetadataManager) throws IOException {
- SnapshotInfo snapshotInfo = omMetadataManager.getSnapshotInfoTable().get(snapshotTableKey);
-
- // It should not be the case that lock is required for non-existing snapshot.
- if (snapshotInfo == null) {
- LOG.error("Snapshot: '{}' doesn't not exist in snapshot table.", snapshotTableKey);
- throw new OMException("Snapshot: '{" + snapshotTableKey + "}' doesn't not exist in snapshot table.",
- OMException.ResultCodes.FILE_NOT_FOUND);
- }
- Triple<String, String, String> lockKey = Triple.of(snapshotInfo.getVolumeName(), snapshotInfo.getBucketName(),
- snapshotInfo.getName());
- if (!lockSet.contains(lockKey)) {
- mergeOmLockDetails(omMetadataManager.getLock()
- .acquireWriteLock(SNAPSHOT_LOCK, lockKey.getLeft(), lockKey.getMiddle(), lockKey.getRight()));
- lockSet.add(lockKey);
- }
- }
-
private void updateSnapshotInfoAndCache(SnapshotInfo snapInfo,
OmMetadataManagerImpl omMetadataManager, long trxnLogIndex,
- Map<String, SnapshotInfo> updatedSnapInfos) throws IOException {
+ Map<String, SnapshotInfo> updatedSnapInfos) throws IOException {
if (snapInfo != null) {
- // Fetch the latest value again after acquiring lock.
- SnapshotInfo updatedSnapshotInfo = omMetadataManager.getSnapshotInfoTable().get(snapInfo.getTableKey());
-
// Setting next snapshot deep clean to false, Since the
// current snapshot is deleted. We can potentially
// reclaim more keys in the next snapshot.
- updatedSnapshotInfo.setDeepClean(false);
+ snapInfo.setDeepClean(false);
// Update table cache first
- omMetadataManager.getSnapshotInfoTable().addCacheEntry(new CacheKey<>(updatedSnapshotInfo.getTableKey()),
- CacheValue.get(trxnLogIndex, updatedSnapshotInfo));
- updatedSnapInfos.put(updatedSnapshotInfo.getTableKey(), updatedSnapshotInfo);
+ omMetadataManager.getSnapshotInfoTable().addCacheEntry(new CacheKey<>(snapInfo.getTableKey()),
+ CacheValue.get(trxnLogIndex, snapInfo));
+ updatedSnapInfos.put(snapInfo.getTableKey(), snapInfo);
}
}
@@ -213,7 +156,6 @@ public class OMSnapshotPurgeRequest extends OMClientRequest {
* update in DB.
*/
private void updateSnapshotChainAndCache(
- Set<Triple<String, String, String>> lockSet,
OmMetadataManagerImpl metadataManager,
SnapshotInfo snapInfo,
long trxnLogIndex,
@@ -247,18 +189,12 @@ public class OMSnapshotPurgeRequest extends OMClientRequest {
snapInfo.getSnapshotPath(), snapInfo.getSnapshotId());
nextPathSnapshotKey = snapshotChainManager
.getTableKey(nextPathSnapshotId);
-
- // Acquire lock from the snapshot
- acquireLock(lockSet, nextPathSnapshotKey, metadataManager);
}
String nextGlobalSnapshotKey = null;
if (hasNextGlobalSnapshot) {
UUID nextGlobalSnapshotId = snapshotChainManager.nextGlobalSnapshot(snapInfo.getSnapshotId());
nextGlobalSnapshotKey = snapshotChainManager.getTableKey(nextGlobalSnapshotId);
-
- // Acquire lock from the snapshot
- acquireLock(lockSet, nextGlobalSnapshotKey, metadataManager);
}
SnapshotInfo nextPathSnapInfo =
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotSetPropertyRequest.java
index c4ca3dc99e..53047fd802 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotSetPropertyRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotSetPropertyRequest.java
@@ -38,7 +38,6 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_NOT_FOUND;
-import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.SNAPSHOT_LOCK;
/**
* Updates the exclusive size of the snapshot.
@@ -55,7 +54,7 @@ public class OMSnapshotSetPropertyRequest extends OMClientRequest {
public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIndex termIndex) {
OMMetrics omMetrics = ozoneManager.getMetrics();
- OMClientResponse omClientResponse = null;
+ OMClientResponse omClientResponse;
OMMetadataManager metadataManager = ozoneManager.getMetadataManager();
OzoneManagerProtocolProtos.OMResponse.Builder omResponse =
@@ -63,33 +62,16 @@ public class OMSnapshotSetPropertyRequest extends OMClientRequest {
OzoneManagerProtocolProtos.SetSnapshotPropertyRequest
setSnapshotPropertyRequest = getOmRequest()
.getSetSnapshotPropertyRequest();
- SnapshotInfo updatedSnapInfo = null;
String snapshotKey = setSnapshotPropertyRequest.getSnapshotKey();
- boolean acquiredSnapshotLock = false;
- String volumeName = null;
- String bucketName = null;
- String snapshotName = null;
try {
- SnapshotInfo snapshotInfo = metadataManager.getSnapshotInfoTable().get(snapshotKey);
- if (snapshotInfo == null) {
+ SnapshotInfo updatedSnapInfo = metadataManager.getSnapshotInfoTable().get(snapshotKey);
+ if (updatedSnapInfo == null) {
LOG.error("Snapshot: '{}' doesn't not exist in snapshot table.", snapshotKey);
throw new OMException("Snapshot: '{" + snapshotKey + "}' doesn't not exist in snapshot table.", FILE_NOT_FOUND);
}
- volumeName = snapshotInfo.getVolumeName();
- bucketName = snapshotInfo.getBucketName();
- snapshotName = snapshotInfo.getName();
-
- mergeOmLockDetails(metadataManager.getLock()
- .acquireWriteLock(SNAPSHOT_LOCK, volumeName, bucketName, snapshotName));
-
- acquiredSnapshotLock = getOmLockDetails().isLockAcquired();
-
- updatedSnapInfo = metadataManager.getSnapshotInfoTable()
- .get(snapshotKey);
-
if (setSnapshotPropertyRequest.hasDeepCleanedDeletedDir()) {
updatedSnapInfo.setDeepCleanedDeletedDir(setSnapshotPropertyRequest
@@ -126,14 +108,6 @@ public class OMSnapshotSetPropertyRequest extends OMClientRequest {
createErrorOMResponse(omResponse, ex));
omMetrics.incNumSnapshotSetPropertyFails();
LOG.error("Failed to execute snapshotSetPropertyRequest: {{}}.", setSnapshotPropertyRequest, ex);
- } finally {
- if (acquiredSnapshotLock) {
- mergeOmLockDetails(metadataManager.getLock()
- .releaseWriteLock(SNAPSHOT_LOCK, volumeName, bucketName, snapshotName));
- }
- if (omClientResponse != null) {
- omClientResponse.setOmLockDetails(getOmLockDetails());
- }
}
return omClientResponse;
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java
index 45b0c5e059..ea9e68cc9a 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java
@@ -81,9 +81,9 @@ public class OMSnapshotPurgeResponse extends OMClientResponse {
OmMetadataManagerImpl metadataManager = (OmMetadataManagerImpl)
omMetadataManager;
+ updateSnapInfo(metadataManager, batchOperation, updatedSnapInfos);
updateSnapInfo(metadataManager, batchOperation,
updatedPreviousAndGlobalSnapInfos);
- updateSnapInfo(metadataManager, batchOperation, updatedSnapInfos);
for (String dbKey: snapshotDbKeys) {
// Skip the cache here because snapshot is purged from cache in OMSnapshotPurgeRequest.
SnapshotInfo snapshotInfo = omMetadataManager
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org