Skip to content

HDDS-13982. OmKeyInfo fileName possible inconsistency between cache and DB#9381

Merged
adoroszlai merged 6 commits into
apache:masterfrom
adoroszlai:HDDS-13982
Dec 1, 2025
Merged

HDDS-13982. OmKeyInfo fileName possible inconsistency between cache and DB#9381
adoroszlai merged 6 commits into
apache:masterfrom
adoroszlai:HDDS-13982

Conversation

@adoroszlai

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

OmKeyInfo#fileName is not persisted to DB, but set during deserialization based on keyName. This happens even if the OmKeyInfo did not have fileName initially. Thus the fileName may be different for the instance added to the table cache and the one read from DB. To avoid that, FSO requests need to manually setFileName, which is error-prone.

This PR changes OmKeyInfo to set fileName whenever keyName is initialized (in build()) or updated.

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

How was this patch tested?

Updated unit and integration test case.

https://github.com/adoroszlai/ozone/actions/runs/19699233660

@adoroszlai adoroszlai self-assigned this Nov 26, 2025
@adoroszlai adoroszlai added the om label Nov 26, 2025
@adoroszlai adoroszlai changed the title HDDS-13982. OmKeyInfo fileName inconsistency between cache and DB HDDS-13982. OmKeyInfo fileName possible inconsistency between cache and DB Nov 26, 2025
openKeyToDelete.setModificationTime(Time.now());
OMFileRequest.addOpenFileTableCacheEntry(omMetadataManager,
dbOpenKeyToDeleteKey, openKeyToDelete, keyName, fileName, trxnLogIndex);
dbOpenKeyToDeleteKey, openKeyToDelete, fileName, trxnLogIndex);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ChenSammi I believe this is a bug in hsync: order of keyName and fileName is mixed up.

Parameters:

public static void addOpenFileTableCacheEntry(
OMMetadataManager omMetadataManager, String dbOpenFileName,
@Nullable OmKeyInfo omFileInfo, String fileName, String keyName, long trxnLogIndex) {

Arguments:

OMFileRequest.addOpenFileTableCacheEntry(omMetadataManager,
dbOpenKeyToDeleteKey, openKeyToDelete, keyName, fileName, trxnLogIndex);

Should we be passing keyName instead of fileName?

@ChenSammi ChenSammi Dec 1, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@adoroszlai , good catch! Yeah, keyName and fileName was accidentally passed in with a reversed order.

@adoroszlai

Copy link
Copy Markdown
Contributor Author

@smengcl could you please review?

@chungen0126

Copy link
Copy Markdown
Contributor

Thanks @adoroszlai for the fix.

Would It be better to create an OmKeyInfoWithFSO subclass that extends OmKeyInfo for FSO buckets. We could then move the fileName-related methods (e.g., getFileName & getPath) to OmKeyInfoWithFSO.

This approach would prevent from calling these methods during OBS operations.

@ChenSammi ChenSammi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @adoroszlai for this improvement. The last patch LGTM.

@adoroszlai adoroszlai marked this pull request as draft December 1, 2025 09:52
@adoroszlai adoroszlai marked this pull request as ready for review December 1, 2025 11:33
@adoroszlai adoroszlai merged commit 75e7ca6 into apache:master Dec 1, 2025
95 of 96 checks passed
@adoroszlai adoroszlai deleted the HDDS-13982 branch December 1, 2025 14:58
@adoroszlai

Copy link
Copy Markdown
Contributor Author

Thanks @ChenSammi, @chungen0126 for the review.

Would It be better to create an OmKeyInfoWithFSO subclass that extends OmKeyInfo for FSO buckets. We could then move the fileName-related methods (e.g., getFileName & getPath) to OmKeyInfoWithFSO.

I'll give it a try in a follow-up.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants