diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledUtil.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledUtil.java
new file mode 100644
index 000000000000..fa3030484e53
--- /dev/null
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledUtil.java
@@ -0,0 +1,44 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.snapshot;
+
+/**
+ * TestSnapshotFeatureEnabledAspect util class.
+ */
+public class SnapshotFeatureEnabledUtil {
+
+ /**
+ * This is an example of an "API" that requires snapshot feature enabled.
+ */
+ @SnapshotFeatureEnabled(true)
+ public String snapshotMethod() {
+ return "yay";
+ }
+
+ /**
+ * Method needed for the Aspect to get current feature state. Emulates
+ * {@link org.apache.hadoop.ozone.om.OzoneManager#isFilesystemSnapshotEnabled}
+ * Note: Method has to be `public` for reflection invocation to work.
+ * @return false
+ */
+ public boolean isFilesystemSnapshotEnabled() {
+ return false;
+ }
+
+}
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java
new file mode 100644
index 000000000000..e8b6f2c19d9c
--- /dev/null
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.snapshot;
+
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.upgrade.MockOmRequest;
+import org.apache.hadoop.ozone.om.upgrade.OMLayoutFeatureAspect;
+import org.apache.hadoop.ozone.om.upgrade.OMLayoutVersionManager;
+import org.apache.ozone.test.LambdaTestUtils;
+import org.aspectj.lang.JoinPoint;
+import org.aspectj.lang.reflect.MethodSignature;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.IOException;
+
+import static org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature.INITIAL_VERSION;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Class to test annotation based interceptor that checks whether
+ * Ozone snapshot feature is enabled.
+ */
+public class TestSnapshotFeatureEnabledAspect {
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+// private OzoneConfiguration configuration = new OzoneConfiguration();
+
+ @Before
+ public void setUp() throws IOException {
+// configuration.set("ozone.metadata.dirs",
+// temporaryFolder.newFolder().getAbsolutePath());
+ }
+
+ /**
+ * Check Aspect implementation with SnapshotFeatureEnabledUtil.
+ */
+ @Test
+ public void testSnapshotFeatureEnabledAnnotation() throws Exception {
+ SnapshotFeatureEnabledUtil testObj = new SnapshotFeatureEnabledUtil();
+ SnapshotFeatureEnabledAspect aspect = new SnapshotFeatureEnabledAspect();
+
+ JoinPoint joinPoint = mock(JoinPoint.class);
+ when(joinPoint.getTarget()).thenReturn(testObj);
+
+ MethodSignature methodSignature = mock(MethodSignature.class);
+ when(methodSignature.getMethod()).thenReturn(
+ SnapshotFeatureEnabledUtil.class.getMethod("snapshotMethod"));
+ when(methodSignature.toShortString()).thenReturn("snapshotMethod");
+ when(joinPoint.getSignature()).thenReturn(methodSignature);
+
+ LambdaTestUtils.intercept(OMException.class,
+ "Operation snapshotMethod cannot be invoked because " +
+ "Ozone snapshot feature is disabled",
+ () -> aspect.checkLayoutFeature(joinPoint));
+ }
+
+ /**
+ * Check Aspect implementation with mocked OzoneManager.
+ */
+// @Test
+// public void testPreExecuteFeatureCheck() throws Exception {
+//
+// OzoneManager om = mock(OzoneManager.class);
+// when(om.isFilesystemSnapshotEnabled()).thenReturn(false);
+//
+// MockOmRequest mockOmRequest = new MockOmRequest();
+// SnapshotFeatureEnabledAspect aspect = new SnapshotFeatureEnabledAspect();
+//
+// JoinPoint joinPoint = mock(JoinPoint.class);
+// when(joinPoint.getArgs()).thenReturn(new Object[]{om});
+// when(joinPoint.getTarget()).thenReturn(mockOmRequest);
+//
+// LambdaTestUtils.intercept(OMException.class,
+// "cannot be invoked",
+// () -> aspect.beforeRequestApplyTxn(joinPoint));
+// }
+}
From 9c6ef61bea0d8fcc1cd4e39d7e63b738b3e26673 Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 17:00:34 -0700
Subject: [PATCH 05/17] Clean up Aspect impl and test.
---
.../ozone/om/request/OMClientRequest.java | 12 ------
.../snapshot/OMSnapshotCreateRequest.java | 2 -
.../om/snapshot/SnapshotFeatureEnabled.java | 2 -
.../TestSnapshotFeatureEnabledAspect.java | 38 -------------------
4 files changed, 54 deletions(-)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
index 5b5557de34ed..963eddc06ccf 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java
@@ -54,7 +54,6 @@
import java.util.LinkedHashMap;
import java.util.Map;
-import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FEATURE_NOT_ENABLED;
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_KEY_NAME;
/**
@@ -570,15 +569,4 @@ public static String isValidKeyPath(String path) throws OMException {
throw new OMException("Invalid KeyPath " + path, INVALID_KEY_NAME);
}
}
-
- /**
- * Helper method that throws OMException if filesystem snapshot is disabled.
- */
- protected static void checkFsSnapshotEnabled(OzoneManager ozoneManager)
- throws OMException {
- if (!ozoneManager.isFilesystemSnapshotEnabled()) {
- throw new OMException("Snapshot is not enabled", FEATURE_NOT_ENABLED);
- }
- }
-
}
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
index c65f08c01ac5..65c89aecd658 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
@@ -90,8 +90,6 @@ public OMSnapshotCreateRequest(OMRequest omRequest) {
@Override
@SnapshotFeatureEnabled(true)
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
- // Reject if snapshot feature is disabled
-// checkFsSnapshotEnabled(ozoneManager);
final OMRequest omRequest = super.preExecute(ozoneManager);
// Verify name
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java
index b5a05252893c..a8c36047f7e8 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java
@@ -18,8 +18,6 @@
package org.apache.hadoop.ozone.om.snapshot;
-import org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature;
-
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java
index e8b6f2c19d9c..bd3c7a856a92 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java
@@ -18,23 +18,14 @@
package org.apache.hadoop.ozone.om.snapshot;
-import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.exceptions.OMException;
-import org.apache.hadoop.ozone.om.upgrade.MockOmRequest;
-import org.apache.hadoop.ozone.om.upgrade.OMLayoutFeatureAspect;
-import org.apache.hadoop.ozone.om.upgrade.OMLayoutVersionManager;
import org.apache.ozone.test.LambdaTestUtils;
import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.reflect.MethodSignature;
-import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
-import java.io.IOException;
-
-import static org.apache.hadoop.ozone.om.upgrade.OMLayoutFeature.INITIAL_VERSION;
-import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -47,14 +38,6 @@ public class TestSnapshotFeatureEnabledAspect {
@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();
-// private OzoneConfiguration configuration = new OzoneConfiguration();
-
- @Before
- public void setUp() throws IOException {
-// configuration.set("ozone.metadata.dirs",
-// temporaryFolder.newFolder().getAbsolutePath());
- }
-
/**
* Check Aspect implementation with SnapshotFeatureEnabledUtil.
*/
@@ -77,25 +60,4 @@ public void testSnapshotFeatureEnabledAnnotation() throws Exception {
"Ozone snapshot feature is disabled",
() -> aspect.checkLayoutFeature(joinPoint));
}
-
- /**
- * Check Aspect implementation with mocked OzoneManager.
- */
-// @Test
-// public void testPreExecuteFeatureCheck() throws Exception {
-//
-// OzoneManager om = mock(OzoneManager.class);
-// when(om.isFilesystemSnapshotEnabled()).thenReturn(false);
-//
-// MockOmRequest mockOmRequest = new MockOmRequest();
-// SnapshotFeatureEnabledAspect aspect = new SnapshotFeatureEnabledAspect();
-//
-// JoinPoint joinPoint = mock(JoinPoint.class);
-// when(joinPoint.getArgs()).thenReturn(new Object[]{om});
-// when(joinPoint.getTarget()).thenReturn(mockOmRequest);
-//
-// LambdaTestUtils.intercept(OMException.class,
-// "cannot be invoked",
-// () -> aspect.beforeRequestApplyTxn(joinPoint));
-// }
}
From 49ca500cf4c834a3acd9496cc5936ab2f949bdf3 Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 17:13:52 -0700
Subject: [PATCH 06/17] More test changes to enable snapshot feature
explicitly.
---
.../hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java | 2 +-
.../ozone/om/snapshot/SnapshotFeatureEnabledAspect.java | 2 +-
.../om/request/key/TestOMKeyPurgeRequestAndResponse.java | 6 ++++++
.../om/request/snapshot/TestOMSnapshotCreateRequest.java | 1 +
.../om/request/snapshot/TestOMSnapshotDeleteRequest.java | 1 +
.../snapshot/TestOMSnapshotPurgeRequestAndResponse.java | 1 +
.../ozone/om/service/TestSnapshotDiffCleanupService.java | 5 +++++
7 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java
index a8c36047f7e8..a4d65c5b05b9 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java
@@ -24,7 +24,7 @@
import java.lang.annotation.Target;
/**
- * Annotation used to "disallow" an API if current layout version does
+ * Annotation used to check that snapshot feature is enabled or disabled.
* not include the associated layout feature. Helps to keep the method logic
* and upgrade related cross cutting concern separate.
*/
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledAspect.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledAspect.java
index cecd81ae09ae..ad7c4d5fae82 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledAspect.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledAspect.java
@@ -49,7 +49,7 @@ public class SnapshotFeatureEnabledAspect {
private static final Logger LOG = LoggerFactory
.getLogger(SnapshotFeatureEnabledAspect.class);
- @Before("@annotation(DisallowedUntilLayoutVersion) && execution(* *(..))")
+ @Before("@annotation(SnapshotFeatureEnabled) && execution(* *(..))")
public void checkLayoutFeature(JoinPoint joinPoint) throws IOException {
boolean desiredFeatureState = ((MethodSignature) joinPoint.getSignature())
.getMethod().getAnnotation(SnapshotFeatureEnabled.class)
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java
index 19df5dee21b7..58c9d035967b 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java
@@ -30,6 +30,7 @@
import org.apache.hadoop.ozone.om.request.snapshot.TestOMSnapshotCreateRequest;
import org.apache.hadoop.ozone.om.response.snapshot.OMSnapshotCreateResponse;
import org.junit.Assert;
+import org.junit.Before;
import org.junit.Test;
import org.apache.hadoop.ozone.om.response.key.OMKeyPurgeResponse;
@@ -54,6 +55,11 @@ public class TestOMKeyPurgeRequestAndResponse extends TestOMKeyRequest {
private int numKeys = 10;
+ @Before
+ public void setUp() throws Exception {
+ when(ozoneManager.isFilesystemSnapshotEnabled()).thenReturn(true);
+ }
+
/**
* Creates volume, bucket and key entries and adds to OM DB and then
* deletes these keys to move them to deletedKeys table.
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotCreateRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotCreateRequest.java
index a67814156db3..38df5c8006c0 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotCreateRequest.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotCreateRequest.java
@@ -97,6 +97,7 @@ public void setup() throws Exception {
when(ozoneManager.getMetrics()).thenReturn(omMetrics);
when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager);
when(ozoneManager.isRatisEnabled()).thenReturn(true);
+ when(ozoneManager.isFilesystemSnapshotEnabled()).thenReturn(true);
when(ozoneManager.isAdmin(any())).thenReturn(false);
when(ozoneManager.isOwner(any(), any())).thenReturn(false);
when(ozoneManager.getBucketOwner(any(), any(),
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java
index 0012e9494f0a..ae28a9cd3152 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java
@@ -89,6 +89,7 @@ public void setup() throws Exception {
when(ozoneManager.getMetrics()).thenReturn(omMetrics);
when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager);
when(ozoneManager.isRatisEnabled()).thenReturn(true);
+ when(ozoneManager.isFilesystemSnapshotEnabled()).thenReturn(true);
when(ozoneManager.isAdmin(any())).thenReturn(false);
when(ozoneManager.isOwner(any(), any())).thenReturn(false);
when(ozoneManager.getBucketOwner(any(), any(),
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java
index 02814c788e17..648e62ad13fe 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java
@@ -110,6 +110,7 @@ public void setup() throws Exception {
when(ozoneManager.getConfiguration()).thenReturn(ozoneConfiguration);
when(ozoneManager.isAdmin(any(UserGroupInformation.class)))
.thenReturn(true);
+ when(ozoneManager.isFilesystemSnapshotEnabled()).thenReturn(true);
OmMetadataReader omMetadataReader = Mockito.mock(OmMetadataReader.class);
when(ozoneManager.getOmMetadataReader()).thenReturn(omMetadataReader);
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDiffCleanupService.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDiffCleanupService.java
index eb7b77f328eb..60eae2c1c8e8 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDiffCleanupService.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestSnapshotDiffCleanupService.java
@@ -54,6 +54,8 @@
import java.util.concurrent.TimeUnit;
import static org.apache.hadoop.hdds.utils.db.DBStoreBuilder.DEFAULT_COLUMN_FAMILY_NAME;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_DEFAULT;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_DIFF_JOB_REPORT_PERSISTENT_TIME;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_DIFF_JOB_REPORT_PERSISTENT_TIME_DEFAULT;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_DIFF_MAX_JOBS_PURGE_PER_TASK;
@@ -151,6 +153,9 @@ public void init() throws RocksDBException, IOException {
TimeUnit.MILLISECONDS)
).thenReturn(TimeUnit.DAYS.toMillis(7));
+ when(config.getBoolean(OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY,
+ OZONE_FILESYSTEM_SNAPSHOT_ENABLED_DEFAULT)).thenReturn(true);
+
when(ozoneManager.getConfiguration()).thenReturn(config);
jobTableCfd = new ColumnFamilyDescriptor(jobTableNameBytes,
From 85c75e60280d068cfb4ad87a857754228fc211bb Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 17:31:30 -0700
Subject: [PATCH 07/17] omSnapshotManager can't easily be null due to the usage
in OMDirectoriesPurgeRequestWithFSO#validateAndUpdateCache -- can lead to
inconsistency when OM is recovering from Ratis log if skipped.
---
.../org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java | 7 +++++--
.../main/java/org/apache/hadoop/ozone/om/OzoneManager.java | 2 +-
.../om/request/key/OMDirectoriesPurgeRequestWithFSO.java | 4 +++-
.../hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java | 3 +--
4 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
index fcd3b2cde286..ef4c190057ae 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
@@ -1471,8 +1471,11 @@ public List getPendingDeletionKeys(final int keyCount,
// Get volume name and bucket name
String[] keySplit = kv.getKey().split(OM_KEY_PREFIX);
// Get the latest snapshot in snapshot path.
- OmSnapshot latestSnapshot = getLatestSnapshot(keySplit[1],
- keySplit[2], omSnapshotManager);
+ OmSnapshot latestSnapshot = null;
+ if (omSnapshotManager != null) {
+ latestSnapshot = getLatestSnapshot(
+ keySplit[1], keySplit[2], omSnapshotManager);
+ }
String bucketKey = getBucketKey(keySplit[1], keySplit[2]);
OmBucketInfo bucketInfo = getBucketTable().get(bucketKey);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
index 1a5347a0c10f..e019d81d28ed 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
@@ -790,7 +790,7 @@ private void instantiateServices(boolean withNewSnapshot) throws IOException {
omMetadataReader = new OmMetadataReader(keyManager, prefixManager,
this, LOG, AUDIT, metrics);
- if (fsSnapshotEnabled) {
+ if (isFilesystemSnapshotEnabled()) {
omSnapshotManager = new OmSnapshotManager(this);
} else {
omSnapshotManager = null;
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
index 90ea43a53ee9..88ee1ff5a422 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
@@ -73,6 +73,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
SnapshotInfo snapshotInfo =
ozoneManager.getMetadataManager().getSnapshotInfoTable()
.get(fromSnapshot);
+ // TODO: If snapshotInfo is not null but omSnapshotManager is null,
+ // abort OM and prompt the user to enable snapshot feature first.
omFromSnapshot = (OmSnapshot) ozoneManager.getOmSnapshotManager()
.checkForSnapshot(snapshotInfo.getVolumeName(),
snapshotInfo.getBucketName(),
@@ -127,7 +129,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
}
}
} catch (IOException ex) {
- // Case of IOException for fromProtobuf will not hanppen
+ // Case of IOException for fromProtobuf will not happen
// as this is created and send within OM
// only case of upgrade where compatibility is broken can have
throw new IllegalStateException(ex);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java
index a4d65c5b05b9..dd7a03b3ac9d 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java
@@ -25,8 +25,7 @@
/**
* Annotation used to check that snapshot feature is enabled or disabled.
- * not include the associated layout feature. Helps to keep the method logic
- * and upgrade related cross cutting concern separate.
+ * TODO: Rename this to RequireSnapshotFeatureState.
*/
@Target({ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)
From fcd2c8b4a2623b7ce402be81c7f0eb027471bfee Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 17:36:06 -0700
Subject: [PATCH 08/17] Even more test changes to enable snapshot feature
explicitly.
---
.../java/org/apache/hadoop/fs/ozone/TestOzoneFsSnapshot.java | 3 +++
.../java/org/apache/hadoop/ozone/freon/TestOMSnapshotDAG.java | 3 +++
.../test/java/org/apache/hadoop/ozone/om/TestOmSnapshot.java | 2 ++
.../hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java | 4 ++++
.../org/apache/hadoop/ozone/om/TestOmSnapshotManager.java | 3 +++
5 files changed, 15 insertions(+)
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFsSnapshot.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFsSnapshot.java
index b6a4bb1fa1c0..20a728719e5e 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFsSnapshot.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFsSnapshot.java
@@ -29,6 +29,7 @@
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
import org.apache.hadoop.util.ToolRunner;
@@ -76,6 +77,8 @@ public class TestOzoneFsSnapshot {
@BeforeAll
public static void initClass() throws Exception {
OzoneConfiguration conf = new OzoneConfiguration();
+ // Enable filesystem snapshot feature for the test regardless of the default
+ conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
// Start the cluster
cluster = MiniOzoneCluster.newOMHABuilder(conf)
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/freon/TestOMSnapshotDAG.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/freon/TestOMSnapshotDAG.java
index 041383264f87..a4e808be41e6 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/freon/TestOMSnapshotDAG.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/freon/TestOMSnapshotDAG.java
@@ -28,6 +28,7 @@
import org.apache.hadoop.ozone.client.OzoneBucket;
import org.apache.hadoop.ozone.client.OzoneClient;
import org.apache.hadoop.ozone.client.OzoneVolume;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
import org.apache.hadoop.ozone.om.OMMetadataManager;
import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
import org.apache.hadoop.ozone.om.OmSnapshotManager;
@@ -98,6 +99,8 @@ public static void init() throws Exception {
raftClientConfig.setRpcRequestTimeout(Duration.ofSeconds(3));
raftClientConfig.setRpcWatchRequestTimeout(Duration.ofSeconds(3));
conf.setFromObject(raftClientConfig);
+ // Enable filesystem snapshot feature for the test regardless of the default
+ conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
// Set DB CF write buffer to a much lower value so that flush and compaction
// happens much more frequently without having to create a lot of keys.
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshot.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshot.java
index bdfca04e6126..acd9237c2c89 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshot.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshot.java
@@ -163,6 +163,8 @@ private void init() throws Exception {
conf.setBoolean(OMConfigKeys.OZONE_OM_SNAPSHOT_FORCE_FULL_DIFF,
forceFullSnapshotDiff);
conf.setEnum(HDDS_DB_PROFILE, DBProfile.TEST);
+ // Enable filesystem snapshot feature for the test regardless of the default
+ conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
cluster = MiniOzoneCluster.newOMHABuilder(conf)
.setClusterId(clusterId)
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java
index d5faf7a8ccd0..92cf5ec2e109 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java
@@ -34,6 +34,7 @@
import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
import org.apache.hadoop.ozone.client.BucketArgs;
import org.apache.hadoop.ozone.om.KeyManagerImpl;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
import org.apache.hadoop.ozone.om.OMStorage;
import org.apache.hadoop.ozone.om.OmSnapshotManager;
import org.apache.hadoop.ozone.om.OzoneManager;
@@ -91,6 +92,9 @@ private static Stream bucketTypesCombinations() {
@BeforeEach
public void init() throws Exception {
OzoneConfiguration conf = new OzoneConfiguration();
+ // Enable filesystem snapshot feature for the test regardless of the default
+ conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
+
String clusterId = UUID.randomUUID().toString();
String scmId = UUID.randomUUID().toString();
String serviceID = OM_SERVICE_ID + RandomStringUtils.randomNumeric(5);
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
index c3162784b051..77f649504322 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
@@ -70,6 +70,9 @@ public void init() throws Exception {
testDir = GenericTestUtils.getRandomizedTestDir();
configuration.set(HddsConfigKeys.OZONE_METADATA_DIRS,
testDir.toString());
+ // Enable filesystem snapshot feature for the test regardless of the default
+ configuration.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY,
+ true);
// Only allow one entry in cache so each new one causes an eviction
configuration.setInt(
From efebf559e670ef6b624333d870d756ba6cfea909 Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 18:20:38 -0700
Subject: [PATCH 09/17] Add comments for HDDS-8529; add debug logging when
omSnapshotManager is null; findbugs.
---
.../hadoop/ozone/om/OmMetadataManagerImpl.java | 8 +++++++-
.../apache/hadoop/ozone/om/OmSnapshotManager.java | 7 +++++++
.../org/apache/hadoop/ozone/om/OzoneManager.java | 2 +-
.../key/OMDirectoriesPurgeRequestWithFSO.java | 6 ++++--
.../ozone/om/request/key/OMKeyPurgeRequest.java | 4 ++++
.../ozone/om/service/DirectoryDeletingService.java | 13 ++++++++++---
.../key/TestOMKeyPurgeRequestAndResponse.java | 5 -----
.../ozone/om/request/key/TestOMKeyRequest.java | 1 +
.../snapshot/TestSnapshotFeatureEnabledAspect.java | 5 -----
9 files changed, 34 insertions(+), 17 deletions(-)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
index ef4c190057ae..db37db1c6273 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
@@ -35,6 +35,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;
+import com.google.common.base.Preconditions;
import org.apache.hadoop.hdds.client.BlockID;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.utils.db.DBCheckpoint;
@@ -1475,6 +1476,9 @@ public List getPendingDeletionKeys(final int keyCount,
if (omSnapshotManager != null) {
latestSnapshot = getLatestSnapshot(
keySplit[1], keySplit[2], omSnapshotManager);
+ } else {
+ LOG.debug("omSnapshotManager is not initialized. " +
+ "Ozone snapshot feature might have been disabled.");
}
String bucketKey = getBucketKey(keySplit[1], keySplit[2]);
OmBucketInfo bucketInfo = getBucketTable().get(bucketKey);
@@ -1553,9 +1557,11 @@ public List getPendingDeletionKeys(final int keyCount,
* Get the latest OmSnapshot for a snapshot path.
*/
public OmSnapshot getLatestSnapshot(String volumeName, String bucketName,
- OmSnapshotManager snapshotManager)
+ OmSnapshotManager snapshotManager)
throws IOException {
+ Preconditions.checkNotNull(snapshotManager);
+
String latestPathSnapshot =
snapshotChainManager.getLatestPathSnapshot(volumeName
+ OM_KEY_PREFIX + bucketName);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
index 4b4dc50b8245..de19bb273122 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
@@ -158,6 +158,13 @@ public final class OmSnapshotManager implements AutoCloseable {
private final int maxPageSize;
public OmSnapshotManager(OzoneManager ozoneManager) {
+ // TODO: [SNAPSHOT] HDDS-8529: Refactor the constructor in a way that when
+ // ozoneManager.isFilesystemSnapshotEnabled() is false, already committed
+ // Ratis transactions can still apply in validateAndUpdateCache even when
+ // snapshot feature is disabled. Affected requests:
+ // 1. OMKeyPurgeRequest#validateAndUpdateCache
+ // 2. OMDirectoriesPurgeRequestWithFSO#validateAndUpdateCache
+
this.options = new ManagedDBOptions();
this.options.setCreateIfMissing(true);
this.columnFamilyOptions = new ManagedColumnFamilyOptions();
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
index e019d81d28ed..75a48452e3b0 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
@@ -790,11 +790,11 @@ private void instantiateServices(boolean withNewSnapshot) throws IOException {
omMetadataReader = new OmMetadataReader(keyManager, prefixManager,
this, LOG, AUDIT, metrics);
+ // TODO: [SNAPSHOT] Remove this condition when HDDS-8529 is done.
if (isFilesystemSnapshotEnabled()) {
omSnapshotManager = new OmSnapshotManager(this);
} else {
omSnapshotManager = null;
- // TODO: [Snapshot] Check all usages of omSnapshotManager.
}
// Snapshot metrics
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
index 88ee1ff5a422..25756a3bad7f 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
@@ -73,8 +73,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
SnapshotInfo snapshotInfo =
ozoneManager.getMetadataManager().getSnapshotInfoTable()
.get(fromSnapshot);
- // TODO: If snapshotInfo is not null but omSnapshotManager is null,
- // abort OM and prompt the user to enable snapshot feature first.
+ // TODO: [SNAPSHOT] As a workaround before HDDS-8529 is merged,
+ // if snapshotInfo is not null but omSnapshotManager is null,
+ // abort OM and prompt the user to enable snapshot feature first
+ // in order to keep OM consistent.
omFromSnapshot = (OmSnapshot) ozoneManager.getOmSnapshotManager()
.checkForSnapshot(snapshotInfo.getVolumeName(),
snapshotInfo.getBucketName(),
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java
index f429afb95374..c236e1fa45ab 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java
@@ -80,6 +80,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
SnapshotInfo snapshotInfo =
ozoneManager.getMetadataManager().getSnapshotInfoTable()
.get(fromSnapshot);
+ // TODO: [SNAPSHOT] As a workaround before HDDS-8529 is merged,
+ // if snapshotInfo is not null but omSnapshotManager is null,
+ // abort OM and prompt the user to enable snapshot feature first
+ // in order to keep OM consistent.
omFromSnapshot = (OmSnapshot) omSnapshotManager
.checkForSnapshot(snapshotInfo.getVolumeName(),
snapshotInfo.getBucketName(),
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
index ceed9ebe9aa7..1a76abe782a5 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
@@ -174,9 +174,16 @@ private boolean previousSnapshotHasDir(
OmMetadataManagerImpl metadataManager = (OmMetadataManagerImpl)
getOzoneManager().getMetadataManager();
- OmSnapshot latestSnapshot =
- metadataManager.getLatestSnapshot(deletedDirInfo.getVolumeName(),
- deletedDirInfo.getBucketName(), omSnapshotManager);
+ OmSnapshot latestSnapshot = null;
+ if (omSnapshotManager != null) {
+ latestSnapshot = metadataManager.getLatestSnapshot(
+ deletedDirInfo.getVolumeName(),
+ deletedDirInfo.getBucketName(),
+ omSnapshotManager);
+ } else {
+ LOG.debug("omSnapshotManager is not initialized. " +
+ "Ozone snapshot feature might have been disabled.");
+ }
if (latestSnapshot != null) {
Table prevDirTable =
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java
index 58c9d035967b..42c849d774db 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java
@@ -55,11 +55,6 @@ public class TestOMKeyPurgeRequestAndResponse extends TestOMKeyRequest {
private int numKeys = 10;
- @Before
- public void setUp() throws Exception {
- when(ozoneManager.isFilesystemSnapshotEnabled()).thenReturn(true);
- }
-
/**
* Creates volume, bucket and key entries and adds to OM DB and then
* deletes these keys to move them to deletedKeys table.
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java
index 462b6c731d73..0a9eda229128 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java
@@ -137,6 +137,7 @@ public void setup() throws Exception {
when(lvm.getMetadataLayoutVersion()).thenReturn(0);
when(ozoneManager.getVersionManager()).thenReturn(lvm);
when(ozoneManager.isRatisEnabled()).thenReturn(true);
+ when(ozoneManager.isFilesystemSnapshotEnabled()).thenReturn(true);
auditLogger = Mockito.mock(AuditLogger.class);
when(ozoneManager.getAuditLogger()).thenReturn(auditLogger);
when(ozoneManager.isAdmin(any(UserGroupInformation.class)))
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java
index bd3c7a856a92..f77fad90f461 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java
@@ -22,9 +22,7 @@
import org.apache.ozone.test.LambdaTestUtils;
import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.reflect.MethodSignature;
-import org.junit.Rule;
import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -35,9 +33,6 @@
*/
public class TestSnapshotFeatureEnabledAspect {
- @Rule
- public TemporaryFolder temporaryFolder = new TemporaryFolder();
-
/**
* Check Aspect implementation with SnapshotFeatureEnabledUtil.
*/
From 3df43946e0fe60efbb80957fff40cb18fd193217 Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 18:28:12 -0700
Subject: [PATCH 10/17] Add workaround for HDDS-8529.
---
.../key/OMDirectoriesPurgeRequestWithFSO.java | 23 +++++++++++++++----
.../om/request/key/OMKeyPurgeRequest.java | 19 +++++++++++----
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
index 25756a3bad7f..e49921195bd2 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
@@ -26,6 +26,7 @@
import org.apache.commons.lang3.tuple.Pair;
import org.apache.hadoop.ozone.om.OMMetadataManager;
import org.apache.hadoop.ozone.om.OmSnapshot;
+import org.apache.hadoop.ozone.om.OmSnapshotManager;
import org.apache.hadoop.ozone.om.OzoneManager;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
@@ -58,6 +59,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) {
OzoneManagerProtocolProtos.PurgeDirectoriesRequest purgeDirsRequest =
getOmRequest().getPurgeDirectoriesRequest();
+ OmSnapshotManager omSnapshotManager = ozoneManager.getOmSnapshotManager();
String fromSnapshot = purgeDirsRequest.hasSnapshotTableKey() ?
purgeDirsRequest.getSnapshotTableKey() : null;
@@ -73,11 +75,22 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
SnapshotInfo snapshotInfo =
ozoneManager.getMetadataManager().getSnapshotInfoTable()
.get(fromSnapshot);
- // TODO: [SNAPSHOT] As a workaround before HDDS-8529 is merged,
- // if snapshotInfo is not null but omSnapshotManager is null,
- // abort OM and prompt the user to enable snapshot feature first
- // in order to keep OM consistent.
- omFromSnapshot = (OmSnapshot) ozoneManager.getOmSnapshotManager()
+ if (snapshotInfo != null && omSnapshotManager == null) {
+ // TODO: [SNAPSHOT] As a workaround before HDDS-8529 is merged,
+ // if snapshotInfo is not null but omSnapshotManager is null,
+ // abort OM and prompt the user to enable snapshot feature first
+ // in order to keep OM consistent.
+ // Remove this once HDDS-8529 is done.
+ throw new IllegalStateException("Unable to process the current " +
+ "OMKeyPurgeRequest as a result of OmSnapshotManager not being " +
+ "initialized. Please enable Ozone snapshot feature in " +
+ "ozone-site.xml for this OM by setting " +
+ "ozone.filesystem.snapshot.enabled to true. After this request " +
+ "is successfully applied, Ozone snapshot feature can be safely " +
+ "disabled again. " +
+ "This workaround would not be required when HDDS-8529 is done");
+ }
+ omFromSnapshot = (OmSnapshot) omSnapshotManager
.checkForSnapshot(snapshotInfo.getVolumeName(),
snapshotInfo.getBucketName(),
getSnapshotPrefix(snapshotInfo.getName()));
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java
index c236e1fa45ab..b8f1af83b415 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java
@@ -80,10 +80,21 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
SnapshotInfo snapshotInfo =
ozoneManager.getMetadataManager().getSnapshotInfoTable()
.get(fromSnapshot);
- // TODO: [SNAPSHOT] As a workaround before HDDS-8529 is merged,
- // if snapshotInfo is not null but omSnapshotManager is null,
- // abort OM and prompt the user to enable snapshot feature first
- // in order to keep OM consistent.
+ if (snapshotInfo != null && omSnapshotManager == null) {
+ // TODO: [SNAPSHOT] As a workaround before HDDS-8529 is merged,
+ // if snapshotInfo is not null but omSnapshotManager is null,
+ // abort OM and prompt the user to enable snapshot feature first
+ // in order to keep OM consistent.
+ // Remove this once HDDS-8529 is done.
+ throw new IllegalStateException("Unable to process the current " +
+ "OMKeyPurgeRequest as a result of OmSnapshotManager not being " +
+ "initialized. Please enable Ozone snapshot feature in " +
+ "ozone-site.xml for this OM by setting " +
+ "ozone.filesystem.snapshot.enabled to true. After this request " +
+ "is successfully applied, Ozone snapshot feature can be safely " +
+ "disabled again. " +
+ "This workaround would not be required when HDDS-8529 is done");
+ }
omFromSnapshot = (OmSnapshot) omSnapshotManager
.checkForSnapshot(snapshotInfo.getVolumeName(),
snapshotInfo.getBucketName(),
From 484a52982bc6b363df09a2b082011a622cb48791 Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 18:39:36 -0700
Subject: [PATCH 11/17] Rename `SnapshotFeatureEnabled` to
`RequireSnapshotFeatureState`; comment and code clean up.
---
.../snapshot/OMSnapshotCreateRequest.java | 4 +-
.../snapshot/OMSnapshotDeleteRequest.java | 2 +
.../OMSnapshotMoveDeletedKeysRequest.java | 1 +
.../snapshot/OMSnapshotPurgeRequest.java | 1 +
....java => RequireSnapshotFeatureState.java} | 5 +--
...=> RequireSnapshotFeatureStateAspect.java} | 44 +++++--------------
.../src/main/resources/META-INF/aop.xml | 4 +-
.../key/TestOMKeyPurgeRequestAndResponse.java | 1 -
.../snapshot/SnapshotFeatureEnabledUtil.java | 2 +-
...estRequireSnapshotFeatureStateAspect.java} | 5 ++-
10 files changed, 25 insertions(+), 44 deletions(-)
rename hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/{SnapshotFeatureEnabled.java => RequireSnapshotFeatureState.java} (86%)
rename hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/{SnapshotFeatureEnabledAspect.java => RequireSnapshotFeatureStateAspect.java} (76%)
rename hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/{TestSnapshotFeatureEnabledAspect.java => TestRequireSnapshotFeatureStateAspect.java} (93%)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
index 65c89aecd658..da81825ef2ee 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
@@ -36,7 +36,7 @@
import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.om.response.snapshot.OMSnapshotCreateResponse;
-import org.apache.hadoop.ozone.om.snapshot.SnapshotFeatureEnabled;
+import org.apache.hadoop.ozone.om.snapshot.RequireSnapshotFeatureState;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.CreateSnapshotRequest;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.CreateSnapshotResponse;
@@ -88,7 +88,7 @@ public OMSnapshotCreateRequest(OMRequest omRequest) {
}
@Override
- @SnapshotFeatureEnabled(true)
+ @RequireSnapshotFeatureState(true)
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
final OMRequest omRequest = super.preExecute(ozoneManager);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotDeleteRequest.java
index 7af3f1486fd3..10ff8c29c7b8 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotDeleteRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotDeleteRequest.java
@@ -33,6 +33,7 @@
import org.apache.hadoop.ozone.om.request.util.OmResponseUtil;
import org.apache.hadoop.ozone.om.response.OMClientResponse;
import org.apache.hadoop.ozone.om.response.snapshot.OMSnapshotDeleteResponse;
+import org.apache.hadoop.ozone.om.snapshot.RequireSnapshotFeatureState;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteSnapshotRequest;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteSnapshotResponse;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest;
@@ -63,6 +64,7 @@ public OMSnapshotDeleteRequest(OMRequest omRequest) {
}
@Override
+ @RequireSnapshotFeatureState(true)
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
final OMRequest omRequest = super.preExecute(ozoneManager);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveDeletedKeysRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveDeletedKeysRequest.java
index 1b0488a326c7..10a5b19e1c5b 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveDeletedKeysRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotMoveDeletedKeysRequest.java
@@ -45,6 +45,7 @@
/**
* Handles OMSnapshotMoveDeletedKeys Request.
+ * This is an OM internal request. Does not need @RequireSnapshotFeatureState.
*/
public class OMSnapshotMoveDeletedKeysRequest extends OMClientRequest {
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 30409c047342..20ce2cc240e6 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
@@ -33,6 +33,7 @@
/**
* Handles OMSnapshotPurge Request.
+ * This is an OM internal request. Does not need @RequireSnapshotFeatureState.
*/
public class OMSnapshotPurgeRequest extends OMClientRequest {
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureState.java
similarity index 86%
rename from hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java
rename to hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureState.java
index dd7a03b3ac9d..97737a1f7062 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabled.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureState.java
@@ -24,11 +24,10 @@
import java.lang.annotation.Target;
/**
- * Annotation used to check that snapshot feature is enabled or disabled.
- * TODO: Rename this to RequireSnapshotFeatureState.
+ * Annotation used to check that the snapshot feature in desired state.
*/
@Target({ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)
-public @interface SnapshotFeatureEnabled {
+public @interface RequireSnapshotFeatureState {
boolean value();
}
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledAspect.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureStateAspect.java
similarity index 76%
rename from hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledAspect.java
rename to hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureStateAspect.java
index ad7c4d5fae82..3f74fc2a665d 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledAspect.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureStateAspect.java
@@ -39,20 +39,20 @@
* 'Aspect' for checking whether snapshot feature is enabled.
* All methods annotated with the specific annotation will have pre-processing
* done here to check layout version compatibility.
- * Note: Append to
+ * Note: Append class to
* hadoop-ozone/ozone-manager/src/main/resources/META-INF/aop.xml
* if the annotation doesn't seem to take affect on other classes' methods.
*/
@Aspect
-public class SnapshotFeatureEnabledAspect {
+public class RequireSnapshotFeatureStateAspect {
private static final Logger LOG = LoggerFactory
- .getLogger(SnapshotFeatureEnabledAspect.class);
+ .getLogger(RequireSnapshotFeatureStateAspect.class);
- @Before("@annotation(SnapshotFeatureEnabled) && execution(* *(..))")
+ @Before("@annotation(RequireSnapshotFeatureState) && execution(* *(..))")
public void checkLayoutFeature(JoinPoint joinPoint) throws IOException {
boolean desiredFeatureState = ((MethodSignature) joinPoint.getSignature())
- .getMethod().getAnnotation(SnapshotFeatureEnabled.class)
+ .getMethod().getAnnotation(RequireSnapshotFeatureState.class)
.value();
boolean isFeatureEnabled;
final Object[] args = joinPoint.getArgs();
@@ -74,8 +74,10 @@ public void checkLayoutFeature(JoinPoint joinPoint) throws IOException {
isFeatureEnabled = (boolean) method.invoke(joinPoint.getTarget());
} catch (Exception ex) {
// Exception being thrown here means this is not called from the UT.
- // Thus this is an unhandled usage. Add more case handling as needed.
- throw new NotImplementedException("Unhandled use case. Please impl");
+ // Thus this is an unhandled usage.
+ // Add more case handling logic as needed.
+ throw new NotImplementedException(
+ "Unhandled use case. Please implement.");
}
}
checkIsAllowed(joinPoint.getSignature().toShortString(),
@@ -106,36 +108,12 @@ private void checkIsAllowed(String operationName,
}
}
-// @Pointcut("execution(* " +
-// "org.apache.hadoop.ozone.om.request.OMClientRequest+.preExecute(..)) " +
-// "&& @this(org.apache.hadoop.ozone.om.upgrade.BelongsToLayoutVersion)")
-// public void omRequestPointCut() {
-// }
-
-// @Before("omRequestPointCut()")
-// public void beforeRequestApplyTxn(final JoinPoint joinPoint)
-// throws OMException {
-//
-// BelongsToLayoutVersion annotation = joinPoint.getTarget().getClass()
-// .getAnnotation(BelongsToLayoutVersion.class);
-// if (annotation == null) {
-// return;
-// }
-//
-// Object[] args = joinPoint.getArgs();
-// OzoneManager om = (OzoneManager) args[0];
-//
-// LayoutFeature lf = annotation.value();
-// checkIsAllowed(joinPoint.getTarget().getClass().getSimpleName(),
-// om.getVersionManager(), lf.name());
-// }
-
/**
* Note: Without this, it occasionally throws NoSuchMethodError when running
* the test.
*/
- public static SnapshotFeatureEnabledAspect aspectOf() {
- return new SnapshotFeatureEnabledAspect();
+ public static RequireSnapshotFeatureStateAspect aspectOf() {
+ return new RequireSnapshotFeatureStateAspect();
}
}
diff --git a/hadoop-ozone/ozone-manager/src/main/resources/META-INF/aop.xml b/hadoop-ozone/ozone-manager/src/main/resources/META-INF/aop.xml
index 2b788086ae5f..a96b5e8b71bc 100644
--- a/hadoop-ozone/ozone-manager/src/main/resources/META-INF/aop.xml
+++ b/hadoop-ozone/ozone-manager/src/main/resources/META-INF/aop.xml
@@ -15,9 +15,9 @@
-
+
-
+
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java
index 42c849d774db..19df5dee21b7 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyPurgeRequestAndResponse.java
@@ -30,7 +30,6 @@
import org.apache.hadoop.ozone.om.request.snapshot.TestOMSnapshotCreateRequest;
import org.apache.hadoop.ozone.om.response.snapshot.OMSnapshotCreateResponse;
import org.junit.Assert;
-import org.junit.Before;
import org.junit.Test;
import org.apache.hadoop.ozone.om.response.key.OMKeyPurgeResponse;
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledUtil.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledUtil.java
index fa3030484e53..3eea670dcaab 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledUtil.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/SnapshotFeatureEnabledUtil.java
@@ -26,7 +26,7 @@ public class SnapshotFeatureEnabledUtil {
/**
* This is an example of an "API" that requires snapshot feature enabled.
*/
- @SnapshotFeatureEnabled(true)
+ @RequireSnapshotFeatureState(true)
public String snapshotMethod() {
return "yay";
}
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestRequireSnapshotFeatureStateAspect.java
similarity index 93%
rename from hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java
rename to hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestRequireSnapshotFeatureStateAspect.java
index f77fad90f461..07e15ff963ee 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotFeatureEnabledAspect.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestRequireSnapshotFeatureStateAspect.java
@@ -31,7 +31,7 @@
* Class to test annotation based interceptor that checks whether
* Ozone snapshot feature is enabled.
*/
-public class TestSnapshotFeatureEnabledAspect {
+public class TestRequireSnapshotFeatureStateAspect {
/**
* Check Aspect implementation with SnapshotFeatureEnabledUtil.
@@ -39,7 +39,8 @@ public class TestSnapshotFeatureEnabledAspect {
@Test
public void testSnapshotFeatureEnabledAnnotation() throws Exception {
SnapshotFeatureEnabledUtil testObj = new SnapshotFeatureEnabledUtil();
- SnapshotFeatureEnabledAspect aspect = new SnapshotFeatureEnabledAspect();
+ RequireSnapshotFeatureStateAspect
+ aspect = new RequireSnapshotFeatureStateAspect();
JoinPoint joinPoint = mock(JoinPoint.class);
when(joinPoint.getTarget()).thenReturn(testObj);
From 18b39dccbf4721a1696fcb792ec9b4827c9bb016 Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 18:45:05 -0700
Subject: [PATCH 12/17] Add debug logging in RequireSnapshotFeatureStateAspect.
---
.../ozone/om/snapshot/RequireSnapshotFeatureStateAspect.java | 5 ++++-
.../om/snapshot/TestRequireSnapshotFeatureStateAspect.java | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureStateAspect.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureStateAspect.java
index 3f74fc2a665d..5f14e841c600 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureStateAspect.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureStateAspect.java
@@ -50,12 +50,15 @@ public class RequireSnapshotFeatureStateAspect {
.getLogger(RequireSnapshotFeatureStateAspect.class);
@Before("@annotation(RequireSnapshotFeatureState) && execution(* *(..))")
- public void checkLayoutFeature(JoinPoint joinPoint) throws IOException {
+ public void checkFeatureState(JoinPoint joinPoint) throws IOException {
boolean desiredFeatureState = ((MethodSignature) joinPoint.getSignature())
.getMethod().getAnnotation(RequireSnapshotFeatureState.class)
.value();
boolean isFeatureEnabled;
final Object[] args = joinPoint.getArgs();
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("joinPoint.getTarget() = {}", joinPoint.getTarget());
+ }
if (joinPoint.getTarget() instanceof OzoneManagerRequestHandler) {
OzoneManager ozoneManager = ((OzoneManagerRequestHandler)
joinPoint.getTarget()).getOzoneManager();
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestRequireSnapshotFeatureStateAspect.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestRequireSnapshotFeatureStateAspect.java
index 07e15ff963ee..b94977455fd0 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestRequireSnapshotFeatureStateAspect.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestRequireSnapshotFeatureStateAspect.java
@@ -54,6 +54,6 @@ public void testSnapshotFeatureEnabledAnnotation() throws Exception {
LambdaTestUtils.intercept(OMException.class,
"Operation snapshotMethod cannot be invoked because " +
"Ozone snapshot feature is disabled",
- () -> aspect.checkLayoutFeature(joinPoint));
+ () -> aspect.checkFeatureState(joinPoint));
}
}
From a9be4428ae849b5d0e5b47ae580fddf7987ba2ba Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 18:56:58 -0700
Subject: [PATCH 13/17] Add one more `omSnapshotManager` null check in
`installCheckpoint`.
---
.../src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
index 75a48452e3b0..1c8402155475 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
@@ -3679,7 +3679,7 @@ TermIndex installCheckpoint(String leaderId, Path checkpointLocation,
termIndex, checkpointTrxnInfo.getTermIndex());
}
- if (oldOmMetadataManagerStopped) {
+ if (oldOmMetadataManagerStopped && omSnapshotManager != null) {
// Close snapDiff's rocksDB instance only if metadataManager gets closed.
omSnapshotManager.close();
}
From 48db6b61a993fef47a6da90feba87942fa1b496a Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 18:59:20 -0700
Subject: [PATCH 14/17] Can't disable `omSnapshotManager` as all read paths now
requires `omSnapshotManager.checkForSnapshot`; Just disables
`snapshotDiffCleanupService` for now.
---
.../hadoop/ozone/om/OmSnapshotManager.java | 26 +++++++++++--------
.../apache/hadoop/ozone/om/OzoneManager.java | 8 ++----
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
index de19bb273122..bcc18a492a89 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
@@ -252,17 +252,21 @@ public OmSnapshotManager(OzoneManager ozoneManager) {
OZONE_OM_SNAPSHOT_DIFF_CLEANUP_SERVICE_TIMEOUT_DEFAULT,
TimeUnit.MILLISECONDS);
- this.snapshotDiffCleanupService = new SnapshotDiffCleanupService(
- diffCleanupServiceInterval,
- diffCleanupServiceTimeout,
- ozoneManager,
- snapshotDiffDb,
- snapDiffJobCf,
- snapDiffPurgedJobCf,
- snapDiffReportCf,
- codecRegistry
- );
- this.snapshotDiffCleanupService.start();
+ if (ozoneManager.isFilesystemSnapshotEnabled()) {
+ this.snapshotDiffCleanupService = new SnapshotDiffCleanupService(
+ diffCleanupServiceInterval,
+ diffCleanupServiceTimeout,
+ ozoneManager,
+ snapshotDiffDb,
+ snapDiffJobCf,
+ snapDiffPurgedJobCf,
+ snapDiffReportCf,
+ codecRegistry
+ );
+ this.snapshotDiffCleanupService.start();
+ } else {
+ this.snapshotDiffCleanupService = null;
+ }
}
private CacheLoader createCacheLoader() {
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
index 1c8402155475..f2eaf817152f 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
@@ -790,12 +790,8 @@ private void instantiateServices(boolean withNewSnapshot) throws IOException {
omMetadataReader = new OmMetadataReader(keyManager, prefixManager,
this, LOG, AUDIT, metrics);
- // TODO: [SNAPSHOT] Remove this condition when HDDS-8529 is done.
- if (isFilesystemSnapshotEnabled()) {
- omSnapshotManager = new OmSnapshotManager(this);
- } else {
- omSnapshotManager = null;
- }
+ // TODO: [SNAPSHOT] Revisit this in HDDS-8529.
+ omSnapshotManager = new OmSnapshotManager(this);
// Snapshot metrics
updateActiveSnapshotMetrics();
From 3e4c57c876b3239af4afd7ff365fe13615fbc32d Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 19:00:16 -0700
Subject: [PATCH 15/17] Disallow snapshot access when feature is disabled by
forcing `OmSnapshotManager#checkForSnapshot` to return active DB
OmMetadataReader instance.
---
.../main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
index bcc18a492a89..8b234a08cc25 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
@@ -513,7 +513,7 @@ private static void deleteKeysInSnapshotScopeFromDTableInternal(
public IOmMetadataReader checkForSnapshot(String volumeName,
String bucketName, String keyname)
throws IOException {
- if (keyname == null) {
+ if (keyname == null || !ozoneManager.isFilesystemSnapshotEnabled()) {
return ozoneManager.getOmMetadataReader();
}
From b832a91d274cf6eee2cfc1c70f0c16904bc81b16 Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Wed, 3 May 2023 19:03:03 -0700
Subject: [PATCH 16/17] Remove workaround now that `omSnapshotManager` won't be
null.
---
.../key/OMDirectoriesPurgeRequestWithFSO.java | 16 +---------------
.../ozone/om/request/key/OMKeyPurgeRequest.java | 16 +---------------
2 files changed, 2 insertions(+), 30 deletions(-)
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
index e49921195bd2..6926de1fedaa 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java
@@ -75,21 +75,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
SnapshotInfo snapshotInfo =
ozoneManager.getMetadataManager().getSnapshotInfoTable()
.get(fromSnapshot);
- if (snapshotInfo != null && omSnapshotManager == null) {
- // TODO: [SNAPSHOT] As a workaround before HDDS-8529 is merged,
- // if snapshotInfo is not null but omSnapshotManager is null,
- // abort OM and prompt the user to enable snapshot feature first
- // in order to keep OM consistent.
- // Remove this once HDDS-8529 is done.
- throw new IllegalStateException("Unable to process the current " +
- "OMKeyPurgeRequest as a result of OmSnapshotManager not being " +
- "initialized. Please enable Ozone snapshot feature in " +
- "ozone-site.xml for this OM by setting " +
- "ozone.filesystem.snapshot.enabled to true. After this request " +
- "is successfully applied, Ozone snapshot feature can be safely " +
- "disabled again. " +
- "This workaround would not be required when HDDS-8529 is done");
- }
+ // TODO: [SNAPSHOT] Revisit in HDDS-8529.
omFromSnapshot = (OmSnapshot) omSnapshotManager
.checkForSnapshot(snapshotInfo.getVolumeName(),
snapshotInfo.getBucketName(),
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java
index b8f1af83b415..1f49a731d587 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java
@@ -80,21 +80,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
SnapshotInfo snapshotInfo =
ozoneManager.getMetadataManager().getSnapshotInfoTable()
.get(fromSnapshot);
- if (snapshotInfo != null && omSnapshotManager == null) {
- // TODO: [SNAPSHOT] As a workaround before HDDS-8529 is merged,
- // if snapshotInfo is not null but omSnapshotManager is null,
- // abort OM and prompt the user to enable snapshot feature first
- // in order to keep OM consistent.
- // Remove this once HDDS-8529 is done.
- throw new IllegalStateException("Unable to process the current " +
- "OMKeyPurgeRequest as a result of OmSnapshotManager not being " +
- "initialized. Please enable Ozone snapshot feature in " +
- "ozone-site.xml for this OM by setting " +
- "ozone.filesystem.snapshot.enabled to true. After this request " +
- "is successfully applied, Ozone snapshot feature can be safely " +
- "disabled again. " +
- "This workaround would not be required when HDDS-8529 is done");
- }
+ // TODO: [SNAPSHOT] Revisit in HDDS-8529.
omFromSnapshot = (OmSnapshot) omSnapshotManager
.checkForSnapshot(snapshotInfo.getVolumeName(),
snapshotInfo.getBucketName(),
From c71cf5d268a2150ff29d0b3c4523b619588ae4fe Mon Sep 17 00:00:00 2001
From: Siyao Meng <50227127+smengcl@users.noreply.github.com>
Date: Thu, 4 May 2023 01:47:33 -0700
Subject: [PATCH 17/17] Revert `omSnapshotManager` null handling; add
integration test.
---
.../ozone/om/TestOmSnapshotDisabled.java | 103 ++++++++++++++++++
.../ozone/om/TestOmSnapshotFileSystem.java | 2 -
.../ozone/om/OmMetadataManagerImpl.java | 13 +--
.../hadoop/ozone/om/OmSnapshotManager.java | 16 ++-
.../apache/hadoop/ozone/om/OzoneManager.java | 7 +-
.../snapshot/OMSnapshotCreateRequest.java | 1 -
.../om/service/DirectoryDeletingService.java | 13 +--
.../snapshot/RequireSnapshotFeatureState.java | 1 +
8 files changed, 121 insertions(+), 35 deletions(-)
create mode 100644 hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotDisabled.java
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotDisabled.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotDisabled.java
new file mode 100644
index 000000000000..ef8012fc6af9
--- /dev/null
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotDisabled.java
@@ -0,0 +1,103 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.om;
+
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.IOUtils;
+import org.apache.hadoop.hdds.utils.db.DBProfile;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.MiniOzoneHAClusterImpl;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneVolume;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.ozone.test.LambdaTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+import java.util.UUID;
+
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DB_PROFILE;
+
+/**
+ * Integration test to verify Ozone snapshot RPCs throw exception when called.
+ */
+public class TestOmSnapshotDisabled {
+
+ private static MiniOzoneCluster cluster = null;
+ private static OzoneClient client;
+ private static ObjectStore store;
+
+ @BeforeAll
+ @Timeout(60)
+ public static void init() throws Exception {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ String clusterId = UUID.randomUUID().toString();
+ String scmId = UUID.randomUUID().toString();
+ conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT,
+ BucketLayout.LEGACY.name());
+ conf.setEnum(HDDS_DB_PROFILE, DBProfile.TEST);
+ // Disable filesystem snapshot feature for this test
+ conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, false);
+
+ cluster = MiniOzoneCluster.newOMHABuilder(conf)
+ .setClusterId(clusterId)
+ .setScmId(scmId)
+ .setOMServiceId("om-service-test1")
+ .setNumOfOzoneManagers(3)
+ .build();
+ cluster.waitForClusterToBeReady();
+ client = cluster.newClient();
+
+ OzoneManager leaderOzoneManager =
+ ((MiniOzoneHAClusterImpl) cluster).getOMLeader();
+ OzoneConfiguration leaderConfig = leaderOzoneManager.getConfiguration();
+ cluster.setConf(leaderConfig);
+ store = client.getObjectStore();
+ }
+
+ @AfterAll
+ public static void tearDown() throws Exception {
+ IOUtils.closeQuietly(client);
+ if (cluster != null) {
+ cluster.shutdown();
+ }
+ }
+
+ @Test
+ public void testExceptionThrown() throws Exception {
+ String volumeName = "vol-" + RandomStringUtils.randomNumeric(5);
+ String bucketName = "buck-" + RandomStringUtils.randomNumeric(5);
+ String snapshotName = "snap-" + RandomStringUtils.randomNumeric(5);
+
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ volume.createBucket(bucketName);
+
+ // create snapshot should throw
+ LambdaTestUtils.intercept(OMException.class, "FEATURE_NOT_ENABLED",
+ () -> store.createSnapshot(volumeName, bucketName, snapshotName));
+ // delete snapshot should throw
+ LambdaTestUtils.intercept(OMException.class, "FEATURE_NOT_ENABLED",
+ () -> store.deleteSnapshot(volumeName, bucketName, snapshotName));
+ }
+}
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotFileSystem.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotFileSystem.java
index cac8181743c3..872ae51235c7 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotFileSystem.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotFileSystem.java
@@ -108,8 +108,6 @@ public class TestOmSnapshotFileSystem {
private static final Logger LOG =
LoggerFactory.getLogger(TestOmSnapshot.class);
-
-
@Rule
public Timeout timeout = new Timeout(120, TimeUnit.SECONDS);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
index db37db1c6273..eadf8f7321e8 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
@@ -35,7 +35,6 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;
-import com.google.common.base.Preconditions;
import org.apache.hadoop.hdds.client.BlockID;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.utils.db.DBCheckpoint;
@@ -1472,14 +1471,8 @@ public List getPendingDeletionKeys(final int keyCount,
// Get volume name and bucket name
String[] keySplit = kv.getKey().split(OM_KEY_PREFIX);
// Get the latest snapshot in snapshot path.
- OmSnapshot latestSnapshot = null;
- if (omSnapshotManager != null) {
- latestSnapshot = getLatestSnapshot(
- keySplit[1], keySplit[2], omSnapshotManager);
- } else {
- LOG.debug("omSnapshotManager is not initialized. " +
- "Ozone snapshot feature might have been disabled.");
- }
+ OmSnapshot latestSnapshot = getLatestSnapshot(keySplit[1],
+ keySplit[2], omSnapshotManager);
String bucketKey = getBucketKey(keySplit[1], keySplit[2]);
OmBucketInfo bucketInfo = getBucketTable().get(bucketKey);
@@ -1560,8 +1553,6 @@ public OmSnapshot getLatestSnapshot(String volumeName, String bucketName,
OmSnapshotManager snapshotManager)
throws IOException {
- Preconditions.checkNotNull(snapshotManager);
-
String latestPathSnapshot =
snapshotChainManager.getLatestPathSnapshot(volumeName
+ OM_KEY_PREFIX + bucketName);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
index 8b234a08cc25..6a717a34fe4b 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
@@ -157,13 +157,17 @@ public final class OmSnapshotManager implements AutoCloseable {
private final int maxPageSize;
+ /**
+ * TODO: [SNAPSHOT] HDDS-8529: Refactor the constructor in a way that when
+ * ozoneManager.isFilesystemSnapshotEnabled() returns false,
+ * no snapshot-related background job or initialization would run,
+ * except for applying previously committed Ratis transactions in e.g.:
+ * 1. {@link OMKeyPurgeRequest#validateAndUpdateCache}
+ * 2. {@link OMDirectoriesPurgeRequestWithFSO#validateAndUpdateCache}
+ */
public OmSnapshotManager(OzoneManager ozoneManager) {
- // TODO: [SNAPSHOT] HDDS-8529: Refactor the constructor in a way that when
- // ozoneManager.isFilesystemSnapshotEnabled() is false, already committed
- // Ratis transactions can still apply in validateAndUpdateCache even when
- // snapshot feature is disabled. Affected requests:
- // 1. OMKeyPurgeRequest#validateAndUpdateCache
- // 2. OMDirectoriesPurgeRequestWithFSO#validateAndUpdateCache
+ LOG.info("Ozone filesystem snapshot feature is {}.",
+ ozoneManager.isFilesystemSnapshotEnabled() ? "enabled" : "disabled");
this.options = new ManagedDBOptions();
this.options.setCreateIfMissing(true);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
index f2eaf817152f..c8d08b6b0363 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
@@ -2210,10 +2210,7 @@ public void stop() {
}
stopTrashEmptier();
metadataManager.stop();
- // omSnapshotManager is null if ozone.filesystem.snapshot.enabled is false
- if (omSnapshotManager != null) {
- omSnapshotManager.close();
- }
+ omSnapshotManager.close();
metrics.unRegister();
omClientProtocolMetrics.unregister();
unregisterMXBean();
@@ -3675,7 +3672,7 @@ TermIndex installCheckpoint(String leaderId, Path checkpointLocation,
termIndex, checkpointTrxnInfo.getTermIndex());
}
- if (oldOmMetadataManagerStopped && omSnapshotManager != null) {
+ if (oldOmMetadataManagerStopped) {
// Close snapDiff's rocksDB instance only if metadataManager gets closed.
omSnapshotManager.close();
}
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
index da81825ef2ee..0bed12118d04 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java
@@ -90,7 +90,6 @@ public OMSnapshotCreateRequest(OMRequest omRequest) {
@Override
@RequireSnapshotFeatureState(true)
public OMRequest preExecute(OzoneManager ozoneManager) throws IOException {
-
final OMRequest omRequest = super.preExecute(ozoneManager);
// Verify name
OmUtils.validateSnapshotName(snapshotName);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
index 1a76abe782a5..ceed9ebe9aa7 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java
@@ -174,16 +174,9 @@ private boolean previousSnapshotHasDir(
OmMetadataManagerImpl metadataManager = (OmMetadataManagerImpl)
getOzoneManager().getMetadataManager();
- OmSnapshot latestSnapshot = null;
- if (omSnapshotManager != null) {
- latestSnapshot = metadataManager.getLatestSnapshot(
- deletedDirInfo.getVolumeName(),
- deletedDirInfo.getBucketName(),
- omSnapshotManager);
- } else {
- LOG.debug("omSnapshotManager is not initialized. " +
- "Ozone snapshot feature might have been disabled.");
- }
+ OmSnapshot latestSnapshot =
+ metadataManager.getLatestSnapshot(deletedDirInfo.getVolumeName(),
+ deletedDirInfo.getBucketName(), omSnapshotManager);
if (latestSnapshot != null) {
Table prevDirTable =
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureState.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureState.java
index 97737a1f7062..19d3f2805489 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureState.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/RequireSnapshotFeatureState.java
@@ -25,6 +25,7 @@
/**
* Annotation used to check that the snapshot feature in desired state.
+ * Can be generalized into checking arbitrary config state.
*/
@Target({ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)