You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "aswinshakil (via GitHub)" <gi...@apache.org> on 2023/03/20 18:47:58 UTC
[GitHub] [ozone] aswinshakil opened a new pull request, #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
aswinshakil opened a new pull request, #4436:
URL: https://github.com/apache/ozone/pull/4436
## What changes were proposed in this pull request?
To track the renames of keys for only those buckets that are snapshotted.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8189
## How was this patch tested?
Unit tests and existing tests.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] aswinshakil merged pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "aswinshakil (via GitHub)" <gi...@apache.org>.
aswinshakil merged PR #4436:
URL: https://github.com/apache/ozone/pull/4436
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4436:
URL: https://github.com/apache/ozone/pull/4436#discussion_r1145102615
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotCreateResponse.java:
##########
@@ -77,9 +76,9 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
key, snapshotInfo);
// TODO: [SNAPSHOT] Move to createOmSnapshotCheckpoint and add table lock
- // Remove all entries from renamedKeyTable
- TableIterator<String, ? extends Table.KeyValue<String, OmKeyRenameInfo>>
- iterator = omMetadataManager.getRenamedKeyTable().iterator();
+ // Remove all entries from snapshotRenamedKeyTable
+ TableIterator<String, ? extends Table.KeyValue<String, String>>
Review Comment:
This is not your change but can you please use `try-with-resources` to close the iterator?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a diff in pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4436:
URL: https://github.com/apache/ozone/pull/4436#discussion_r1144055169
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java:
##########
@@ -230,14 +230,25 @@ String.class, new StringCodec(), OmKeyInfo.class,
SnapshotInfo.class,
new OmDBSnapshotInfoCodec());
- public static final DBColumnFamilyDefinition<String, OmKeyRenameInfo>
- RENAMED_KEY_TABLE =
+ /** <p>
+ * SnapshotRenamedKeyTable that complements the keyTable (or fileTable)
+ * entries of the immediately previous snapshot in the same snapshot
+ * scope. (bucket or volume)
+ * </p><p>
+ * Key renames between the two subsequent snapshots are captured, this
+ * information is used in {@link SnapshotDeletingService} to check if the
+ * renamedKey is present in the previous snapshot's keyTable
+ * (or fileTable)
+ * </p>
+ */
Review Comment:
```suggestion
/**
* SnapshotRenamedKeyTable that complements the keyTable (or fileTable)
* entries of the immediately previous snapshot in the same snapshot
* scope (bucket or volume).
* <p>
* Key renames between the two subsequent snapshots are captured, this
* information is used in {@link SnapshotDeletingService} to check if the
* renamedKey is present in the previous snapshot's keyTable
* (or fileTable).
*/
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] aswinshakil commented on a diff in pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "aswinshakil (via GitHub)" <gi...@apache.org>.
aswinshakil commented on code in PR #4436:
URL: https://github.com/apache/ozone/pull/4436#discussion_r1143830294
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -47,4 +60,40 @@ public static void checkClientRequestPrecondition(
OMException.ResultCodes.INTERNAL_ERROR);
}
}
+
+ public static boolean isSnapshotBucket(OMMetadataManager omMetadataManager,
+ OmKeyInfo keyInfo) throws IOException {
+
+ boolean status;
+ TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
+ iterator = omMetadataManager.getSnapshotInfoTable().iterator();
+ String dbSnapshotBucketKey = omMetadataManager.getBucketKey(
+ keyInfo.getVolumeName(), keyInfo.getBucketName())
+ + OM_KEY_PREFIX;
+
+ iterator.seek(dbSnapshotBucketKey);
+ status = iterator.hasNext() && iterator.next().getKey()
Review Comment:
We are also checking the snapshotDbKey which is `volume/bucket/snapName` along with the `seek()`, so it should be fine.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a diff in pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4436:
URL: https://github.com/apache/ozone/pull/4436#discussion_r1142985406
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java:
##########
@@ -230,14 +229,14 @@ String.class, new StringCodec(), OmKeyInfo.class,
SnapshotInfo.class,
new OmDBSnapshotInfoCodec());
- public static final DBColumnFamilyDefinition<String, OmKeyRenameInfo>
+ public static final DBColumnFamilyDefinition<String, String>
RENAMED_KEY_TABLE =
new DBColumnFamilyDefinition<>(
OmMetadataManagerImpl.RENAMED_KEY_TABLE,
String.class, // /volumeName/bucketName/objectID
new StringCodec(),
- OmKeyRenameInfo.class, // list of key renames
- new OmKeyRenameInfoCodec());
+ String.class, // renamed key
Review Comment:
```suggestion
String.class, // path to the key in previous snapshot's key(file)Table
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java:
##########
@@ -230,14 +229,14 @@ String.class, new StringCodec(), OmKeyInfo.class,
SnapshotInfo.class,
new OmDBSnapshotInfoCodec());
- public static final DBColumnFamilyDefinition<String, OmKeyRenameInfo>
+ public static final DBColumnFamilyDefinition<String, String>
Review Comment:
```suggestion
// SnapshotRenamedKeyTable that complements the keyTable (or fileTable) entries
// of the immediately previous snapshot in the same snapshot scope (bucket or volume)
public static final DBColumnFamilyDefinition<String, String>
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -47,4 +60,40 @@ public static void checkClientRequestPrecondition(
OMException.ResultCodes.INTERNAL_ERROR);
}
}
+
+ public static boolean isSnapshotBucket(OMMetadataManager omMetadataManager,
+ OmKeyInfo keyInfo) throws IOException {
+
+ boolean status;
+ TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
Review Comment:
Yes. Use try-with-resources
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java:
##########
@@ -230,14 +229,14 @@ String.class, new StringCodec(), OmKeyInfo.class,
SnapshotInfo.class,
new OmDBSnapshotInfoCodec());
- public static final DBColumnFamilyDefinition<String, OmKeyRenameInfo>
+ public static final DBColumnFamilyDefinition<String, String>
RENAMED_KEY_TABLE =
Review Comment:
```suggestion
SNAPSHOT_RENAMED_KEY_TABLE =
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java:
##########
@@ -230,14 +229,14 @@ String.class, new StringCodec(), OmKeyInfo.class,
SnapshotInfo.class,
new OmDBSnapshotInfoCodec());
- public static final DBColumnFamilyDefinition<String, OmKeyRenameInfo>
+ public static final DBColumnFamilyDefinition<String, String>
Review Comment:
Also it would be useful to put a larger block comment here to demonstrate the intended usage of this table.
Can asciify the tables in the design doc here.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -196,7 +194,7 @@ public class OmMetadataManagerImpl implements OMMetadataManager,
* |----------------------------------------------------------------------|
* | snapshotInfoTable | /volume/bucket/snapshotName -> SnapshotInfo |
* |----------------------------------------------------------------------|
- * | renamedKeyTable | /volumeName/bucketName/objectID -> OmKeyRenameInfo|
+ * | renamedKeyTable | /volumeName/bucketName/objectID -> renamedKey |
Review Comment:
Let's rename the table to be more specific.
```suggestion
* | snapshotRenamedKeyTable | /volName/buckName/objID -> /v/b/origKey |
```
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] aswinshakil commented on pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "aswinshakil (via GitHub)" <gi...@apache.org>.
aswinshakil commented on PR #4436:
URL: https://github.com/apache/ozone/pull/4436#issuecomment-1480353935
Thanks, @smengcl and @hemantk-12 for the review.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] aswinshakil commented on a diff in pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "aswinshakil (via GitHub)" <gi...@apache.org>.
aswinshakil commented on code in PR #4436:
URL: https://github.com/apache/ozone/pull/4436#discussion_r1143830294
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -47,4 +60,40 @@ public static void checkClientRequestPrecondition(
OMException.ResultCodes.INTERNAL_ERROR);
}
}
+
+ public static boolean isSnapshotBucket(OMMetadataManager omMetadataManager,
+ OmKeyInfo keyInfo) throws IOException {
+
+ boolean status;
+ TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
+ iterator = omMetadataManager.getSnapshotInfoTable().iterator();
+ String dbSnapshotBucketKey = omMetadataManager.getBucketKey(
+ keyInfo.getVolumeName(), keyInfo.getBucketName())
+ + OM_KEY_PREFIX;
+
+ iterator.seek(dbSnapshotBucketKey);
+ status = iterator.hasNext() && iterator.next().getKey()
Review Comment:
We are also checking the snapshotDbKey which is `/volumeName/bucketName/` along with the `seek()`, so it should be fine.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] aswinshakil commented on pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "aswinshakil (via GitHub)" <gi...@apache.org>.
aswinshakil commented on PR #4436:
URL: https://github.com/apache/ozone/pull/4436#issuecomment-1478418796
Thanks for the review @hemantk-12 and @smengcl. I have updated the PR, please take a look at it again.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4436:
URL: https://github.com/apache/ozone/pull/4436#discussion_r1142796852
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -47,4 +60,40 @@ public static void checkClientRequestPrecondition(
OMException.ResultCodes.INTERNAL_ERROR);
}
}
+
+ public static boolean isSnapshotBucket(OMMetadataManager omMetadataManager,
+ OmKeyInfo keyInfo) throws IOException {
+
+ boolean status;
+ TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
+ iterator = omMetadataManager.getSnapshotInfoTable().iterator();
+ String dbSnapshotBucketKey = omMetadataManager.getBucketKey(
+ keyInfo.getVolumeName(), keyInfo.getBucketName())
+ + OM_KEY_PREFIX;
+
+ iterator.seek(dbSnapshotBucketKey);
+ status = iterator.hasNext() && iterator.next().getKey()
Review Comment:
Curious if we need to check the volume and bucket in case of legacy bucket type. Similar to https://github.com/apache/ozone/blob/master/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java#L1240
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -648,9 +645,9 @@ protected void initializeOmTables(boolean addCacheMetrics)
String.class, SnapshotInfo.class);
checkTableStatus(snapshotInfoTable, SNAPSHOT_INFO_TABLE, addCacheMetrics);
- // objectID -> renamedKeys (renamed keys for key table)
+ // objectID -> renamed Key (renamed key for key table)
Review Comment:
```suggestion
// objectID -> renamedKey (renamed key for key table)
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -47,4 +60,40 @@ public static void checkClientRequestPrecondition(
OMException.ResultCodes.INTERNAL_ERROR);
}
}
+
+ public static boolean isSnapshotBucket(OMMetadataManager omMetadataManager,
+ OmKeyInfo keyInfo) throws IOException {
+
+ boolean status;
+ TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
+ iterator = omMetadataManager.getSnapshotInfoTable().iterator();
+ String dbSnapshotBucketKey = omMetadataManager.getBucketKey(
+ keyInfo.getVolumeName(), keyInfo.getBucketName())
+ + OM_KEY_PREFIX;
+
+ iterator.seek(dbSnapshotBucketKey);
+ status = iterator.hasNext() && iterator.next().getKey()
+ .startsWith(dbSnapshotBucketKey);
+
+ return status || checkSnapshotCache(omMetadataManager, keyInfo);
Review Comment:
Shouldn't we check cache first and then DB?
Like:
```
return checkSnapshotCache(omMetadataManager, keyInfo) || checkSnapshotInDB(omMetadataManager, keyInfo);
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -47,4 +60,40 @@ public static void checkClientRequestPrecondition(
OMException.ResultCodes.INTERNAL_ERROR);
}
}
+
+ public static boolean isSnapshotBucket(OMMetadataManager omMetadataManager,
+ OmKeyInfo keyInfo) throws IOException {
+
+ boolean status;
+ TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
+ iterator = omMetadataManager.getSnapshotInfoTable().iterator();
+ String dbSnapshotBucketKey = omMetadataManager.getBucketKey(
+ keyInfo.getVolumeName(), keyInfo.getBucketName())
+ + OM_KEY_PREFIX;
+
+ iterator.seek(dbSnapshotBucketKey);
+ status = iterator.hasNext() && iterator.next().getKey()
+ .startsWith(dbSnapshotBucketKey);
+
+ return status || checkSnapshotCache(omMetadataManager, keyInfo);
+ }
+
+ private static boolean checkSnapshotCache(OMMetadataManager omMetadataManager,
Review Comment:
```suggestion
private static boolean checkSnapshotInCache(OMMetadataManager omMetadataManager,
OmKeyInfo keyInfo)
```
Or `checkInSnapshotCache` whatever is more relevant. `checkSnapshotCache` is vague to me.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -47,4 +60,40 @@ public static void checkClientRequestPrecondition(
OMException.ResultCodes.INTERNAL_ERROR);
}
}
+
+ public static boolean isSnapshotBucket(OMMetadataManager omMetadataManager,
+ OmKeyInfo keyInfo) throws IOException {
+
+ boolean status;
+ TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
Review Comment:
Close the iterator after the use.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4436:
URL: https://github.com/apache/ozone/pull/4436#discussion_r1145102615
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotCreateResponse.java:
##########
@@ -77,9 +76,9 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
key, snapshotInfo);
// TODO: [SNAPSHOT] Move to createOmSnapshotCheckpoint and add table lock
- // Remove all entries from renamedKeyTable
- TableIterator<String, ? extends Table.KeyValue<String, OmKeyRenameInfo>>
- iterator = omMetadataManager.getRenamedKeyTable().iterator();
+ // Remove all entries from snapshotRenamedKeyTable
+ TableIterator<String, ? extends Table.KeyValue<String, String>>
Review Comment:
This is not use change but can you please use `try-with-resources` to close the iterator?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] smengcl commented on a diff in pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4436:
URL: https://github.com/apache/ozone/pull/4436#discussion_r1143866184
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotCreateRequest.java:
##########
@@ -77,6 +78,7 @@ public class TestOMSnapshotCreateRequest {
private String volumeName;
private String bucketName;
private String snapshotName;
+ private String snapshotName2;
Review Comment:
```suggestion
private String snapshotName1;
private String snapshotName2;
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java:
##########
@@ -230,14 +230,22 @@ String.class, new StringCodec(), OmKeyInfo.class,
SnapshotInfo.class,
new OmDBSnapshotInfoCodec());
- public static final DBColumnFamilyDefinition<String, OmKeyRenameInfo>
- RENAMED_KEY_TABLE =
+ /** <p> SnapshotRenamedKeyTable that complements the keyTable (or fileTable)
+ * entries of the immediately previous snapshot in the same snapshot
+ * scope. (bucket or volume) </p>
+ * <p> Key renames between the two subsequent snapshots are captured, this
+ * information is used in {@link SnapshotDeletingService} to check if the
+ * renamedKey is present in the previous snapshot's keyTable
+ * (or fileTable)</p>
+ */
+ public static final DBColumnFamilyDefinition<String, String>
Review Comment:
Reformat the javadoc a bit.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequestUtils.java:
##########
@@ -47,4 +60,42 @@ public static void checkClientRequestPrecondition(
OMException.ResultCodes.INTERNAL_ERROR);
}
}
+
+ public static boolean isSnapshotBucket(OMMetadataManager omMetadataManager,
+ OmKeyInfo keyInfo) throws IOException {
+
+ String dbSnapshotBucketKey = omMetadataManager.getBucketKey(
+ keyInfo.getVolumeName(), keyInfo.getBucketName())
+ + OM_KEY_PREFIX;
+
+ return checkInSnapshotCache(omMetadataManager, dbSnapshotBucketKey) ||
+ checkInSnapshotDB(omMetadataManager, dbSnapshotBucketKey);
+ }
+
+ private static boolean checkInSnapshotDB(OMMetadataManager omMetadataManager,
+ String dbSnapshotBucketKey) throws IOException {
+ try (TableIterator<String, ? extends Table.KeyValue<String, SnapshotInfo>>
+ iterator = omMetadataManager.getSnapshotInfoTable().iterator()) {
+ iterator.seek(dbSnapshotBucketKey);
+ return iterator.hasNext() && iterator.next().getKey()
+ .startsWith(dbSnapshotBucketKey);
+ }
+ }
+
+ private static boolean checkInSnapshotCache(
+ OMMetadataManager omMetadataManager,
+ String dbSnapshotBucketKey) {
+ Iterator<Map.Entry<CacheKey<String>, CacheValue<SnapshotInfo>>> cacheIter =
+ omMetadataManager.getSnapshotInfoTable().cacheIterator();
+
+ while (cacheIter.hasNext()) {
+ Map.Entry<CacheKey<String>, CacheValue<SnapshotInfo>> cacheKeyValue =
+ cacheIter.next();
+ String key = cacheKeyValue.getKey().getCacheKey();
+ if (key.startsWith(dbSnapshotBucketKey)) {
+ return true;
+ }
Review Comment:
Not only do we need to check for cache entry existence. We also need to check whether the entry is a tombstone entry (cache entry value is `null`) in cache.
If it is a tombstone, should return `false`.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] aswinshakil commented on a diff in pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "aswinshakil (via GitHub)" <gi...@apache.org>.
aswinshakil commented on code in PR #4436:
URL: https://github.com/apache/ozone/pull/4436#discussion_r1145114860
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotCreateResponse.java:
##########
@@ -77,9 +76,9 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
key, snapshotInfo);
// TODO: [SNAPSHOT] Move to createOmSnapshotCheckpoint and add table lock
- // Remove all entries from renamedKeyTable
- TableIterator<String, ? extends Table.KeyValue<String, OmKeyRenameInfo>>
- iterator = omMetadataManager.getRenamedKeyTable().iterator();
+ // Remove all entries from snapshotRenamedKeyTable
+ TableIterator<String, ? extends Table.KeyValue<String, String>>
Review Comment:
Done.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] aswinshakil commented on a diff in pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "aswinshakil (via GitHub)" <gi...@apache.org>.
aswinshakil commented on code in PR #4436:
URL: https://github.com/apache/ozone/pull/4436#discussion_r1143816490
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java:
##########
@@ -230,14 +229,14 @@ String.class, new StringCodec(), OmKeyInfo.class,
SnapshotInfo.class,
new OmDBSnapshotInfoCodec());
- public static final DBColumnFamilyDefinition<String, OmKeyRenameInfo>
+ public static final DBColumnFamilyDefinition<String, String>
Review Comment:
Sure, will add it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] aswinshakil commented on pull request #4436: HDDS-8189. [Snapshot] renamedKeyTable should only track keys in buckets that has at least one active snapshot.
Posted by "aswinshakil (via GitHub)" <gi...@apache.org>.
aswinshakil commented on PR #4436:
URL: https://github.com/apache/ozone/pull/4436#issuecomment-1480247457
The test passed here: https://github.com/aswinshakil/ozone/actions/runs/4492231188
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org