Skip to content
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.

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

HDDS-10324. Metadata are not updated when keys are overwritten. #6273

Merged
merged 5 commits into from
Feb 28, 2024

Conversation

ArafatKhan2198
Copy link
Contributor

@ArafatKhan2198 ArafatKhan2198 commented Feb 26, 2024

What changes were proposed in this pull request?

A bug was identified where updating an existing key with new metadata does not lead to the expected update of the metadata; instead, the system retains the old metadata. These keys were created using the S3 CLI. To simulate this problem please take a look at the JIRA description.

Root Cause :-

When we create an object with the same key name as one already in the database, the request is forwarded to the OM side of the code, specifically to the OmKeyRequest class, which contains a method called prepareFileInfo(). This method persists the data to the openKeyTable. Initially, it checks if a key with the same name exists. If it does, it proceeds to update it with new data size, modification time, updateID, and replicationConfig. However, it fails to update the metadata of the overridden file. Consequently, the old metadata stored earlier is retained, which is the root cause of the problem and requires fixing.

Changes made :-

  1. Extracting the new metadata from the KeyArgs object.
  2. Comparing the new metadata with the existing metadata in the OmKeyInfo object.
  3. Updating the OmKeyInfo object with any new or modified metadata entries.
  4. Ensuring that metadata entries not mentioned in the overwrite operation are retained as is.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10324

How was this patch tested?

Tested it out Manually & Tested it out with UT's for the below two scenarios :-

  1. Test Metadata Is Correctly Updated: Wrote tests to verify that when a key is created with metadata, and then overwritten with new or updated metadata, the metadata is correctly updated in the OmKeyInfo object that would be persisted. This involves:
  2. Initial creation without metadata, followed by an overwrite that includes metadata.

@ivandika3
Copy link
Contributor

@ArafatKhan2198 Will the bug also causes ETag field to not be updated when overwriting an object with another object with different content?

@ArafatKhan2198
Copy link
Contributor Author

@ArafatKhan2198 Will the bug also causes ETag field to not be updated when overwriting an object with another object with different content?

@ivandika3
The ETag serves as a unique identifier for the content of a file at a given time. When you upload a file to S3 with a specific key, S3 generates an ETag based on the content of that file. If you upload the same file again (with the exact same content and under the same key), the ETag remains the same because the content has not changed.

If you modify the contents of the file, the ETag will change accordingly. I have tested this scenario, and when uploading a file with the same key name but different content, the ETag also changes.

@ArafatKhan2198
Copy link
Contributor Author

@ChenSammi @sumitagrawl Could you please take a look!

@adoroszlai
Copy link
Contributor

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
 26: Using the '.*' form of import should be avoided - java.util.*.
 35: Unused import - org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.

https://github.com/ArafatKhan2198/ozone/actions/runs/8045754649/job/21971634453

@ArafatKhan2198 ArafatKhan2198 marked this pull request as ready for review February 26, 2024 20:58
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @ArafatKhan2198 for the patch.

Copy link
Contributor

@ChenSammi ChenSammi left a comment

Choose a reason for hiding this comment

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

The last patch looks good. Thanks @ArafatKhan2198 .

@adoroszlai adoroszlai merged commit aa68aec into apache:master Feb 28, 2024
36 checks passed
@adoroszlai
Copy link
Contributor

Thanks @ArafatKhan2198 for the patch, @ChenSammi for the review.

adoroszlai pushed a commit to adoroszlai/ozone that referenced this pull request Mar 5, 2024
jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Mar 15, 2024
…he#6273)

(cherry picked from commit aa68aec)
Change-Id: I6cc6d24aebc603fd8215401b2ea41b5528fea07d
swamirishi pushed a commit to swamirishi/ozone that referenced this pull request Jun 10, 2024
…tten. apache#6273

(cherry picked from commit aa68aec)
Change-Id: I05e802a9580565cbd10e9c1f59c9228b4527b44e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants