Skip to content

HDDS-6278 Improve memory profile for listStatus API call. #3053

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
adoroszlai merged 2 commits into from
Feb 10, 2022
Merged

HDDS-6278 Improve memory profile for listStatus API call. #3053

adoroszlai merged 2 commits into from
Feb 10, 2022

Conversation

@kerneltime
Copy link
Contributor

@kerneltime kerneltime commented Feb 8, 2022 •
edited
Loading

What changes were proposed in this pull request?

Remove deleted cache

What is the link to the Apache JIRA

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

How was this patch tested?

Unit tests in CI test should exercise this code.

@sodonnel
Copy link
Contributor

sodonnel commented Feb 8, 2022

The original Jira is already merged and closed. We probably should attach this change to a new Jira "Improve memory profile for listStatus API call for FSO buckets" or something like that.

@kerneltime
Copy link
Contributor Author

kerneltime commented Feb 8, 2022

The original Jira is already merged and closed. We probably should attach this change to a new Jira "Improve memory profile for listStatus API call for FSO buckets" or something like that.

I did.. I just copied the wrong URL for the PR. The commit message links to the right one https://issues.apache.org/jira/browse/HDDS-6278
Fixed it here as well.

@kerneltime
Copy link
Contributor Author

kerneltime commented Feb 8, 2022

TestOzoneFileSystem passed locally for me.. not sure why CI timed out.

@kerneltime
Copy link
Contributor Author

kerneltime commented Feb 9, 2022

CI failure seems unrelated

