aswinshakil commented on code in PR #4651:
URL: https://github.com/apache/ozone/pull/4651#discussion_r1186414964
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -403,73 +406,147 @@ public static DBCheckpoint createOmSnapshotCheckpoint(
}
/**
- * Helper method to delete keys in the snapshot scope from active DB's
- * deletedTable.
- *
+ * Helper method to delete DB keys in the snapshot scope (bucket)
+ * from active DB's deletedDirectoryTable.
* @param omMetadataManager OMMetadataManager instance
* @param volumeName volume name
* @param bucketName bucket name
*/
- private static void deleteKeysInSnapshotScopeFromDTableInternal(
+ private static void deleteKeysFromDelDirTableInSnapshotScope(
OMMetadataManager omMetadataManager,
String volumeName,
String bucketName) throws IOException {
// Range delete start key (inclusive)
- String beginKey =
- omMetadataManager.getOzoneKey(volumeName, bucketName, "");
-
- // Range delete end key (exclusive) to be found
+ final String beginKey = getOzonePathKeyWithVolumeBucketNames(
+ omMetadataManager, volumeName, bucketName);
+ // Range delete end key (exclusive). To be calculated
String endKey;
- // Start performance tracking timer
- long startTime = System.nanoTime();
+ try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+ iter = omMetadataManager.getDeletedDirTable().iterator()) {
+ endKey = findEndKeyGivenPrefix(iter, beginKey);
+ }
- try (TableIterator<String,
- ? extends Table.KeyValue<String, RepeatedOmKeyInfo>>
- keyIter = omMetadataManager.getDeletedTable().iterator()) {
-
- keyIter.seek(beginKey);
- // Continue only when there are entries of snapshot (bucket) scope
- // in deletedTable in the first place
- if (!keyIter.hasNext()) {
- // Use null as a marker. No need to do 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 next predicted bucket name (speed up?)
- while (keyIter.hasNext()) {
- Table.KeyValue<String, RepeatedOmKeyInfo> entry = keyIter.next();
- String dbKey = entry.getKey();
- if (dbKey.startsWith(beginKey)) {
- endKey = dbKey;
- }
+ // Clean up deletedDirectoryTable
+ deleteRangeInclusive(omMetadataManager.getDeletedDirTable(),
+ beginKey, endKey);
+ }
+
+ /**
+ * Helper method to generate /volumeId/bucketId/ DB key prefix from given
+ * volume name and bucket name as a prefix in FSO deletedDirectoryTable.
+ * Follows:
+ * {@link OmMetadataManagerImpl#getOzonePathKey(long, long, long, String)}.
+ * @param volumeName volume name
+ * @param bucketName bucket name
+ * @return /volumeId/bucketId/
+ * e.g. /-9223372036854772480/-9223372036854771968/
+ */
+ public static String getOzonePathKeyWithVolumeBucketNames(
+ OMMetadataManager omMetadataManager,
+ String volumeName,
+ String bucketName) throws IOException {
+
+ final long volumeId = omMetadataManager.getVolumeId(volumeName);
+ final long bucketId = omMetadataManager.getBucketId(volumeName,
bucketName);
+ return OM_KEY_PREFIX + volumeId + OM_KEY_PREFIX + bucketId + OM_KEY_PREFIX;
+ }
+
+ /**
+ * Helper method to locate the end key with the given prefix and iterator.
+ * @param keyIter TableIterator
+ * @param keyPrefix DB key prefix String
+ * @return endKey String, or null if no keys with such prefix is found
+ */
+ private static String findEndKeyGivenPrefix(
+ TableIterator<String, ? extends Table.KeyValue<String, ?>> keyIter,
+ String keyPrefix) throws IOException {
+
+ String endKey;
+ keyIter.seek(keyPrefix);
+ // 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());
+ }
}
+ return endKey;
+ }
- // 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 clean up deletedTable", timeElapsed);
- }
+ /**
+ * Helper method to do deleteRange on a table, including endKey.
+ * TODO: Move this into {@link Table} ?
+ * @param table Table
+ * @param beginKey begin key
+ * @param endKey end key
+ */
+ private static void deleteRangeInclusive(
+ Table<String, ?> table, String beginKey, String endKey)
+ throws IOException {
if (endKey != null) {
- // Clean up deletedTable
- omMetadataManager.getDeletedTable().deleteRange(beginKey, endKey);
-
+ table.deleteRange(beginKey, endKey);
Review Comment:
Just wondering if this itself is an inclusive delete?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -367,9 +368,11 @@ public static DBCheckpoint createOmSnapshotCheckpoint(
dbCheckpoint = store.getSnapshot(snapshotInfo.getCheckpointDirName());
// Clean up active DB's deletedTable right after checkpoint is taken,
// with table write lock held
- deleteKeysInSnapshotScopeFromDTableInternal(omMetadataManager,
+ deleteKeysFromDelKeyTableInSnapshotScope(omMetadataManager,
+ snapshotInfo.getVolumeName(), snapshotInfo.getBucketName());
+ // Clean up deletedDirectoryTable as well
+ deleteKeysFromDelDirTableInSnapshotScope(omMetadataManager,
Review Comment:
Shouldn't we add table lock for `deletedDirTable` as well?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java:
##########
@@ -403,73 +406,147 @@ public static DBCheckpoint createOmSnapshotCheckpoint(
}
/**
- * Helper method to delete keys in the snapshot scope from active DB's
- * deletedTable.
- *
+ * Helper method to delete DB keys in the snapshot scope (bucket)
+ * from active DB's deletedDirectoryTable.
* @param omMetadataManager OMMetadataManager instance
* @param volumeName volume name
* @param bucketName bucket name
*/
- private static void deleteKeysInSnapshotScopeFromDTableInternal(
+ private static void deleteKeysFromDelDirTableInSnapshotScope(
OMMetadataManager omMetadataManager,
String volumeName,
String bucketName) throws IOException {
// Range delete start key (inclusive)
- String beginKey =
- omMetadataManager.getOzoneKey(volumeName, bucketName, "");
-
- // Range delete end key (exclusive) to be found
+ final String beginKey = getOzonePathKeyWithVolumeBucketNames(
+ omMetadataManager, volumeName, bucketName);
+ // Range delete end key (exclusive). To be calculated
String endKey;
- // Start performance tracking timer
- long startTime = System.nanoTime();
+ try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
+ iter = omMetadataManager.getDeletedDirTable().iterator()) {
+ endKey = findEndKeyGivenPrefix(iter, beginKey);
+ }
- try (TableIterator<String,
- ? extends Table.KeyValue<String, RepeatedOmKeyInfo>>
- keyIter = omMetadataManager.getDeletedTable().iterator()) {
-
- keyIter.seek(beginKey);
- // Continue only when there are entries of snapshot (bucket) scope
- // in deletedTable in the first place
- if (!keyIter.hasNext()) {
- // Use null as a marker. No need to do 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 next predicted bucket name (speed up?)
- while (keyIter.hasNext()) {
- Table.KeyValue<String, RepeatedOmKeyInfo> entry = keyIter.next();
- String dbKey = entry.getKey();
- if (dbKey.startsWith(beginKey)) {
- endKey = dbKey;
- }
+ // Clean up deletedDirectoryTable
+ deleteRangeInclusive(omMetadataManager.getDeletedDirTable(),
+ beginKey, endKey);
+ }
+
+ /**
+ * Helper method to generate /volumeId/bucketId/ DB key prefix from given
+ * volume name and bucket name as a prefix in FSO deletedDirectoryTable.
+ * Follows:
+ * {@link OmMetadataManagerImpl#getOzonePathKey(long, long, long, String)}.
+ * @param volumeName volume name
+ * @param bucketName bucket name
+ * @return /volumeId/bucketId/
+ * e.g. /-9223372036854772480/-9223372036854771968/
+ */
+ public static String getOzonePathKeyWithVolumeBucketNames(
Review Comment:
Can we move this to `OmMetadatadataManager` for consistency? I see all the
other similar methods are there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
