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

rgw: Add option to configure backend store #43054

Merged
merged 2 commits into from Sep 22, 2021
Merged

rgw: Add option to configure backend store #43054

merged 2 commits into from Sep 22, 2021

Conversation

soumyakoduri
Copy link
Contributor

@soumyakoduri soumyakoduri commented Sep 5, 2021 •
edited

Include a config option to be able to configure backend store type. Also till there is CLI available to configure options for dbstore, insert testID user during startup.

Signed-off-by: Soumya Koduri skoduri@redhat.com

dang
dang reviewed Sep 7, 2021
View reviewed changes
Copy link
Contributor

@dang dang left a comment

Choose a reason for hiding this comment

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

Otherwise, looks good.

src/common/options/rgw.yaml.in Outdated
- name: rgw_backend_store
type: str
level: advanced
desc: Backend store type
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that this is how we want to pick the store long term, so can we add to the description that this is experimental, so that we can easily change it to some other method if that's what we want?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@soumyakoduri soumyakoduri force-pushed the wip-skoduri-dbstore-vstart branch 4 times, most recently from dcc2a82 to 273fb58 Compare September 7, 2021 17:30
@cbodley
Copy link
Contributor

cbodley commented Sep 7, 2021

agreed with @dang - this looks fine for now, but we'll want to revisit it later:

  • we'll need to be able to describe layered stores, like a 'policy layer' on top of a 'cache layer' on top of a 'rados backend'
  • maybe this backend configuration belongs in multisite's zone configuration? if we have multiple radosgws in the same zone, does it ever make sense for them to use different backends?

specific to dbstore's use of the local filesystem, that means radosgw+dbstore isn't 'stateless' like it is with the rados store. since the rados backend is shared by all radosgws, we can start as many daemons as we want, and move them around between machines

because dbstore isn't stateless, the orchestrator won't be able to move them around between machines, and should avoid deploying more than one daemon on the same zone

@soumyakoduri soumyakoduri force-pushed the wip-skoduri-dbstore-vstart branch 2 times, most recently from 2db5725 to cdf72ff Compare September 8, 2021 11:25
soumyakoduri
soumyakoduri commented Sep 9, 2021
View reviewed changes
src/rgw/rgw_main.cc Outdated
@@ -364,6 +376,25 @@ int radosgw_Main(int argc, const char **argv)
derr << "Couldn't init storage provider (RADOS)" << dendl;
return EIO;
}

#ifdef WITH_RADOSGW_DBSTORE
if (config_store == "dbstore") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dang @cbodley .. this is additional change (temporary till dbstore CLIs are available) to create testid user on rgw startup on dbstore. Please let me know if these temp changes here are fine or is it better to add it in sal_dbstore*/dbstore* layer instead (probably while creating .db connection)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay with this.

Copy link
Contributor

Choose a reason for hiding this comment

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

sure. my only feedback here is that it doesn't need to be in rgw_main.cc; maybe StoreManager::init_storage_provider() in rgw_sal.cc is a better place for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@soumyakoduri soumyakoduri force-pushed the wip-skoduri-dbstore-vstart branch 3 times, most recently from 17d2d5b to 40480a9 Compare September 13, 2021 12:59
@dang dang mentioned this pull request Sep 15, 2021
Closed
8 tasks
@soumyakoduri
rgw: Add option to configure backend store
ebc9723 
Signed-off-by: Soumya Koduri <skoduri@redhat.com>
@soumyakoduri
rgw/dbstore: Insert testid user on startup
44b7a56 
Its temporary change to insert testid user while
starting up rgw server on dbstore.

Signed-off-by: Soumya Koduri <skoduri@redhat.com>
@soumyakoduri
Copy link
Contributor Author

https://pulpito.ceph.com/soumyakoduri-2021-09-20_16:27:01-rgw-wip-skoduri-dbstore-vstart-distro-basic-smithi

@dang dang merged commit a5ffc44 into ceph:master Sep 22, 2021
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 Awaiting requested review from mattbenjamin

@alimaredia alimaredia Awaiting requested review from alimaredia

@cbodley cbodley Awaiting requested review from cbodley

@kalebskeithley kalebskeithley Awaiting requested review from kalebskeithley

Assignees
No one assigned
Labels
common rgw
Projects
None yet
Milestone
No milestone
3 participants
@soumyakoduri @cbodley @dang