You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "hemantk-12 (via GitHub)" <gi...@apache.org> on 2023/01/31 23:40:56 UTC
[GitHub] [ozone] hemantk-12 opened a new pull request, #4230: HDDS-7845. Appended the slash to copy to bucket path to fix the flaky snapshot restore tests.
hemantk-12 opened a new pull request, #4230:
URL: https://github.com/apache/ozone/pull/4230
## What changes were proposed in this pull request?
Snapshot [restore integration test](https://github.com/apache/ozone/blob/HDDS-6517-Snapshot/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java#L236) fails when restoring form bucket1's snapshot to bucket2 for legacy bucket type.
This change is to append the slash so that bucket path denormalized properly.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7861
## How was this patch tested?
Existing integration 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] hemantk-12 commented on a diff in pull request #4230: HDDS-7861. Delete keys from the source bucket to fix the flakiness of snapshot restore tests.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4230:
URL: https://github.com/apache/ozone/pull/4230#discussion_r1093668802
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
I agree with @smengcl on checkout intention of `createFakeDirIfShould` and that's why I went ahead by deleting the keys from source bucket because it doesn't matter for that test if keys are present in source bucket. The requirement for test is that keys should be present in snapshot path (snapshotKeyPrefix).
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
I agree with @smengcl on checking intention of `createFakeDirIfShould` and that's why I went ahead by deleting the keys from source bucket because it doesn't matter for that test if keys are present in source bucket. The requirement for test is that keys should be present in snapshot path (snapshotKeyPrefix).
--
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 #4230: HDDS-7861. Delete keys from the source bucket to fix the flakiness of snapshot restore tests.
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4230:
URL: https://github.com/apache/ozone/pull/4230#discussion_r1093857773
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
Thanks @hemantk-12 . Just for the record, HDDS-7870/HDDS-7871 would be the follow-up to properly fix the issue in the master branch.
--
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 #4230: HDDS-7861. Delete keys from the source bucket to fix the flakiness of snapshot restore tests.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4230:
URL: https://github.com/apache/ozone/pull/4230#discussion_r1093584820
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
Problem is that RocksDB's seek operation doesn't 100% guarantee that item key is available. Tho we believe that it is. When we seek if there is a directory for the key name, sometime it returns non-null response but in reality item doesn't exist.
[Here](https://github.com/apache/ozone/blob/d8765436c2bf76547973cc568f1648f1b1fe9fcb/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java#L1184), sometimes seek returns null and sometimes it return source bucket (which is wrong). When is returns source bucket, we create fake dir and return the response.
I was unsure what should be the correct behavior for this so I went ahead to delete keys from source bucket for now as temporary fix. But we should revisit the `createFakeDirIfShould` logic and fix it there. This is the original PR: https://github.com/apache/ozone/pull/3774/files
--
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 #4230: HDDS-7861. Delete keys from the source bucket to fix the flakiness of snapshot restore tests.
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4230:
URL: https://github.com/apache/ozone/pull/4230#discussion_r1093664505
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
> Problem is that RocksDB's seek operation doesn't 100% guarantee that item key is available. Tho we believe that it is. When we seek if there is a directory for the key name, sometime it returns non-null response but in reality item doesn't exist.
This could be a bug. When locating a key (check existence), OM is responsible for verifying if the key name exactly matches the input right after a DB seek. But I have yet to fully understand the intention of `createFakeDirIfShould` to make a call.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
> Problem is that RocksDB's seek operation doesn't 100% guarantee that item key is available. Tho we believe that it is. When we seek if there is a directory for the key name, sometime it returns non-null response but in reality item doesn't exist.
This could be a bug. When locating a key (e.g. checking existence), OM is responsible for verifying if the key name exactly matches the input right after a DB seek. But I have yet to fully understand the intention of `createFakeDirIfShould` to make a call.
--
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 #4230: HDDS-7861. Delete keys from the source bucket to fix the flakiness of snapshot restore tests.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4230:
URL: https://github.com/apache/ozone/pull/4230#discussion_r1093584820
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
Problem is that RocksDB's seek operation doesn't 100% guarantee that item key is available. Tho we believe that it is. When we seek if there is a directory for the key name, sometime it returns non-null response but in reality item doesn't exist.
[Here](https://github.com/apache/ozone/blob/d8765436c2bf76547973cc568f1648f1b1fe9fcb/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java#L1184), sometimes seek returns null and sometimes it return source bucket (which is wrong). When is returns source bucket, we create fake dir and return the response.
I was unsure what should be the correct behavior for this so I went ahead with deleting keys from source bucket for now as temporary fix. But we should revisit the `createFakeDirIfShould` logic and fix it there. This is the original PR: https://github.com/apache/ozone/pull/3774/files
--
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 #4230: HDDS-7861. Delete keys from the source bucket to fix the flakiness of snapshot restore tests.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4230:
URL: https://github.com/apache/ozone/pull/4230#discussion_r1093584820
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
Problem is that RocksDB's seek operation doesn't 100% guarantee that item key is available. Tho we believe that it is. When we seek if there is a directory for the key name, sometime it returns non-null response but in reality item doesn't exist.
[Here](https://github.com/apache/ozone/blob/d8765436c2bf76547973cc568f1648f1b1fe9fcb/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java#L1184), sometimes seek returns null and sometimes it return source bucket (which is wrong). When is returns source bucket, we create fake dir and return the response.
I was unsure what should be the correct behavior for this so I went ahead with deleting keys from source bucket for now as temporary fix. But we should revisit the `createFakeDirIfShould` logic and fix it there. This is the original PR: https://github.com/apache/ozone/pull/3774 and https://github.com/apache/ozone/pull/4038
--
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 #4230: HDDS-7861. Delete keys from the source bucket to fix the flakiness of snapshot restore tests.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4230:
URL: https://github.com/apache/ozone/pull/4230#discussion_r1093584820
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
Problem is that RocksDB's seek operation doesn't 100% guarantee that item key is available. Tho we believe that it is. When we check if there is a directory for the key name, sometime it returns non-null response but in reality item doesn't exist.
[Here](https://github.com/apache/ozone/blob/d8765436c2bf76547973cc568f1648f1b1fe9fcb/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java#L1184) sometimes seek returns null and sometimes it return source bucket (which is wrong). When is returns source bucket, we create fake dir and return the response.
I was unsure what should be the correct behavior for this so I went ahead to delete keys from source bucket for now as temporary fix. But we should revisit the `createFakeDirIfShould` logic and fix it there. This is the original PR: https://github.com/apache/ozone/pull/3774/files
--
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] slfan1989 commented on pull request #4230: HDDS-7845. Appended the slash to copy to bucket path to fix the flaky snapshot restore tests.
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #4230:
URL: https://github.com/apache/ozone/pull/4230#issuecomment-1411291965
+1 LGTM.
--
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 #4230: HDDS-7861. Delete keys from the source bucket to fix the flakiness of snapshot restore tests.
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4230:
URL: https://github.com/apache/ozone/pull/4230#discussion_r1093668318
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
fyi `createFakeDirIfShould` was brought in 2 months ago as a test flakiness fix: #4038
--
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] prashantpogde commented on a diff in pull request #4230: HDDS-7861. Delete keys from the source bucket to fix the flakiness of snapshot restore tests.
Posted by "prashantpogde (via GitHub)" <gi...@apache.org>.
prashantpogde commented on code in PR #4230:
URL: https://github.com/apache/ozone/pull/4230#discussion_r1093531615
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
Why do you need to delete the keys from the source bucket ?
--
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 merged pull request #4230: HDDS-7861. [Snapshot] Delete keys from the source bucket to fix the flakiness of snapshot restore tests
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl merged PR #4230:
URL: https://github.com/apache/ozone/pull/4230
--
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 #4230: HDDS-7861. Delete keys from the source bucket to fix the flakiness of snapshot restore tests.
Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4230:
URL: https://github.com/apache/ozone/pull/4230#discussion_r1093584820
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
Problem is that RocksDB's seek operation doesn't 100% guarantee that item key is available. Tho we believe that it is. When we check if there is a directory for the key name, sometime it returns non-null response but in reality item doesn't exist.
[Here](https://github.com/apache/ozone/blob/d8765436c2bf76547973cc568f1648f1b1fe9fcb/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java#L1184), sometimes seek returns null and sometimes it return source bucket (which is wrong). When is returns source bucket, we create fake dir and return the response.
I was unsure what should be the correct behavior for this so I went ahead to delete keys from source bucket for now as temporary fix. But we should revisit the `createFakeDirIfShould` logic and fix it there. This is the original PR: https://github.com/apache/ozone/pull/3774/files
--
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 #4230: HDDS-7861. Delete keys from the source bucket to fix the flakiness of snapshot restore tests.
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4230:
URL: https://github.com/apache/ozone/pull/4230#discussion_r1093664505
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOzoneSnapshotRestore.java:
##########
@@ -258,9 +259,24 @@ public void testRestoreSnapshotDifferentBucket(BucketLayout bucketLayoutTest)
int volBucketKeyCount = keyCount(buck, snapshotKeyPrefix + keyPrefix);
Assertions.assertEquals(5, volBucketKeyCount);
+ // Delete keys from the source bucket.
Review Comment:
> Problem is that RocksDB's seek operation doesn't 100% guarantee that item key is available. Tho we believe that it is. When we seek if there is a directory for the key name, sometime it returns non-null response but in reality item doesn't exist.
This could be a bug. When locating a key (e.g. checking existence), OM is responsible for verifying that the key name exactly matches the input right after an iterator seek. But I have yet to fully understand the intention of `createFakeDirIfShould` to make a call.
--
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