You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smengcl (via GitHub)" <gi...@apache.org> on 2024/01/24 03:16:12 UTC
[PR] HDDS-9930. Remove open keys as well when keys are deleted from KeyTable [ozone]
smengcl opened a new pull request, #6079:
URL: https://github.com/apache/ozone/pull/6079
## What changes were proposed in this pull request?
In order to fix the undesired behavior of keys reappearing in the namespace when they are deleted after first being hsync()'ed (see HDDS-9930 description for an example), we need to remove open keys when keys are deleted from `KeyTable` so that they cannot be hsync()'ed again. This is achieved by checking OmKeyInfo's hsync metadata upon key deletion.
Note as a natural consequence of this change, the first client that does the initial `hsync()` can get OMException `KEY_NOT_FOUND` when trying to close/commit the file.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9930
## How was this patch tested?
- [x] Add test case `testHSyncDeletedKey` that covers single key deletion.
- [ ] Add more test cases to cover batch key deletion.
--
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
Re: [PR] HDDS-9930. Remove open keys as well when keys are deleted from KeyTable [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #6079:
URL: https://github.com/apache/ozone/pull/6079
--
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
Re: [PR] HDDS-9930. Remove open keys as well when keys are deleted from KeyTable [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #6079:
URL: https://github.com/apache/ozone/pull/6079#issuecomment-1912583945
There's some flakiness in TestHSync. I wasn't able to reproduce the test failure.
--
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
Re: [PR] HDDS-9930. Remove open keys as well when keys are deleted from KeyTable [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #6079:
URL: https://github.com/apache/ozone/pull/6079#issuecomment-1912793101
Thanks @jojochuang for reviewing and merging this.
--
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
Re: [PR] HDDS-9930. Remove open keys as well when keys are deleted from KeyTable [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #6079:
URL: https://github.com/apache/ozone/pull/6079#issuecomment-1909219120
Thanks @jojochuang for reviewing this.
> On an unrelated note, validateAndUpdateCache in OMKeyDeleteRequest and OMKeyDeleteRequestWithFSO are largely the same, there has to be a way to merge the code.
I did attempt to create a util method just like I did in the other PR but found there are too much variables needed to make it clean. Maybe I could put it in the super class.
--
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
Re: [PR] HDDS-9930. Remove open keys as well when keys are deleted from KeyTable [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #6079:
URL: https://github.com/apache/ozone/pull/6079#issuecomment-1912791070
Thanks @smengcl !
--
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
Re: [PR] HDDS-9930. Remove open keys as well when keys are deleted from KeyTable [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #6079:
URL: https://github.com/apache/ozone/pull/6079#discussion_r2072515629
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java:
##########
@@ -226,6 +227,46 @@ public void testOfsHSync() throws Exception {
}
}
+ @Test
+ public void testHSyncDeletedKey() throws Exception {
+ // Verify that a key can't be successfully hsync'ed again after it's deleted,
+ // and that key won't reappear after a failed hsync.
+
+ // Set the fs.defaultFS
+ final String rootPath = String.format("%s://%s/",
+ OZONE_OFS_URI_SCHEME, CONF.get(OZONE_OM_ADDRESS_KEY));
+ CONF.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath);
+
+ final String dir = OZONE_ROOT + bucket.getVolumeName()
+ + OZONE_URI_DELIMITER + bucket.getName();
+ final Path key1 = new Path(dir, "key-hsync-del");
+
+ try (FileSystem fs = FileSystem.get(CONF)) {
+ // Create key1
+ try (FSDataOutputStream os = fs.create(key1, true)) {
+ os.write(1);
+ os.hsync();
+ fs.delete(key1, false);
+
+ // getFileStatus should throw FNFE because the key is deleted
+ assertThrows(FileNotFoundException.class,
+ () -> fs.getFileStatus(key1));
+ // hsync should throw because the open key is gone
+ try {
+ os.hsync();
Review Comment:
Observed from the test log when I was doing https://github.com/apache/ozone/pull/7792,
the second `os.hsync` would be ignore with the [logic](https://github.com/apache/ozone/pull/6054/files#diff-db8e4914806b73e8ec37c15ffd146fc70bd1fcba631df9a314c7389af64a895dR377-R383) since there is not new write(`lastUpdatedBlockId == lastBLockId`).
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java:
##########
@@ -226,6 +227,46 @@ public void testOfsHSync() throws Exception {
}
}
+ @Test
+ public void testHSyncDeletedKey() throws Exception {
+ // Verify that a key can't be successfully hsync'ed again after it's deleted,
+ // and that key won't reappear after a failed hsync.
+
+ // Set the fs.defaultFS
+ final String rootPath = String.format("%s://%s/",
+ OZONE_OFS_URI_SCHEME, CONF.get(OZONE_OM_ADDRESS_KEY));
+ CONF.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath);
+
+ final String dir = OZONE_ROOT + bucket.getVolumeName()
+ + OZONE_URI_DELIMITER + bucket.getName();
+ final Path key1 = new Path(dir, "key-hsync-del");
+
+ try (FileSystem fs = FileSystem.get(CONF)) {
+ // Create key1
+ try (FSDataOutputStream os = fs.create(key1, true)) {
+ os.write(1);
+ os.hsync();
+ fs.delete(key1, false);
+
+ // getFileStatus should throw FNFE because the key is deleted
+ assertThrows(FileNotFoundException.class,
+ () -> fs.getFileStatus(key1));
+ // hsync should throw because the open key is gone
+ try {
+ os.hsync();
Review Comment:
cc @jojochuang
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java:
##########
@@ -226,6 +227,46 @@ public void testOfsHSync() throws Exception {
}
}
+ @Test
+ public void testHSyncDeletedKey() throws Exception {
+ // Verify that a key can't be successfully hsync'ed again after it's deleted,
+ // and that key won't reappear after a failed hsync.
+
+ // Set the fs.defaultFS
+ final String rootPath = String.format("%s://%s/",
+ OZONE_OFS_URI_SCHEME, CONF.get(OZONE_OM_ADDRESS_KEY));
+ CONF.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath);
+
+ final String dir = OZONE_ROOT + bucket.getVolumeName()
+ + OZONE_URI_DELIMITER + bucket.getName();
+ final Path key1 = new Path(dir, "key-hsync-del");
+
+ try (FileSystem fs = FileSystem.get(CONF)) {
+ // Create key1
+ try (FSDataOutputStream os = fs.create(key1, true)) {
+ os.write(1);
+ os.hsync();
+ fs.delete(key1, false);
+
+ // getFileStatus should throw FNFE because the key is deleted
+ assertThrows(FileNotFoundException.class,
+ () -> fs.getFileStatus(key1));
+ // hsync should throw because the open key is gone
+ try {
+ os.hsync();
Review Comment:
I don't think this would throw error as if there is not new block then the second `hsync` call would be ignore.
reference: https://github.com/apache/ozone/pull/6054/files#diff-db8e4914806b73e8ec37c15ffd146fc70bd1fcba631df9a314c7389af64a895dR377-R382
--
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