Skip to content

crimson/osd: make the ObjectContextRegistry per-PG#49729

Merged
athanatos merged 1 commit intoceph:mainfrom
rzarzynski:wip-crimson-obcregistry-per-pg
Feb 14, 2023
Merged

crimson/osd: make the ObjectContextRegistry per-PG#49729
athanatos merged 1 commit intoceph:mainfrom
rzarzynski:wip-crimson-obcregistry-per-pg

Conversation

@rzarzynski
Copy link
Contributor

@rzarzynski rzarzynski commented Jan 12, 2023

This patch moves the OBC registry from ShardServices (which is basicaly a gateway to a bunch of PGs) into PG itself. Dividing OBCs by PG (they truly belong to) minimizes the space to search when e.g. checking whether a client is blocked or not. Therefore, this commit is enabler of more performant PR #47637.

The changeset draws the assumption that OBC registry and all its users live on the same CPU core. It looks valid to me.

Signed-off-by: Radoslaw Zarzynski rzarzyns@redhat.com

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

@rzarzynski rzarzynski requested a review from a team as a code owner January 12, 2023 18:50
@rzarzynski rzarzynski mentioned this pull request Jan 12, 2023
Merged
14 tasks
@Matan-B
Copy link
Contributor

Matan-B commented Jan 15, 2023

make check failing with:

/home/jenkins-build/build/workspace/ceph-pull-requests/src/fmt/include/fmt/core.h:1756:3: error: static_assert failed due to requirement 'formattable' "Cannot format an argument. To make type T formattable provide a formatter<T> specialization: https://fmt.dev/latest/api.html#udt"
  static_assert(

Including "osd/osd_types_fmt.h" in the loader should help.

@rzarzynski
Copy link
Contributor Author

rzarzynski commented Jan 15, 2023

Thanks, Matan!

@rzarzynski rzarzynski force-pushed the wip-crimson-obcregistry-per-pg branch from 9acf5d3 to 628c588 Compare January 15, 2023 12:02
liu-chunmei
liu-chunmei approved these changes Jan 19, 2023
View reviewed changes
Copy link
Contributor

@liu-chunmei liu-chunmei left a comment

Choose a reason for hiding this comment

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

LGTM.

@liu-chunmei
Copy link
Contributor

liu-chunmei commented Jan 19, 2023

jenkins test make check

@Matan-B Matan-B mentioned this pull request Jan 23, 2023
Merged
20 tasks
@github-actions
Copy link

github-actions bot commented Feb 2, 2023

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

@rzarzynski rzarzynski force-pushed the wip-crimson-obcregistry-per-pg branch from 628c588 to ea6bdd4 Compare February 7, 2023 16:07
Matan-B
Matan-B reviewed Feb 8, 2023
View reviewed changes
@rzarzynski
crimson/osd: make the ObjectContextRegistry per-PG
59c8ee4 
This patch moves the OBC registry from ShardServices (which
is basicaly a gateway to a bunch of PGs) into PG itself.
Dividing OBCs by PG (they truly belong to) minimizes the space
to search when e.g. checking whether a client is blocked or
not. Therefore, this commit is enabler of more performant
PR ceph#47637.

The changeset draws the assumption that OBC registry and all
its users live on the same CPU core. It looks valid to me.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
@rzarzynski rzarzynski force-pushed the wip-crimson-obcregistry-per-pg branch from ea6bdd4 to 59c8ee4 Compare February 9, 2023 17:11
@Matan-B
Copy link
Contributor

Matan-B commented Feb 13, 2023

Test results LGTM
http://pulpito.front.sepia.ceph.com/matan-2023-02-12_11:21:05-crimson-rados-wip-matanb-crimson-only-testing-12.2-distro-crimson-smithi/

4f02cb2d3f6 Merge branch 'wip-crimson-obcregistry-per-pg' into wip-matanb-crimson-only-testing-12.2                                                   
24c60d130cd Merge branch 'wip-sync-peering-backfill' into wip-matanb-crimson-only-testing-12.2                         
e9c09fdf42d Merge branch 'wip-pg-remove' into wip-matanb-crimson-only-testing-12.2                                     
4d1f67b1b6c Merge branch 'wip-obc-watchers-checkonload' into wip-matanb-crimson-only-testing-12.2                      
c014aa5ee0f Merge branch 'wip-pg-stats-read' into wip-matanb-crimson-only-testing-12.2                                 
74179c8ba76 Merge branch 'wip-matanb-c-pg-pipeline' into wip-matanb-crimson-only-testing-12.2                          
02b59b3ed3f Merge branch 'wip-crimson-fix-zeroed-startevent' into wip-matanb-crimson-only-testing-12.2

@athanatos
Copy link
Contributor

athanatos commented Feb 13, 2023

jenkins test api

@athanatos athanatos merged commit 73d4a7e into ceph:main Feb 14, 2023
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 Matan-B approved these changes

Assignees

No one assigned

Labels

crimson wip-matan-testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rzarzynski @Matan-B @liu-chunmei @athanatos