This is an automated email from the ASF dual-hosted git repository.
swamirishi 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 33140cc3da HDDS-9432. Perform individual delete key instead of delete
range function (#5454)
33140cc3da is described below
commit 33140cc3dab284c7f830770c22b438d0dbb3a076
Author: Swaminathan Balachandran <[email protected]>
AuthorDate: Wed Oct 18 17:22:25 2023 -0700
HDDS-9432. Perform individual delete key instead of delete range function
(#5454)
* HDDS-9432. Perform individual delete key instead of delete range function
* HDDS-9432. Fix issue
* HDDS-9432. Fix checkstyle issue
* HDDS-9432. Address review comments
* HDDS-9432. Add test cases
* HDDS-9432. Fix test case
* HDDS-9432. Address review comment
* HDDS-9432. Address review comment
* HDDS-9432. Fix checkstyle
* HDDS-9432. Fix javadocs
---
.../hadoop/hdds/utils/ThrowableFunction.java | 27 ++++++
.../apache/hadoop/ozone/om/OmSnapshotManager.java | 100 +++++++++------------
.../snapshot/TestOMSnapshotCreateResponse.java | 80 +++++++++--------
3 files changed, 113 insertions(+), 94 deletions(-)
diff --git
a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/ThrowableFunction.java
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/ThrowableFunction.java
new file mode 100644
index 0000000000..d6dbb6871d
--- /dev/null
+++
b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/ThrowableFunction.java
@@ -0,0 +1,27 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.hdds.utils;
+
+
+/**
+ * Utility interface for function which throws exceptions.
+ * Similar to {@link java.util.function.Function}.
+ */
+public interface ThrowableFunction<T, R, E extends Throwable> {
+ R apply(T t) throws E;
+}
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 4ec14e1596..d88bd13c0d 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
@@ -40,6 +40,7 @@ import com.google.common.cache.RemovalListener;
import org.apache.hadoop.hdds.StringUtils;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.server.ServerUtils;
+import org.apache.hadoop.hdds.utils.ThrowableFunction;
import org.apache.hadoop.hdds.utils.db.CodecRegistry;
import org.apache.hadoop.hdds.utils.db.DBCheckpoint;
import org.apache.hadoop.hdds.utils.db.RDBStore;
@@ -473,24 +474,24 @@ public final class OmSnapshotManager implements
AutoCloseable {
* @param bucketName bucket name
*/
private static void deleteKeysFromDelDirTableInSnapshotScope(
- OMMetadataManager omMetadataManager,
- String volumeName,
+ OMMetadataManager omMetadataManager, String volumeName,
String bucketName) throws IOException {
// Range delete start key (inclusive)
- final String beginKey = getOzonePathKeyWithVolumeBucketNames(
+ final String keyPrefix = getOzonePathKeyWithVolumeBucketNames(
omMetadataManager, volumeName, bucketName);
- // Range delete end key (exclusive). To be calculated
- String endKey;
try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
- iter = omMetadataManager.getDeletedDirTable().iterator()) {
- endKey = findEndKeyGivenPrefix(iter, beginKey);
+ iter = omMetadataManager.getDeletedDirTable().iterator(keyPrefix)) {
+ performOperationOnKeys(iter,
+ entry -> {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Removing key {} from DeletedDirTable",
entry.getKey());
+ }
+ omMetadataManager.getDeletedDirTable().delete(entry.getKey());
+ return null;
+ });
}
-
- // Clean up deletedDirectoryTable
- deleteRangeInclusive(omMetadataManager.getDeletedDirTable(),
- beginKey, endKey);
}
/**
@@ -530,52 +531,37 @@ public final class OmSnapshotManager implements
AutoCloseable {
}
/**
- * Helper method to locate the end key with the given prefix and iterator.
+ * Helper method to perform operation on keys with a given iterator.
* @param keyIter TableIterator
- * @param keyPrefix DB key prefix String
- * @return endKey String, or null if no keys with such prefix is found
+ * @param operationFunction operation to be performed for each key.
*/
- private static String findEndKeyGivenPrefix(
+ private static void performOperationOnKeys(
TableIterator<String, ? extends Table.KeyValue<String, ?>> keyIter,
- String keyPrefix) throws IOException {
-
- String endKey;
- keyIter.seek(keyPrefix);
+ ThrowableFunction<Table.KeyValue<String, ?>,
+ Void, IOException> operationFunction) throws IOException {
// Continue only when there are entries of snapshot (bucket) scope
// in deletedTable in the first place
- if (!keyIter.hasNext()) {
- // No key matching keyPrefix. No need to do delete or deleteRange at all.
- endKey = null;
- } else {
- // Remember the last key with a matching prefix
- endKey = keyIter.next().getKey();
-
- // Loop until prefix mismatches.
- // TODO: [SNAPSHOT] Try to seek to next predicted bucket name instead of
- // the while-loop for a potential speed up?
- // Start performance tracking timer
- long startTime = System.nanoTime();
- while (keyIter.hasNext()) {
- Table.KeyValue<String, ?> entry = keyIter.next();
- String dbKey = entry.getKey();
- if (dbKey.startsWith(keyPrefix)) {
- endKey = dbKey;
- }
- }
- // Time took for the iterator to finish (in ns)
- long timeElapsed = System.nanoTime() - startTime;
- if (timeElapsed >= DB_TABLE_ITER_LOOP_THRESHOLD_NS) {
- // Print time elapsed
- LOG.warn("Took {} ns to find endKey. Caller is {}", timeElapsed,
- new Throwable().fillInStackTrace().getStackTrace()[1]
- .getMethodName());
- }
+ // Loop until prefix matches.
+ // Start performance tracking timer
+ long startTime = System.nanoTime();
+ while (keyIter.hasNext()) {
+ Table.KeyValue<String, ?> entry = keyIter.next();
+ operationFunction.apply(entry);
+ }
+ // Time took for the iterator to finish (in ns)
+ long timeElapsed = System.nanoTime() - startTime;
+ if (timeElapsed >= DB_TABLE_ITER_LOOP_THRESHOLD_NS) {
+ // Print time elapsed
+ LOG.warn("Took {} ns to find endKey. Caller is {}", timeElapsed,
+ new Throwable().fillInStackTrace().getStackTrace()[1]
+ .getMethodName());
}
- return endKey;
}
/**
* Helper method to do deleteRange on a table, including endKey.
+ * TODO: Do remove this method, it is not used anywhere. Need to check if
+ * deleteRange causes RocksDB corruption.
* TODO: Move this into {@link Table} ?
* @param table Table
* @param beginKey begin key
@@ -600,25 +586,25 @@ public final class OmSnapshotManager implements
AutoCloseable {
* @param bucketName bucket name
*/
private static void deleteKeysFromDelKeyTableInSnapshotScope(
- OMMetadataManager omMetadataManager,
- String volumeName,
+ OMMetadataManager omMetadataManager, String volumeName,
String bucketName) throws IOException {
// Range delete start key (inclusive)
- final String beginKey =
+ final String keyPrefix =
omMetadataManager.getOzoneKey(volumeName, bucketName, "");
- // Range delete end key (exclusive). To be found
- String endKey;
try (TableIterator<String,
? extends Table.KeyValue<String, RepeatedOmKeyInfo>>
- iter = omMetadataManager.getDeletedTable().iterator()) {
- endKey = findEndKeyGivenPrefix(iter, beginKey);
+ iter = omMetadataManager.getDeletedTable().iterator(keyPrefix)) {
+ performOperationOnKeys(iter, entry -> {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Removing key {} from DeletedTable", entry.getKey());
+ }
+ omMetadataManager.getDeletedTable().delete(entry.getKey());
+ return null;
+ });
}
- // Clean up deletedTable
- deleteRangeInclusive(omMetadataManager.getDeletedTable(), beginKey,
endKey);
-
// No need to invalidate deletedTable (or deletedDirectoryTable) table
// cache since entries are not added to its table cache in the first place.
// See OMKeyDeleteRequest and OMKeyPurgeRequest#validateAndUpdateCache.
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotCreateResponse.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotCreateResponse.java
index bd551ab5c2..e837a17360 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotCreateResponse.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/snapshot/TestOMSnapshotCreateResponse.java
@@ -34,12 +34,12 @@ import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
import org.apache.hadoop.util.Time;
-import org.junit.After;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.om.OMConfigKeys;
@@ -60,32 +60,33 @@ import static
org.apache.hadoop.ozone.om.OmSnapshotManager.getSnapshotPath;
*/
public class TestOMSnapshotCreateResponse {
- @Rule
- public TemporaryFolder folder = new TemporaryFolder();
-
+ @TempDir
+ private File folder;
+
private OMMetadataManager omMetadataManager;
private BatchOperation batchOperation;
private OzoneConfiguration ozoneConfiguration;
- @Before
+ @BeforeEach
public void setup() throws Exception {
ozoneConfiguration = new OzoneConfiguration();
- String fsPath = folder.newFolder().getAbsolutePath();
+ String fsPath = folder.getAbsolutePath();
ozoneConfiguration.set(OMConfigKeys.OZONE_OM_DB_DIRS,
fsPath);
omMetadataManager = new OmMetadataManagerImpl(ozoneConfiguration, null);
batchOperation = omMetadataManager.getStore().initBatchOperation();
}
- @After
+ @AfterEach
public void tearDown() {
if (batchOperation != null) {
batchOperation.close();
}
}
- @Test
- public void testAddToDBBatch() throws Exception {
+ @ParameterizedTest
+ @ValueSource(ints = {0, 1, 5, 10, 25})
+ public void testAddToDBBatch(int numberOfKeys) throws Exception {
String volumeName = UUID.randomUUID().toString();
String bucketName = UUID.randomUUID().toString();
String snapshotName = UUID.randomUUID().toString();
@@ -97,15 +98,15 @@ public class TestOMSnapshotCreateResponse {
Time.now());
// confirm table is empty
- Assert.assertEquals(0,
+ Assertions.assertEquals(0,
omMetadataManager
- .countRowsInTable(omMetadataManager.getSnapshotInfoTable()));
+ .countRowsInTable(omMetadataManager.getSnapshotInfoTable()));
// Populate deletedTable and deletedDirectoryTable
Set<String> dtSentinelKeys =
- addTestKeysToDeletedTable(volumeName, bucketName);
+ addTestKeysToDeletedTable(volumeName, bucketName, numberOfKeys);
Set<String> ddtSentinelKeys =
- addTestKeysToDeletedDirTable(volumeName, bucketName);
+ addTestKeysToDeletedDirTable(volumeName, bucketName, numberOfKeys);
// commit to table
OMSnapshotCreateResponse omSnapshotCreateResponse =
@@ -114,17 +115,17 @@ public class TestOMSnapshotCreateResponse {
.setStatus(OzoneManagerProtocolProtos.Status.OK)
.setCreateSnapshotResponse(
CreateSnapshotResponse.newBuilder()
- .setSnapshotInfo(snapshotInfo.getProtobuf())
- .build()).build(), snapshotInfo);
+ .setSnapshotInfo(snapshotInfo.getProtobuf())
+ .build()).build(), snapshotInfo);
omSnapshotCreateResponse.addToDBBatch(omMetadataManager, batchOperation);
omMetadataManager.getStore().commitBatchOperation(batchOperation);
// Confirm snapshot directory was created
String snapshotDir = getSnapshotPath(ozoneConfiguration, snapshotInfo);
- Assert.assertTrue((new File(snapshotDir)).exists());
+ Assertions.assertTrue((new File(snapshotDir)).exists());
// Confirm table has 1 entry
- Assert.assertEquals(1, omMetadataManager
+ Assertions.assertEquals(1, omMetadataManager
.countRowsInTable(omMetadataManager.getSnapshotInfoTable()));
// Check contents of entry
@@ -133,17 +134,19 @@ public class TestOMSnapshotCreateResponse {
it = omMetadataManager.getSnapshotInfoTable().iterator()) {
Table.KeyValue<String, SnapshotInfo> keyValue = it.next();
storedInfo = keyValue.getValue();
- Assert.assertEquals(snapshotInfo.getTableKey(), keyValue.getKey());
+ Assertions.assertEquals(snapshotInfo.getTableKey(), keyValue.getKey());
}
- Assert.assertEquals(snapshotInfo, storedInfo);
+ Assertions.assertEquals(snapshotInfo, storedInfo);
// Check deletedTable and deletedDirectoryTable clean up work as expected
verifyEntriesLeftInDeletedTable(dtSentinelKeys);
verifyEntriesLeftInDeletedDirTable(ddtSentinelKeys);
}
- private Set<String> addTestKeysToDeletedTable(
- String volumeName, String bucketName) throws IOException {
+ private Set<String> addTestKeysToDeletedTable(String volumeName,
+ String bucketName,
+ int numberOfKeys)
+ throws IOException {
RepeatedOmKeyInfo dummyRepeatedKeyInfo = new RepeatedOmKeyInfo.Builder()
.setOmKeyInfos(new ArrayList<>()).build();
@@ -176,7 +179,7 @@ public class TestOMSnapshotCreateResponse {
}
// Add deletedTable key entries in the snapshot (bucket) scope
- for (int i = 0; i < 10; i++) {
+ for (int i = 0; i < numberOfKeys; i++) {
String dtKey = omMetadataManager.getOzoneKey(volumeName, bucketName,
"dtkey" + i);
omMetadataManager.getDeletedTable().put(dtKey, dummyRepeatedKeyInfo);
@@ -189,12 +192,15 @@ public class TestOMSnapshotCreateResponse {
/**
* Populates deletedDirectoryTable for the test.
+ *
* @param volumeName volume name
* @param bucketName bucket name
* @return A set of DB keys
*/
- private Set<String> addTestKeysToDeletedDirTable(
- String volumeName, String bucketName) throws IOException {
+ private Set<String> addTestKeysToDeletedDirTable(String volumeName,
+ String bucketName,
+ int numberOfKeys)
+ throws IOException {
OMSnapshotResponseTestUtil.addVolumeBucketInfoToTable(
omMetadataManager, volumeName, bucketName);
@@ -218,7 +224,7 @@ public class TestOMSnapshotCreateResponse {
char bucketIdLastChar = dbKeyPfx.charAt(offset);
- String dbKeyPfxBefore = dbKeyPfx.substring(0, offset) +
+ String dbKeyPfxBefore = dbKeyPfx.substring(0, offset) +
(char) (bucketIdLastChar - 1) + dbKeyPfx.substring(offset);
for (int i = 0; i < 3; i++) {
String dtKey = dbKeyPfxBefore + "dir" + i;
@@ -226,7 +232,7 @@ public class TestOMSnapshotCreateResponse {
sentinelKeys.add(dtKey);
}
- String dbKeyPfxAfter = dbKeyPfx.substring(0, offset) +
+ String dbKeyPfxAfter = dbKeyPfx.substring(0, offset) +
(char) (bucketIdLastChar + 1) + dbKeyPfx.substring(offset);
for (int i = 0; i < 3; i++) {
String dtKey = dbKeyPfxAfter + "dir" + i;
@@ -235,7 +241,7 @@ public class TestOMSnapshotCreateResponse {
}
// Add key entries in the snapshot (bucket) scope
- for (int i = 0; i < 10; i++) {
+ for (int i = 0; i < numberOfKeys; i++) {
String dtKey = dbKeyPfx + "dir" + i;
omMetadataManager.getDeletedDirTable().put(dtKey, dummyOmKeyInfo);
// These are the keys that should be deleted.
@@ -261,18 +267,18 @@ public class TestOMSnapshotCreateResponse {
Table<String, ?> table, Set<String> expectedKeys) throws IOException {
try (TableIterator<String, ? extends Table.KeyValue<String, ?>>
- keyIter = table.iterator()) {
+ keyIter = table.iterator()) {
keyIter.seekToFirst();
while (keyIter.hasNext()) {
Table.KeyValue<String, ?> entry = keyIter.next();
String dbKey = entry.getKey();
- Assert.assertTrue(table.getName() + " should contain key",
- expectedKeys.contains(dbKey));
+ Assertions.assertTrue(expectedKeys.contains(dbKey),
+ table.getName() + " should contain key");
expectedKeys.remove(dbKey);
}
}
- Assert.assertTrue(table.getName() + " is missing keys that should be
there",
- expectedKeys.isEmpty());
+ Assertions.assertTrue(expectedKeys.isEmpty(),
+ table.getName() + " is missing keys that should be there");
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
