From 0797be3f86df8b413256d69e3770ec39ed6e6912 Mon Sep 17 00:00:00 2001 From: Yehuda Sadeh Date: Thu, 13 Dec 2012 15:52:34 -0800 Subject: [PATCH] rgw: key indexes are only link to user info Instead of keeping multiple copies of the user info, we just treat the key index as a pointer to the actual user info (indexed by uid). This helps with two issues: first, it scales better as we don't need to update the entire set of keys whenever we make any change. Second, it helps with the uid index atomicity. One point to keep in mind is that both the links and the info can be cached, so effect on performance is minimal. Signed-off-by: Yehuda Sadeh Reviewed-by: Sage Weil Reviewed-by: caleb miles --- src/rgw/rgw_admin.cc | 8 +++--- src/rgw/rgw_swift.cc | 2 +- src/rgw/rgw_user.cc | 64 +++++++++++++++++++++++++++++++++++--------- src/rgw/rgw_user.h | 2 +- 4 files changed, 57 insertions(+), 19 deletions(-) diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index 41b8ec5f1b427..860a4d19eba6f 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -1082,7 +1082,7 @@ int main(int argc, char **argv) info.subusers[subuser] = u; } - if ((err = rgw_store_user_info(store, info, false)) < 0) { + if ((err = rgw_store_user_info(store, info, &old_info, false)) < 0) { cerr << "error storing user info: " << cpp_strerror(-err) << std::endl; break; } @@ -1108,7 +1108,7 @@ int main(int argc, char **argv) keys_map->erase(kiter); } } - if ((err = rgw_store_user_info(store, info, false)) < 0) { + if ((err = rgw_store_user_info(store, info, &old_info, false)) < 0) { cerr << "error storing user info: " << cpp_strerror(-err) << std::endl; break; } @@ -1134,7 +1134,7 @@ int main(int argc, char **argv) } else { rgw_remove_key_index(store, kiter->second); keys_map->erase(kiter); - if ((err = rgw_store_user_info(store, info, false)) < 0) { + if ((err = rgw_store_user_info(store, info, &old_info, false)) < 0) { cerr << "error storing user info: " << cpp_strerror(-err) << std::endl; break; } @@ -1529,7 +1529,7 @@ next: int ret; info.suspended = disable; - ret = rgw_store_user_info(store, info, false); + ret = rgw_store_user_info(store, info, &old_info, false); if (ret < 0) { cerr << "ERROR: failed to store user info user=" << user_id << " ret=" << ret << std::endl; return 1; diff --git a/src/rgw/rgw_swift.cc b/src/rgw/rgw_swift.cc index 9bb7cecad8211..58d93fcead498 100644 --- a/src/rgw/rgw_swift.cc +++ b/src/rgw/rgw_swift.cc @@ -511,7 +511,7 @@ int RGWSwift::update_user_info(RGWRados *store, struct rgw_swift_auth_info *info user_info.user_id = info->user; user_info.display_name = info->display_name; - int ret = rgw_store_user_info(store, user_info, true); + int ret = rgw_store_user_info(store, user_info, NULL, true); if (ret < 0) { ldout(cct, 0) << "ERROR: failed to store new user's info: ret=" << ret << dendl; return ret; diff --git a/src/rgw/rgw_user.cc b/src/rgw/rgw_user.cc index 68c91534e04de..f05f594d321e9 100644 --- a/src/rgw/rgw_user.cc +++ b/src/rgw/rgw_user.cc @@ -34,7 +34,7 @@ bool rgw_user_is_authenticated(RGWUserInfo& info) * Save the given user information to storage. * Returns: 0 on success, -ERR# on failure. */ -int rgw_store_user_info(RGWRados *store, RGWUserInfo& info, bool exclusive) +int rgw_store_user_info(RGWRados *store, RGWUserInfo& info, RGWUserInfo *old_info, bool exclusive) { bufferlist bl; info.encode(bl); @@ -44,6 +44,8 @@ int rgw_store_user_info(RGWRados *store, RGWUserInfo& info, bool exclusive) map::iterator iter; for (iter = info.swift_keys.begin(); iter != info.swift_keys.end(); ++iter) { + if (old_info && old_info->swift_keys.count(iter->first) != 0) + continue; RGWAccessKey& k = iter->second; /* check if swift mapping exists */ RGWUserInfo inf; @@ -60,6 +62,8 @@ int rgw_store_user_info(RGWRados *store, RGWUserInfo& info, bool exclusive) map::iterator iter = info.access_keys.begin(); for (; iter != info.access_keys.end(); ++iter) { RGWAccessKey& k = iter->second; + if (old_info && old_info->access_keys.count(iter->first) != 0) + continue; int r = rgw_get_user_info_by_access_key(store, k.id, inf); if (r >= 0 && inf.user_id.compare(info.user_id) != 0) { ldout(store->ctx(), 0) << "WARNING: can't store user info, access key already mapped to another user" << dendl; @@ -68,27 +72,37 @@ int rgw_store_user_info(RGWRados *store, RGWUserInfo& info, bool exclusive) } } - bufferlist uid_bl; RGWUID ui; ui.user_id = info.user_id; - ::encode(ui, uid_bl); - ::encode(info, uid_bl); - ret = rgw_put_system_obj(store, store->params.user_uid_pool, info.user_id, uid_bl.c_str(), uid_bl.length(), exclusive); + bufferlist link_bl; + ::encode(ui, link_bl); + + bufferlist data_bl; + ::encode(ui, data_bl); + ::encode(info, data_bl); + + ret = rgw_put_system_obj(store, store->params.user_uid_pool, info.user_id, data_bl.c_str(), data_bl.length(), exclusive); if (ret < 0) return ret; if (info.user_email.size()) { - ret = rgw_put_system_obj(store, store->params.user_email_pool, info.user_email, uid_bl.c_str(), uid_bl.length(), exclusive); - if (ret < 0) - return ret; + if (!old_info || + old_info->user_email.compare(info.user_email) != 0) { /* only if new index changed */ + ret = rgw_put_system_obj(store, store->params.user_email_pool, info.user_email, link_bl.c_str(), link_bl.length(), exclusive); + if (ret < 0) + return ret; + } } if (info.access_keys.size()) { map::iterator iter = info.access_keys.begin(); for (; iter != info.access_keys.end(); ++iter) { RGWAccessKey& k = iter->second; - ret = rgw_put_system_obj(store, store->params.user_keys_pool, k.id, uid_bl.c_str(), uid_bl.length(), exclusive); + if (old_info && old_info->access_keys.count(iter->first) != 0) + continue; + + ret = rgw_put_system_obj(store, store->params.user_keys_pool, k.id, link_bl.c_str(), link_bl.length(), exclusive); if (ret < 0) return ret; } @@ -97,7 +111,10 @@ int rgw_store_user_info(RGWRados *store, RGWUserInfo& info, bool exclusive) map::iterator siter; for (siter = info.swift_keys.begin(); siter != info.swift_keys.end(); ++siter) { RGWAccessKey& k = siter->second; - ret = rgw_put_system_obj(store, store->params.user_swift_pool, k.id, uid_bl.c_str(), uid_bl.length(), exclusive); + if (old_info && old_info->swift_keys.count(siter->first) != 0) + continue; + + ret = rgw_put_system_obj(store, store->params.user_swift_pool, k.id, link_bl.c_str(), link_bl.length(), exclusive); if (ret < 0) return ret; } @@ -117,8 +134,7 @@ int rgw_get_user_info_from_index(RGWRados *store, string& key, rgw_bucket& bucke bufferlist::iterator iter = bl.begin(); try { ::decode(uid, iter); - if (!iter.end()) - info.decode(iter); + return rgw_get_user_info_by_uid(store, uid.user_id, info); } catch (buffer::error& err) { ldout(store->ctx(), 0) << "ERROR: failed to decode user info, caught buffer::error" << dendl; return -EIO; @@ -133,7 +149,29 @@ int rgw_get_user_info_from_index(RGWRados *store, string& key, rgw_bucket& bucke */ int rgw_get_user_info_by_uid(RGWRados *store, string& uid, RGWUserInfo& info) { - return rgw_get_user_info_from_index(store, uid, store->params.user_uid_pool, info); + bufferlist bl; + RGWUID user_id; + + int ret = rgw_get_obj(store, NULL, store->params.user_uid_pool, uid, bl); + if (ret < 0) + return ret; + + bufferlist::iterator iter = bl.begin(); + try { + ::decode(user_id, iter); + if (user_id.user_id.compare(uid) != 0) { + lderr(store->ctx()) << "ERROR: rgw_get_user_info_by_uid(): user id mismatch: " << user_id.user_id << " != " << uid << dendl; + return -EIO; + } + if (!iter.end()) { + ::decode(info, iter); + } + } catch (buffer::error& err) { + ldout(store->ctx(), 0) << "ERROR: failed to decode user info, caught buffer::error" << dendl; + return -EIO; + } + + return 0; } /** diff --git a/src/rgw/rgw_user.h b/src/rgw/rgw_user.h index 3ae6b38a15655..d8ae3c3f2ec74 100644 --- a/src/rgw/rgw_user.h +++ b/src/rgw/rgw_user.h @@ -40,7 +40,7 @@ extern bool rgw_user_is_authenticated(RGWUserInfo& info); * Save the given user information to storage. * Returns: 0 on success, -ERR# on failure. */ -extern int rgw_store_user_info(RGWRados *store, RGWUserInfo& info, bool exclusive); +extern int rgw_store_user_info(RGWRados *store, RGWUserInfo& info, RGWUserInfo *old_info, bool exclusive); /** * Given an email, finds the user info associated with it. * returns: 0 on success, -ERR# on failure (including nonexistence) -- 2.39.5