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

radosgw-admin: fix for 'realm pull' #8404

Merged
merged 2 commits into from Apr 1, 2016
Merged

radosgw-admin: fix for 'realm pull' #8404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7567b45
rgw: add exclusive flag to RGWRealm::create_control
cbodley Mar 31, 2016
280b8a1
rgw: add exclusive flag to set_as_default()
cbodley Mar 31, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 22 additions & 37 deletions src/rgw/rgw_rados.cc
View file
Open in desktop
Expand Up @@ -350,7 +350,7 @@ int RGWZoneGroup::read_default_id(string& default_id, bool old_format)
return RGWSystemMetaObj::read_default_id(default_id, old_format);
}

int RGWZoneGroup::set_as_default()
int RGWZoneGroup::set_as_default(bool exclusive)
{
if (realm_id.empty()) {
/* try using default realm */
Expand All @@ -363,7 +363,7 @@ int RGWZoneGroup::set_as_default()
realm_id = realm.get_id();
}

return RGWSystemMetaObj::set_as_default();
return RGWSystemMetaObj::set_as_default(exclusive);
}

int RGWSystemMetaObj::init(CephContext *_cct, RGWRados *_store, bool setup_obj, bool old_format)
Expand Down Expand Up @@ -441,7 +441,7 @@ int RGWSystemMetaObj::use_default(bool old_format)
return read_default_id(id, old_format);
}