Errors during downloading metadata for repository 'AppStream':
[660](https://github.com/apache/ozone/runs/5117443339?check_suite_focus=true#step:5:660)
  - Curl error (28): Timeout was reached for https://vault.centos.org/centos/8/AppStream/x86_64/os/repodata/d8472d61c5e53a3e9cbffb68e0dddbd04a07c2b7d864b07ddd211c6ad1380c6e-primary.xml.gz [Connection timed out after 30932 milliseconds]
[661](https://github.com/apache/ozone/runs/5117443339?check_suite_focus=true#step:5:661)
  - Curl error (28): Timeout was reached for http://vault.centos.org/centos/8/AppStream/x86_64/os/repodata/d8472d61c5e53a3e9cbffb68e0dddbd04a07c2b7d864b07ddd211c6ad1380c6e-primary.xml.gz [Operation too slow. Less than 1000 bytes/sec transferred the last 30 seconds]
[662](https://github.com/apache/ozone/runs/5117443339?check_suite_focus=true#step:5:662)
  - Curl error (28): Timeout was reached for http://vault.centos.org/centos/8/AppStream/x86_64/os/repodata/768e088faaaba73d00aee49f134e22d5d1803171ffb167260c8b55f4165e0372-filelists.xml.gz [Operation too slow. Less than 1000 bytes/sec transferred the last 30 seconds]
[663](https://github.com/apache/ozone/runs/5117443339?check_suite_focus=true#step:5:663)
  - Curl error (28): Timeout was reached for http://vault.centos.org/centos/8/AppStream/x86_64/os/repodata/5ea46cc5dfdd4a6f9c181ef29daa4a386e7843080cd625843267860d444df2f3-comps-AppStream.x86_64.xml [Operation too slow. Less than 1000 bytes/sec transferred the last 30 seconds]
[664](https://github.com/apache/ozone/runs/5117443339?check_suite_focus=true#step:5:664)
Error: Failed to download metadata for repo 'AppStream': Yum repo downloading error: Downloading error(s): repodata/d8472d61c5e53a3e9cbffb68e0dddbd04a07c2b7d864b07ddd211c6ad1380c6e-primary.xml.gz - Cannot download, all mirrors were already tried without success; repodata/768e088faaaba73d00aee49f134e22d5d1803171ffb167260c8b55f4165e0372-filelists.xml.gz - Cannot download, all mirrors were already tried without success; repodata/5ea46cc5dfdd4a6f9c181ef29daa4a386e7843080cd625843267860d444df2f3-comps-AppStream.x86_64.xml - Cannot download, all mirrors were already tried without success; repodata/75e00d390273f65b94d63a5ac7db2677d832bfa95b3d7eecf651c6f45e33f8ff-modules.yaml.xz - Cannot download, all mirrors were already tried without success
[665](https://github.com/apache/ozone/runs/5117443339?check_suite_focus=true#step:5:665)
ERROR: Test execution of ozonesecure-mr is FAILED!!!!```

rakeshadr
rakeshadr reviewed Feb 9, 2022
View reviewed changes
Copy link
Contributor

@rakeshadr rakeshadr left a comment

Choose a reason for hiding this comment

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

Thanks @kerneltime for the contribution. Added few improvements, please go through it.

@rakeshadr
Copy link
Contributor

rakeshadr commented Feb 9, 2022 •
edited
Loading

@kerneltime
We need to handle deletedKeySet usage in KeyManagerImpl#listStatusFSO() logic as well.

In FSO, the number of entries in the cache will be pretty less compares to OBS and the overall memory foot print in FSO will be comparatively lesser than OBS. Because in OBS a single OMKeysDeleteRequest contains a bunch of keys, but in FSO OMKeyDeleteRequest will have only one key(file/directory) entry and it is optimized one.

KeyManagerImpl.java#L1668

@adoroszlai
Copy link
Contributor

adoroszlai commented Feb 9, 2022

CI failure seems unrelated

  - Curl error (28): Timeout was reached for https://vault.centos.org/centos/8/AppStream/x86_64/os/repodata/d8472d61c5e53a3e9cbffb68e0dddbd04a07c2b7d864b07ddd211c6ad1380c6e-primary.xml.gz
...
ERROR: Test execution of ozonesecure-mr is FAILED!!!!```

Unrelated indeed, being fixed in #3057.

@kerneltime
Copy link
Contributor Author

kerneltime commented Feb 9, 2022

@kerneltime

We need to handle deletedKeySet usage in KeyManagerImpl#listStatusFSO() logic as well.

In FSO, the number of entries in the cache will be pretty less compares to OBS and the overall memory foot print in FSO will be comparatively lesser than OBS. Because in OBS a single OMKeysDeleteRequest contains a bunch of keys, but in FSO OMKeyDeleteRequest will have only one key(file/directory) entry and it is optimized one.

KeyManagerImpl.java#L1668

Yes I saw that and have a patch but I want to keep this one simple and limited to one API. Will file a separate PR. I want to go over FSO logic in detail.

@kerneltime
Copy link
Contributor Author

kerneltime commented Feb 9, 2022

@adoroszlai this should be good to merge

@kerneltime @adoroszlai
Update hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozo…
77fb113 
…ne/om/KeyManagerImpl.java

Co-authored-by: Doroszlai, Attila <[email protected]>
@adoroszlai adoroszlai requested a review from sodonnel February 10, 2022 14:56
@adoroszlai adoroszlai merged commit a5accd2 into apache:master Feb 10, 2022
@adoroszlai
Copy link
Contributor

adoroszlai commented Feb 10, 2022

Thanks @kerneltime for the patch, @rakeshadr and @sodonnel for the review.

@errose28 errose28 mentioned this pull request Mar 11, 2022
Closed
errose28 added a commit to errose28/ozone that referenced this pull request Mar 30, 2022
@errose28
Merge branch 'HDDS-4944' into HDDS-5836
9bf4814 
* HDDS-4944: (268 commits)
  HDDS-6366. [Multi-Tenant] Disallow specifying custom accessId in OzoneManager (apache#3166)
  HDDS-6275. [Multi-Tenant] Add feature documentation and CLI quick start guide (apache#3095)
  HDDS-6063. [Multi-Tenant] Use VOLUME_LOCK in read and write requests, and some minor refactoring (apache#3051)
  HDDS-6214. [Multi-Tenant] Fix KMS Encryption/Decryption (apache#3010)
  HDDS-6322. Fix Recon getting inccorrect sequenceNumber from OM (apache#3090)
  HDDS-5913. Avoid integer overflow when setting dfs.container.ratis.lo… (apache#2785)
  HDDS-6313. Remove replicas in ContainerStateMap when a container is deleted (apache#3086)
  HDDS-6186. Selective checks: skip integration check for unit test changes (apache#3061)
  HDDS-6310. Update json-smart to 2.4.7. (apache#3080)
  HDDS-6190. Cleanup unnecessary datanode id path checks. (apache#2993)
  HDDS-6304. Add enforcer to make sure ozone.version equals project.version (apache#3075)
  HDDS-6309. Update ozone-runner version to 20220212-1 (apache#3079)
  HDDS-6293. Allow using custom ozone-runner image (apache#3072)
  HDDS-4126. Freon key generator should support >2GB files. (apache#3054)
  HDDS-6088. Implement O3FS/OFS getFileChecksum() using file checksum helpers - addendum: fix checkstyle
  HDDS-6088. Implement O3FS/OFS getFileChecksum() using file checksum helpers. (apache#2935)
  HDDS-6084. [Multi-Tenant] Handle upgrades to version supporting S3 multi-tenancy (apache#3018)
  HDDS-6257. Wrong stack trace for S3 errors (apache#3066)
  HDDS-6278 Improve memory profile for listStatus API call. (apache#3053)
  HDDS-6285. ozonesecure-mr intermittently failing with timeout downloading packages (apache#3057)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adoroszlai adoroszlai adoroszlai left review comments

@rakeshadr rakeshadr rakeshadr left review comments

@sodonnel sodonnel sodonnel approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kerneltime @sodonnel @rakeshadr @adoroszlai