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-8371. Avoid concatenating unexpected a full path and prefix #4530

Closed
wants to merge 1 commit into from

Conversation

k5342
Copy link
Contributor

@k5342 k5342 commented Apr 4, 2023

What changes were proposed in this pull request?

This is a workaround for HDDS-8371 when we have a dirty OMKeyInfo entry in the keyTable. The dirty in this context means keyName includes a prefix to the key in the keyName. As the listStatus API assumes non-prefixed keys to concatenate keys, we eventually get a wrong result from listStatus.

As a workaround, we can use fileName instead of keyName. I noticed keyInfo#fileName is not dirty in this case, which means does not include the prefix.

I think this is a short-term fix for this problem. But I'm not confident about how this change is valuable because it is something ad-hoc. The background is described as the above and Jira, so could you give me some comments?

For the root cause, I don't make sure whether this is from persistent information or dynamically generated information, but it seems OMKeyInfo#fromPersistedFormat() returns the dirty result. We need to fix the root cause later by reviewing the write path for keys.

What is the link to the Apache JIRA

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

How was this patch tested?

  • mvn test locally
  • Unit Test triggered by CI

@ashishkumar50
Copy link
Contributor

@k5342 , The main reason for this is caused by cache which is used by ListStatusHelper causing corruption. You can view the PR below for actual fix.
#4531

@kerneltime kerneltime self-requested a review April 10, 2023 16:25
String fullKeyPath = OMFileRequest.getAbsolutePath(prefixPath,
keyInfo.getKeyName());
keyInfo.getFileName());
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of doing this, I think it's better to have tool which should correct metadata. This will just avoid one problem but metadata will remain in corrupt state.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can refer tools like #4337.

@adoroszlai
Copy link
Contributor

@k5342 I assume after HDDS-8292 this workaround is no longer needed. HDDS-8371 can be the task to create a tool to fix entries already corrupted, if necessary.

@adoroszlai adoroszlai closed this Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants