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.

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

crimson/osd: fixes and cleanups around multi-core OSD #52306

Merged
merged 3 commits into from
Jul 11, 2023
Merged

crimson/osd: fixes and cleanups around multi-core OSD #52306

merged 3 commits into from
Jul 11, 2023

Conversation

cyx1231st
Copy link
Member

@cyx1231st cyx1231st commented Jul 4, 2023 •
edited

Follow up #51147

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
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
  • jenkins test windows

@cyx1231st
crimson/osd: make sure OSD is constructing in the primary core
d6c5f05 
Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@cyx1231st
crimson/osd: avoid unnecessary foreign copy in ms_dispatch()
436706c 
Foreign-copy also implies that requests may be out-of-order, which is
incorrect in the data path.

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@cyx1231st
crimson/osd: cleanup ShardDispatcher
4ecc58d 
ShardDispatcher logically equals to seastar::sharded<PGShardManager>,
drop unnecessary members.

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
@cyx1231st cyx1231st requested a review from a team as a code owner July 4, 2023 02:50
@cyx1231st cyx1231st mentioned this pull request Jul 4, 2023
Merged
14 tasks
@cyx1231st
Copy link
Member Author

I'm able to run tests now.

Waiting for https://pulpito.ceph.com/yingxin-2023-07-04_09:04:25-crimson-rados-wip-yingxin-multi-core-crimson-osd-cleanup-1-distro-default-smithi/

@cyx1231st
Copy link
Member Author

The above test result looks good.

@cyx1231st
Copy link
Member Author

jenkins test api

@cyx1231st
Copy link
Member Author

jenkins test docs

@cyx1231st
Copy link
Member Author

Does it look good to you @Matan-B ?

@Matan-B
Copy link
Contributor

Matan-B commented Jul 9, 2023

Does it look good to you @Matan-B ?

Yes, once API tests will pass, we can merge this based on https://pulpito.ceph.com/yingxin-2023-07-04_09:04:25-crimson-rados-wip-yingxin-multi-core-crimson-osd-cleanup-1-distro-default-smithi/

@Matan-B
Copy link
Contributor

Matan-B commented Jul 9, 2023

jenkins test api

@Matan-B
Copy link
Contributor

Matan-B commented Jul 9, 2023

Unless I'm missing something, I think that the tests run on main (without the changes of this PR) by mistake:
https://github.com/ceph/ceph-ci/commits/wip-yingxin-multi-core-crimson-osd-cleanup-1
It looks like the only diff is a blank commit (f60eec2) which is the one used by the running jobs.

@cyx1231st
Copy link
Member Author

It looks like the only diff is a blank commit (f60eec2) which is the one used by the running jobs.

Ah, I was wondering why I need to add a blank commit to trigger a build ... it is indeed a wrong branch.

I'll test again, sorry about the mistake, and thanks to point it out!

@Matan-B
Copy link
Contributor

Matan-B commented Jul 10, 2023

Will be merging based on
https://pulpito.ceph.com/yingxin-2023-07-10_06:20:40-crimson-rados-wip-yingxin-multi-core-crimson-osd-cleanup-2-distro-default-smithi/

@Matan-B
Copy link
Contributor

Matan-B commented Jul 10, 2023

jenkins test api

@cyx1231st
Copy link
Member Author

Will be merging based on
https://pulpito.ceph.com/yingxin-2023-07-10_06:20:40-crimson-rados-wip-yingxin-multi-core-crimson-osd-cleanup-2-distro-default-smithi/

https://github.com/ceph/ceph-ci/commits/wip-yingxin-multi-core-crimson-osd-cleanup-2 should be correct now.

@cyx1231st
Copy link
Member Author

jenkins test api

@Matan-B Matan-B merged commit 914d0dd into ceph:main Jul 11, 2023
10 of 14 checks passed
@cyx1231st cyx1231st deleted the wip-seastar-multi-core-osd-cleanup branch July 11, 2023 07:15
@cyx1231st cyx1231st mentioned this pull request Aug 7, 2023
Merged
14 tasks
@Matan-B Matan-B mentioned this pull request Oct 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@athanatos athanatos athanatos approved these changes

@liu-chunmei liu-chunmei liu-chunmei approved these changes

@Matan-B Matan-B Awaiting requested review from Matan-B

Assignees
No one assigned
Labels
crimson ready-to-merge
Projects
None yet
Milestone
No milestone
4 participants
@cyx1231st @Matan-B @athanatos @liu-chunmei