Skip to content

mds: optimization for large number of clients/opened files#43126

Closed
ukernel wants to merge 5 commits intoceph:mainfrom
ukernel:wip-mds-misc-optimize
Closed

mds: optimization for large number of clients/opened files#43126
ukernel wants to merge 5 commits intoceph:mainfrom
ukernel:wip-mds-misc-optimize

Conversation

@ukernel
Copy link
Contributor

@ukernel ukernel commented Sep 10, 2021

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@github-actions github-actions bot added the cephfs Ceph File System label Sep 10, 2021
@ukernel ukernel force-pushed the wip-mds-misc-optimize branch from e04f25f to c16897a Compare September 10, 2021 07:09
@jtlayton jtlayton requested a review from a team September 15, 2021 10:11
Yan, Zheng added 5 commits October 27, 2021 11:44
Signed-off-by: "Yan, Zheng" <yanzheng03@kuaishou.com>
Signed-off-by: "Yan, Zheng" <yanzheng03@kuaishou.com>
Signed-off-by: "Yan, Zheng" <yanzheng03@kuaishou.com>
Signed-off-by: "Yan, Zheng" <yanzheng03@kuaishou.com>
Signed-off-by: "Yan, Zheng" <yanzheng03@kuaishou.com>
@ukernel ukernel force-pushed the wip-mds-misc-optimize branch from c16897a to 88954de Compare October 27, 2021 03:45
@batrick batrick self-requested a review November 24, 2021 18:23
@vshankar vshankar self-assigned this Dec 2, 2021
Comment on lines +366 to +368
elist<Capability*>::item item_snaprealm_caps;
elist<Capability*>::item item_revoking_caps;
elist<Capability*>::item item_client_revoking_caps;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is so memory consuming in xlist<> over elist<> ?

Comment on lines +491 to +495
auto em = client_leases.emplace(std::piecewise_construct,
std::forward_as_tuple(client),
std::forward_as_tuple(this, session));
ClientLease *l = &em.first->second;
if (em.second) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use C++ Structured Binding to name the items returned by client_leases.emplace(...) rather than using .first and .second

Copy link
Contributor

Choose a reason for hiding this comment

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

Structured Bindings helps give friendly names to data items and further helps is maintaining continuity when reading and reviewing code instead of flipping back and forth to decipher what .first and .second is.

@djgalloway djgalloway changed the base branch from master to main July 1, 2022 00:00
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Aug 30, 2022
@github-actions
Copy link

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

@github-actions github-actions bot closed this Dec 22, 2022
@batrick
Copy link
Member

batrick commented Nov 27, 2023

@ukernel if you're willing, I'd like to see some of the changes merged. Can you rebase and split

mds: reduce Capability size by using elist::item
mds: put ClientLease in map container

into a separate PR. Those are particularly straight-forward.

Very sorry for letting this sit on the back-burner so long.

@github-actions
Copy link

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@github-actions github-actions bot removed the stale label Nov 27, 2023
@vshankar vshankar requested a review from a team November 28, 2023 13:47
@ukernel
Copy link
Contributor Author

ukernel commented Dec 22, 2023

@ukernel if you're willing, I'd like to see some of the changes merged. Can you rebase and split

mds: reduce Capability size by using elist::item mds: put ClientLease in map container

into a separate PR. Those are particularly straight-forward.

Very sorry for letting this sit on the back-burner so long.

the most valuable patch of this PR is "mds: optimize get_caps_{wanted,issued}() for large amount of caps". For the first two patches, you can ask some new guy to pick them up, and create a new PR

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Feb 20, 2024
@github-actions
Copy link

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

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.

4 participants