aswinshakil commented on code in PR #7200:
URL: https://github.com/apache/ozone/pull/7200#discussion_r1762037328
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -1976,6 +2032,27 @@ public Table.KeyValue<String, OmKeyInfo>
getPendingDeletionDir()
return null;
}
+ @Override
+ public TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
getPendingDeletionDirs()
+ throws IOException {
+ return this.getPendingDeletionDirs(null, null);
+ }
+
+ @Override
+ public TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
getPendingDeletionDirs(String volume,
+
String bucket)
+ throws IOException {
+
+ // Either both volume & bucket should be null or none of them should be
null.
+ if (!StringUtils.isBlank(volume) && StringUtils.isBlank(bucket) ||
+ StringUtils.isBlank(volume) && !StringUtils.isBlank(bucket)) {
+ throw new IOException("One of volume : " + volume + ", bucket: " +
bucket + " is empty. Either both should be " +
+ "empty or none of the arguments should be empty");
+ }
+ return StringUtils.isBlank(volume) ?
metadataManager.getDeletedDirTable().iterator() :
Review Comment:
Instead of this function returning both prefix-based iterator and full
iterator, we should separate it as different functions.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -838,6 +838,28 @@ public String getBucketKey(String volume, String bucket) {
return builder.toString();
}
+ /**
+ * Given a volume and bucket, return the corresponding DB key prefix.
+ *
+ * @param volume - User name
+ * @param bucket - Bucket name
+ */
+ @Override
+ public String getBucketKeyPrefix(String volume, String bucket) {
+ return OzoneFSUtils.addTrailingSlashIfNeeded(getBucketKey(volume, bucket));
+ }
+
+ /**
+ * Given a volume and bucket, return the corresponding DB key prefix.
+ *
+ * @param volume - User name
Review Comment:
nit: Volume name
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotUtils.java:
##########
@@ -86,6 +88,13 @@ public static SnapshotInfo getSnapshotInfo(final
OzoneManager ozoneManager,
return snapshotInfo;
}
+ public static SnapshotInfo getSnapshotInfo(OzoneManager ozoneManager,
+ SnapshotChainManager chainManager,
+ UUID snapshotId) throws
IOException {
+ String tableKey = chainManager.getTableKey(snapshotId);
Review Comment:
If the `tableKey` becomes `null` because of some race condition, Then we
should handle it gracefully.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -662,6 +663,61 @@ public PendingKeysDeletion getPendingDeletionKeys(final
int count)
.getPendingDeletionKeys(count, ozoneManager.getOmSnapshotManager());
}
+ @Override
+ public List<Table.KeyValue<String, String>> getRenamesKeyEntries(
+ String volume, String bucket, String startKey, int count) throws
IOException {
+ // Bucket prefix would be empty if volume is empty i.e. either null or "".
+ Optional<String> bucketPrefix = Optional.ofNullable(volume).map(vol ->
vol.isEmpty() ? null : vol)
+ .map(vol -> metadataManager.getBucketKeyPrefix(vol, bucket));
+ List<Table.KeyValue<String, String>> renamedEntries = new ArrayList<>();
+ try (TableIterator<String, ? extends Table.KeyValue<String, String>>
+ renamedKeyIter =
metadataManager.getSnapshotRenamedTable().iterator(bucketPrefix.orElse(""))) {
+
+ /* Seeking to the start key if it not null. The next key picked up would
be ensured to start with the bucket
+ prefix, {@link
org.apache.hadoop.hdds.utils.db.Table#iterator(bucketPrefix)} would ensure this.
+ */
+ if (startKey != null) {
+ renamedKeyIter.seek(startKey);
+ }
+ int currentCount = 0;
+ while (renamedKeyIter.hasNext() && currentCount < count) {
+ Table.KeyValue<String, String> kv = renamedKeyIter.next();
+ if (kv != null) {
+ renamedEntries.add(Table.newKeyValue(kv.getKey(), kv.getValue()));
+ currentCount++;
+ }
+ }
+ }
+ return renamedEntries;
+ }
+
+ @Override
+ public List<Table.KeyValue<String, List<OmKeyInfo>>> getDeletedKeyEntries(
+ String volume, String bucket, String startKey, int count) throws
IOException {
+ // Bucket prefix would be empty if volume is empty i.e. either null or "".
+ Optional<String> bucketPrefix = Optional.ofNullable(volume).map(vol ->
vol.isEmpty() ? null : vol)
+ .map(vol -> metadataManager.getBucketKeyPrefix(vol, bucket));
+ List<Table.KeyValue<String, List<OmKeyInfo>>> deletedKeyEntries = new
ArrayList<>(count);
+ try (TableIterator<String, ? extends Table.KeyValue<String,
RepeatedOmKeyInfo>>
+ delKeyIter =
metadataManager.getDeletedTable().iterator(bucketPrefix.orElse(""))) {
+
+ /* Seeking to the start key if it not null. The next key picked up would
be ensured to start with the bucket
+ prefix, {@link
org.apache.hadoop.hdds.utils.db.Table#iterator(bucketPrefix)} would ensure this.
+ */
+ if (startKey != null) {
+ delKeyIter.seek(startKey);
+ }
+ int currentCount = 0;
+ while (delKeyIter.hasNext() && currentCount < count) {
+ Table.KeyValue<String, RepeatedOmKeyInfo> kv = delKeyIter.next();
+ if (kv != null) {
+ deletedKeyEntries.add(Table.newKeyValue(kv.getKey(),
kv.getValue().cloneOmKeyInfoList()));
Review Comment:
As @prashantpogde mentioned we should update the `currentCount` here.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -662,6 +663,61 @@ public PendingKeysDeletion getPendingDeletionKeys(final
int count)
.getPendingDeletionKeys(count, ozoneManager.getOmSnapshotManager());
}
+ @Override
+ public List<Table.KeyValue<String, String>> getRenamesKeyEntries(
+ String volume, String bucket, String startKey, int count) throws
IOException {
+ // Bucket prefix would be empty if volume is empty i.e. either null or "".
+ Optional<String> bucketPrefix = Optional.ofNullable(volume).map(vol ->
vol.isEmpty() ? null : vol)
Review Comment:
We can have the checks that @hemantk-12 mentioned instead of this nested
mapping, making it hard to read and simplify this.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -1976,6 +2032,27 @@ public Table.KeyValue<String, OmKeyInfo>
getPendingDeletionDir()
return null;
}
+ @Override
+ public TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
getPendingDeletionDirs()
+ throws IOException {
+ return this.getPendingDeletionDirs(null, null);
+ }
+
+ @Override
+ public TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>>
getPendingDeletionDirs(String volume,
+
String bucket)
+ throws IOException {
+
+ // Either both volume & bucket should be null or none of them should be
null.
+ if (!StringUtils.isBlank(volume) && StringUtils.isBlank(bucket) ||
Review Comment:
AFAIK we don't allow whitespaces in volume or bucket names. So `isEmpty()`
should be enough.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java:
##########
@@ -119,6 +122,29 @@ ListKeysResult listKeys(String volumeName, String
bucketName, String startKey,
*/
PendingKeysDeletion getPendingDeletionKeys(int count) throws IOException;
+ /**
+ * Returns a list renamed entries from the snapshotRenamedTable.
+ *
+ * @param count max number of keys to return.
+ * @return a Pair of list of {@link
org.apache.hadoop.hdds.utils.db.Table.KeyValue} representing the keys in the
+ * underlying metadataManager.
+ * @throws IOException
+ */
+ List<Table.KeyValue<String, String>> getRenamesKeyEntries(
+ String volume, String bucket, String startKey, int count) throws
IOException;
+
+
+ /**
+ * Returns a list rename entries from the snapshotRenamedTable.
Review Comment:
Should be a list of deleted key entries from the `deletedTable`
--
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]
