Skip to content

rgw: Collapse Layers#41798

Closed
adamemerson wants to merge 20 commits intoceph:masterfrom
adamemerson:wip-rgw-layer-collapse
Closed

rgw: Collapse Layers#41798
adamemerson wants to merge 20 commits intoceph:masterfrom
adamemerson:wip-rgw-layer-collapse

Conversation

@adamemerson
Copy link
Contributor

@adamemerson adamemerson commented Jun 10, 2021

Wrappers of wrappers of wrappers introduce overhead, especially when virtualized and with allocating types like std::function.

Some of the abstractions are not well dimensioned and map almost 1:1 to RADOS in too low-level a way to be useful, as well as being superseded by zipper. This is especially the case for the MetaBE stuff.

The MetaBE especially obscures the code using it.

Collapse some layers together.

@adamemerson adamemerson force-pushed the wip-rgw-layer-collapse branch 3 times, most recently from 55f0114 to c0072d5 Compare June 10, 2021 02:52
mattbenjamin
mattbenjamin approved these changes Jun 10, 2021
View reviewed changes
Copy link
Contributor

@mattbenjamin mattbenjamin left a comment

Choose a reason for hiding this comment

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

my call stack thanks you!

@cbodley
Copy link
Contributor

cbodley commented Jun 10, 2021

cc @yehudasa

@yehudasa
Copy link
Member

yehudasa commented Jun 10, 2021

@adamemerson I'm not sure what's achieved here other than renaming stuff that was explicitly named with_rados so that we don't accidentally add new dependencies.

@adamemerson adamemerson force-pushed the wip-rgw-layer-collapse branch from e631751 to adb6621 Compare June 10, 2021 19:18
@cbodley cbodley mentioned this pull request Jun 11, 2021
Closed
8 tasks
@adamemerson adamemerson force-pushed the wip-rgw-layer-collapse branch 3 times, most recently from 6eae4bf to bd099b0 Compare June 18, 2021 21:25
@github-actions
Copy link

github-actions bot commented Jun 23, 2021

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

@adamemerson adamemerson force-pushed the wip-rgw-layer-collapse branch from 90798d5 to e5af6d8 Compare June 25, 2021 17:56
@github-actions
Copy link

github-actions bot commented Jun 28, 2021

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

@adamemerson
rgw: The finisher does not need to be wrapped in a 'service'
76918a6 
Since RGWSI_Notify is the only user, just move the Finisher there.

It'll get pulled out as part of the asyncening and then we'll just use
boost::asio::post.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@adamemerson
rgw: get_config_key properly belongs to Zipper
7f61407 
Since it's just a Mon call, move get_config_key into RadosStore, along
with support functions.

(Overall, RGWSI_RADOS's functionality should either be rolled into
RadosStore or, in the case of the more wrappery parts, teased apart
and untied.)

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@adamemerson
rgw: Collapse RGWSI_BucketIndex_RADOS into RGWSI_BucketIndex
e26b8ca 
A lot of stuff was accessing RGWSI_BucketIndex_Rados directly anyway,
so this wasn't really helping us any.

As all the users are either in RGWRados or call through RadosStore,
this properly belongs under RadosStore, but it only makes sense to do
that once we pull out its dependence on RGWSI_RADOS.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@adamemerson
rgw: Collapse RGWSI_User_RADOS into RGWSI_User
98e84f1 
Preparatory to getting rid of MetaBE.

Also rename datalog_rados to datalog, though it should be moved under
RadosStore, too.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@adamemerson
include/str_list: Everything can take string_view
416b660 
Rather than requiring `const std::string&`.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@adamemerson
rgw: marker_mgr::from_string can take string_view
2905dd6 
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@adamemerson
rgw: Disentangle BucketIndex from RGWSI_RADOS
c5bb755 
Also make a few changes like using std::vector for output instead of
std::list.

Both BucketIndex and BILog now move under Zipper once I figure out
what to do with their dependents.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@adamemerson adamemerson force-pushed the wip-rgw-layer-collapse branch from e5af6d8 to b93f988 Compare June 30, 2021 00:09
@github-actions
Copy link

github-actions bot commented Jul 2, 2021

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

@stale
Copy link

stale bot commented Jan 9, 2022

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.

@stale stale bot added the stale label Jan 9, 2022
@adamemerson adamemerson deleted the wip-rgw-layer-collapse branch September 15, 2022 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dang dang dang approved these changes

@mattbenjamin mattbenjamin mattbenjamin approved these changes

@cbodley cbodley Awaiting requested review from cbodley

Assignees

No one assigned

Labels

build/ops common needs-rebase rgw stale

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@adamemerson @cbodley @yehudasa @dang @mattbenjamin