int RGWSystemMetaObj::set_as_default()
int RGWSystemMetaObj::set_as_default(bool exclusive)
{
string pool_name = get_pool_name(cct);
string oid = get_default_oid();
Expand All @@ -454,7 +454,8 @@ int RGWSystemMetaObj::set_as_default()

::encode(default_info, bl);

int ret = rgw_put_system_obj(store, pool, oid, bl.c_str(), bl.length(), false, NULL, real_time(), NULL);
int ret = rgw_put_system_obj(store, pool, oid, bl.c_str(), bl.length(),
exclusive, NULL, real_time(), NULL);
if (ret < 0)
return ret;

Expand Down Expand Up @@ -688,22 +689,13 @@ const string& RGWRealm::get_predefined_name(CephContext *cct) {

int RGWRealm::create(bool exclusive)
{
list<string> realms;
int ret = store->list_realms(realms);
if (ret < 0 && ret != -ENOENT) {
ldout(cct, 0) << "ERROR: listing realms, ret=" << ret << dendl;
return ret;
}

bool first_realm = realms.empty();

ret = RGWSystemMetaObj::create(exclusive);
int ret = RGWSystemMetaObj::create(exclusive);
if (ret < 0) {
ldout(cct, 0) << "ERROR creating new realm object " << name << ": " << cpp_strerror(-ret) << dendl;
return ret;
}
// create the control object for watch/notify
ret = create_control();
ret = create_control(exclusive);
if (ret < 0) {
ldout(cct, 0) << "ERROR creating control for new realm " << name << ": " << cpp_strerror(-ret) << dendl;
return ret;
Expand Down Expand Up @@ -733,12 +725,11 @@ int RGWRealm::create(bool exclusive)
ldout(cct, 0) << "ERROR: failed set current period " << current_period << dendl;
return ret;
}

if (first_realm) { /* this is racy, but it's fine */
ret = set_as_default();
if (ret < 0) {
ldout(cct, 0) << "WARNING: failed to set realm as default realm, ret=" << ret << dendl;
}
// try to set as default. may race with another create, so pass exclusive=true
// so we don't override an existing default
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oritwas how does this look?

Copy link
Member

Choose a reason for hiding this comment

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

great

ret = set_as_default(true);
Copy link
Member

Choose a reason for hiding this comment

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

The problem here is that we may race with another create operation. Can you add a comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passing exclusive=true makes set_as_default() an atomic operation, as I understand it. Two calls to RGWRealm::create() can go in parallel, but only one can successfully set_as_default().

Copy link
Member

Choose a reason for hiding this comment

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

True , but it could be the newer create realm command. It won't be deterministic. It will be the first one to execute set_as_default (or more correctly to execute the put_obj), better from the previous version but still racy

if (ret < 0 && ret != -EEXIST) {
ldout(cct, 0) << "WARNING: failed to set realm as default realm, ret=" << ret << dendl;
}

return 0;
Expand All @@ -753,12 +744,12 @@ int RGWRealm::delete_obj()
return delete_control();
}

int RGWRealm::create_control()
int RGWRealm::create_control(bool exclusive)
{
auto pool_name = get_pool_name(cct);
auto pool = rgw_bucket{pool_name.c_str()};
auto oid = get_control_oid();
return rgw_put_system_obj(store, pool, oid, nullptr, 0, true,
return rgw_put_system_obj(store, pool, oid, nullptr, 0, exclusive,
nullptr, real_time(), nullptr);
}

Expand Down Expand Up @@ -1485,15 +1476,9 @@ int RGWZoneParams::fix_pool_names()

int RGWZoneParams::create(bool exclusive)
{
list<string> zones;
int r = store->list_zones(zones);
if (r < 0) {
ldout(cct, 10) << "WARNING: store->list_zones() returned r=" << r << dendl;
}

/* check for old pools config */
rgw_obj obj(domain_root, avail_pools);
r = store->raw_obj_stat(obj, NULL, NULL, NULL, NULL, NULL, NULL);
int r = store->raw_obj_stat(obj, NULL, NULL, NULL, NULL, NULL, NULL);
if (r < 0) {
ldout(store->ctx(), 10) << "couldn't find old data placement pools config, setting up new ones for the zone" << dendl;
/* a new system, let's set new placement info */
Expand All @@ -1516,11 +1501,11 @@ int RGWZoneParams::create(bool exclusive)
return r;
}

if (zones.empty()) { /* first zone? maybe, it's a racy check */
r = set_as_default();
if (r < 0) {
ldout(cct, 10) << "WARNING: failed to set zone as default, r=" << r << dendl;
}
// try to set as default. may race with another create, so pass exclusive=true
// so we don't override an existing default
r = set_as_default(true);
Copy link
Member

Choose a reason for hiding this comment

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

add a comment here too

if (r < 0 && r != -EEXIST) {
ldout(cct, 10) << "WARNING: failed to set zone as default, r=" << r << dendl;
}

return 0;
Expand Down Expand Up @@ -1589,7 +1574,7 @@ int RGWZoneParams::read_default_id(string& default_id, bool old_format)
}


int RGWZoneParams::set_as_default()
int RGWZoneParams::set_as_default(bool exclusive)
{
if (realm_id.empty()) {
/* try using default realm */
Expand All @@ -1602,7 +1587,7 @@ int RGWZoneParams::set_as_default()
realm_id = realm.get_id();
}

return RGWSystemMetaObj::set_as_default();
return RGWSystemMetaObj::set_as_default(exclusive);
}

void RGWPeriodMap::encode(bufferlist& bl) const {
Expand Down
8 changes: 4 additions & 4 deletions src/rgw/rgw_rados.h
View file
Open in desktop
Expand Up @@ -783,7 +783,7 @@ class RGWSystemMetaObj {
}
int init(CephContext *_cct, RGWRados *_store, bool setup_obj = true, bool old_format = false);
virtual int read_default_id(string& default_id, bool old_format = false);
virtual int set_as_default();
virtual int set_as_default(bool exclusive = false);
int delete_default();
virtual int create(bool exclusive = true);
int delete_obj(bool old_format = false);
Expand Down Expand Up @@ -884,7 +884,7 @@ struct RGWZoneParams : RGWSystemMetaObj {
bool old_format = false);
using RGWSystemMetaObj::init;
int read_default_id(string& default_id, bool old_format = false);
int set_as_default();
int set_as_default(bool exclusive = false) override;
int create_default(bool old_format = false);
int create(bool exclusive = true);
int fix_pool_names();
Expand Down Expand Up @@ -1151,7 +1151,7 @@ struct RGWZoneGroup : public RGWSystemMetaObj {
}

int read_default_id(string& default_id, bool old_format = false);
int set_as_default();
int set_as_default(bool exclusive = false) override;
int create_default(bool old_format = false);
int equals(const string& other_zonegroup) const;
int add_zone(const RGWZoneParams& zone_params, bool *is_master, bool *read_only, const list<string>& endpoints);
Expand Down Expand Up @@ -1301,7 +1301,7 @@ class RGWRealm : public RGWSystemMetaObj
string current_period;
epoch_t epoch{0}; //< realm epoch, incremented for each new period

int create_control();
int create_control(bool exclusive);
int delete_control();
public:
RGWRealm() {}
Expand Down