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-9534. Support namespace summaries (du, dist & counts) for LEGACY buckets with file system disabled #5517

Merged
merged 19 commits into from
Mar 29, 2024

Conversation

ArafatKhan2198
Copy link
Contributor

@ArafatKhan2198 ArafatKhan2198 commented Oct 31, 2023

What changes were proposed in this pull request?

Recon NSSummaries currently support FSO, OBS buckets, and Legacy buckets with the FSEnabled flag set to true. When FSEnabled is false, Legacy buckets act like OBS buckets, requiring a similar approach for managing keys. We've adapted our OBS bucket handling methods for these cases, maximizing code reuse.

Here's a breakdown of the changes :-

  1. NSSummaryTaskWithLegacy.java: This includes methods to process keys in Legacy buckets, distinguishing between filesystem and object store layouts to generate NSSummaries accurately.
  2. BucketHandler.java: Depending on whether FileSystemPaths is enabled, we assign the appropriate handler for each bucket. For Legacy buckets with disabled FileSystemPaths, we employ OBS bucket handler, thanks to their identical key management approach.
  3. EntityHandler.java: This class is activated by the NSSummaryEndpoint and determines which handler to assign based on the path provided by the user. For Legacy buckets with the FileSystemPaths option disabled, we directly employ the OBSHandler and its associated methods. We have introduced specific normalization methods for OBS keys. OBS keys follow the format volumeName/bucketName/keyName, where the keyName can include any characters, even multiple slashes, such as “////KeyName.” There's no need to normalize the keyName itself, but it's essential to ensure the path up to the bucket is correctly normalized. This is because volumeName and bucketName must not contain multiple slashes or spaces. Our standard normalization methods for the FSO layout normalize the entire path, including the keyName, which could alter the keyName and lead to invalid output. To address this, I've added a method in the OmUtils.java class that normalizes the path up to the bucket level.

What is the link to the Apache JIRA

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

How was this patch tested?

Manual Testing and Unit Testing

@ArafatKhan2198
Copy link
Contributor Author

@devmadhuu @adoroszlai @GeorgeJahad Could you please take a look !!

Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

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

@ArafatKhan2198 Thanks for working over this, can add testcases for OBS Legacy type

@ArafatKhan2198
Copy link
Contributor Author

Thanks for the comments @sumitagrawl as informed I will have to merge this one first #4245 and then move on to incorporate the comments posted by you on this PR.

Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

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

@ArafatKhan2198 Plz check, given few minor comments

Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

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

LGTM+1

Copy link
Contributor

@dombizita dombizita left a comment

Choose a reason for hiding this comment

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

Thanks for the effort on this @ArafatKhan2198, it looks good to me, I have one minor comment.

@dombizita dombizita changed the title HDDS-9534 Support namespace summaries (du, dist & counts) for LEGACY buckets with file system disabled HDDS-9534. Support namespace summaries (du, dist & counts) for LEGACY buckets with file system disabled Mar 28, 2024
Copy link
Contributor

@devmadhuu devmadhuu 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 continue working on this patch. Just few Nits

// Add remaining segments as the key
if (segments.length > 2) {
normalizedPath.append("/").append(
String.join("/", Arrays.copyOfRange(segments, 2, segments.length)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
String.join("/", Arrays.copyOfRange(segments, 2, segments.length)));
String.join(OM_KEY_PREFIX, Arrays.copyOfRange(segments, 2, segments.length)));

@devmadhuu
Copy link
Contributor

@ArafatKhan2198 , you can ignore If (LOG.isDebugEnabled()) comments, as current logger library being used is already having check of If (LOG.isDebugEnabled()) , so you can fix rest of other comments.

Copy link
Contributor

@devmadhuu devmadhuu 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 . LGTM +1

@devmadhuu devmadhuu merged commit cb5d519 into apache:master Mar 29, 2024
38 checks passed
@devmadhuu
Copy link
Contributor

Thanks @dombizita @sumitagrawl for review.

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request May 29, 2024
… buckets with file system disabled (apache#5517)

(cherry picked from commit cb5d519)
swamirishi pushed a commit to swamirishi/ozone that referenced this pull request Jun 10, 2024
… for LEGACY buckets with file system disabled (apache#5517)

(cherry picked from commit cb5d519)
Change-Id: I6c05f74e99cd1628d0f8489f71f1f092b9ca896f
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Jul 17, 2024
… buckets with file system disabled (apache#5517)

(cherry picked from commit cb5d519)
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Jul 17, 2024
… buckets with file system disabled (apache#5517)

(cherry picked from commit cb5d519)
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Jul 17, 2024
… buckets with file system disabled (apache#5517)

(cherry picked from commit cb5d519)
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Jul 18, 2024
… buckets with file system disabled (apache#5517)

(cherry picked from commit cb5d519)
xichen01 pushed a commit to xichen01/ozone that referenced this pull request Jul 18, 2024
… buckets with file system disabled (apache#5517)

(cherry picked from commit cb5d519)
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.

5 participants