Skip to content

HDDS-9930. Remove open keys as well when keys are deleted from KeyTable #6079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jojochuang merged 3 commits into from
Jan 26, 2024
Merged

HDDS-9930. Remove open keys as well when keys are deleted from KeyTable #6079

jojochuang merged 3 commits into from
Jan 26, 2024

Conversation

@smengcl
Copy link
Contributor

@smengcl smengcl commented Jan 24, 2024 •
edited by jojochuang
Loading

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?

  • Add test case testHSyncDeletedKey that covers single key deletion.
  • Add more test cases to cover batch key deletion.

@smengcl smengcl added the bug Something isn't working label Jan 24, 2024
@smengcl smengcl requested a review from jojochuang January 24, 2024 03:16
@smengcl smengcl self-assigned this Jan 24, 2024
@smengcl smengcl added the hbase HBase on Ozone support label Jan 24, 2024
jojochuang
jojochuang reviewed Jan 24, 2024
View reviewed changes
Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you double check the test failure in TestHSync? Looks related.

jojochuang
jojochuang approved these changes Jan 24, 2024
View reviewed changes
Copy link
Contributor

@jojochuang jojochuang left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Let's check if the TestHSync is related though.

On an unrelated note, validateAndUpdateCache in OMKeyDeleteRequest and OMKeyDeleteRequestWithFSO are largely the same, there has to be a way to merge the code.

@smengcl
Copy link
Contributor Author

smengcl commented Jan 25, 2024

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.

@jojochuang
Copy link
Contributor

jojochuang commented Jan 26, 2024

There's some flakiness in TestHSync. I wasn't able to reproduce the test failure.

@jojochuang jojochuang merged commit 2651d4c into apache:HDDS-7593 Jan 26, 2024
@jojochuang
Copy link
Contributor

jojochuang commented Jan 26, 2024

Thanks @smengcl !

@smengcl
Copy link
Contributor Author

smengcl commented Jan 26, 2024

Thanks @jojochuang for reviewing and merging this.

smengcl added a commit to smengcl/hadoop-ozone that referenced this pull request Apr 3, 2024
@smengcl
HDDS-9930. Remove open keys as well when keys are deleted from KeyTab…
cda5a40 
…le (apache#6079)

(cherry picked from commit 2651d4c)

Conflicts:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponseWithFSO.java

Change-Id: I4d0ad7375cbe2d9b2bd795efeae0b35d6abcc35c
smengcl added a commit to smengcl/hadoop-ozone that referenced this pull request Apr 3, 2024
@smengcl
HDDS-9930. Remove open keys as well when keys are deleted from KeyTab…
6e6f5d7 
…le (apache#6079)

(cherry picked from commit 2651d4c)

Conflicts:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponseWithFSO.java
jojochuang pushed a commit that referenced this pull request Apr 3, 2024
@smengcl
HDDS-9930. Remove open keys as well when keys are deleted from KeyTab…
a38570a 
…le (#6079) (#6472)

(cherry picked from commit 2651d4c)

Conflicts:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponseWithFSO.java
@smengcl smengcl mentioned this pull request Apr 10, 2024
Merged
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request May 29, 2024
@smengcl
HDDS-9930. Remove open keys as well when keys are deleted from KeyTab…
96b5104 
…le (apache#6079) (apache#6472)

(cherry picked from commit 2651d4c)

Conflicts:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponseWithFSO.java

(cherry picked from commit a38570a)

Conflicts:
hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java
@peterxcli peterxcli mentioned this pull request Feb 2, 2025
Closed
peterxcli
peterxcli reviewed May 4, 2025
View reviewed changes
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java
() -> fs.getFileStatus(key1));
// hsync should throw because the open key is gone
try {
os.hsync();
Copy link
Member

@peterxcli peterxcli May 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@peterxcli peterxcli May 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Observed from the test log when I was doing #7792,
the second os.hsync would be ignore with the logic since there is not new write(lastUpdatedBlockId == lastBLockId).

Copy link
Member

@peterxcli peterxcli May 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @jojochuang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peterxcli peterxcli peterxcli left review comments

@jojochuang jojochuang jojochuang approved these changes

Assignees

@smengcl smengcl

Labels

bug Something isn't working hbase HBase on Ozone support

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@smengcl @jojochuang @peterxcli