Skip to content

Commit

Permalink
rgw/services: No null_yield
Browse files Browse the repository at this point in the history
Null yield is fine at top levels where we know what kind of thread
we're running into, but do not belong in general use functions.

Signed-off-by: Adam C. Emerson <[email protected]>
  • Loading branch information
adamemerson committed Nov 13, 2020
1 parent a0e1a8f commit 4cd42ce
Show file tree
Hide file tree
Showing 53 changed files with 468 additions and 397 deletions.
45 changes: 24 additions & 21 deletions src/rgw/rgw_admin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1314,7 +1314,7 @@ int set_user_bucket_quota(OPT opt_cmd, RGWUser& user, RGWUserAdminOpState& op_st
op_state.set_bucket_quota(user_info.bucket_quota);

string err;
int r = user.modify(op_state, &err);
int r = user.modify(op_state, null_yield, &err);
if (r < 0) {
cerr << "ERROR: failed updating user info: " << cpp_strerror(-r) << ": " << err << std::endl;
return -r;
Expand All @@ -1332,7 +1332,7 @@ int set_user_quota(OPT opt_cmd, RGWUser& user, RGWUserAdminOpState& op_state, in
op_state.set_user_quota(user_info.user_quota);

string err;
int r = user.modify(op_state, &err);
int r = user.modify(op_state, null_yield, &err);
if (r < 0) {
cerr << "ERROR: failed updating user info: " << cpp_strerror(-r) << ": " << err << std::endl;
return -r;
Expand Down Expand Up @@ -5575,7 +5575,7 @@ int main(int argc, const char **argv)
if (yes_i_really_mean_it) {
user_op.set_overwrite_new_user(true);
}
ret = user.rename(user_op, &err_msg);
ret = user.rename(user_op, null_yield, &err_msg);
if (ret < 0) {
if (ret == -EEXIST) {
err_msg += ". to overwrite this user, add --yes-i-really-mean-it";
Expand All @@ -5588,7 +5588,7 @@ int main(int argc, const char **argv)
case OPT::USER_ENABLE:
case OPT::USER_SUSPEND:
case OPT::USER_MODIFY:
ret = user.modify(user_op, &err_msg);
ret = user.modify(user_op, null_yield, &err_msg);
if (ret < 0) {
cerr << "could not modify user: " << err_msg << std::endl;
return -ret;
Expand All @@ -5604,7 +5604,7 @@ int main(int argc, const char **argv)

break;
case OPT::SUBUSER_MODIFY:
ret = user.subusers.modify(user_op, &err_msg);
ret = user.subusers.modify(user_op, null_yield, &err_msg);
if (ret < 0) {
cerr << "could not modify subuser: " << err_msg << std::endl;
return -ret;
Expand Down Expand Up @@ -5969,7 +5969,7 @@ int main(int argc, const char **argv)
user_ids.push_back(user_id.id);
ret =
RGWBucketAdminOp::limit_check(store, bucket_op, user_ids, f,
warnings_only);
null_yield, warnings_only);
} else {
/* list users in groups of max-keys, then perform user-bucket
* limit-check on each group */
Expand All @@ -5991,7 +5991,7 @@ int main(int argc, const char **argv)
/* ok, do the limit checks for this group */
ret =
RGWBucketAdminOp::limit_check(store, bucket_op, user_ids, f,
warnings_only);
null_yield, warnings_only);
if (ret < 0)
break;
}
Expand All @@ -6010,7 +6010,7 @@ int main(int argc, const char **argv)
return -ENOENT;
}
}
RGWBucketAdminOp::info(store, bucket_op, f);
RGWBucketAdminOp::info(store, bucket_op, f, null_yield);
} else {
RGWBucketInfo bucket_info;
int ret = init_bucket(tenant, bucket_name, bucket_id, bucket_info, bucket);
Expand Down Expand Up @@ -6106,7 +6106,7 @@ int main(int argc, const char **argv)
}
bucket_op.set_fetch_stats(true);

int r = RGWBucketAdminOp::info(store, bucket_op, f);
int r = RGWBucketAdminOp::info(store, bucket_op, f, null_yield);
if (r < 0) {
cerr << "failure: " << cpp_strerror(-r) << ": " << err << std::endl;
return -r;
Expand Down Expand Up @@ -6286,7 +6286,7 @@ int main(int argc, const char **argv)
exit(1);
}

int ret = store->svc()->zone->add_bucket_placement(pool);
int ret = store->svc()->zone->add_bucket_placement(pool, null_yield);
if (ret < 0)
cerr << "failed to add bucket placement: " << cpp_strerror(-ret) << std::endl;
}
Expand All @@ -6297,14 +6297,14 @@ int main(int argc, const char **argv)
exit(1);
}

int ret = store->svc()->zone->remove_bucket_placement(pool);
int ret = store->svc()->zone->remove_bucket_placement(pool, null_yield);
if (ret < 0)
cerr << "failed to remove bucket placement: " << cpp_strerror(-ret) << std::endl;
}

if (opt_cmd == OPT::POOLS_LIST) {
set<rgw_pool> pools;
int ret = store->svc()->zone->list_placement_set(pools);
int ret = store->svc()->zone->list_placement_set(pools, null_yield);
if (ret < 0) {
cerr << "could not list placement set: " << cpp_strerror(-ret) << std::endl;
return -ret;
Expand Down Expand Up @@ -7408,7 +7408,7 @@ int main(int argc, const char **argv)
}

if (opt_cmd == OPT::USER_CHECK) {
check_bad_user_bucket_mapping(store, user_id, fix);
check_bad_user_bucket_mapping(store, user_id, fix, null_yield);
}

if (opt_cmd == OPT::USER_STATS) {
Expand All @@ -7428,7 +7428,7 @@ int main(int argc, const char **argv)
"so at most one of the two should be specified" << std::endl;
return EINVAL;
}
ret = store->ctl()->user->reset_stats(user_id);
ret = store->ctl()->user->reset_stats(user_id, null_yield);
if (ret < 0) {
cerr << "ERROR: could not reset user stats: " << cpp_strerror(-ret) <<
std::endl;
Expand All @@ -7444,14 +7444,14 @@ int main(int argc, const char **argv)
cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl;
return -ret;
}
ret = store->ctl()->bucket->sync_user_stats(user_id, bucket_info);
ret = store->ctl()->bucket->sync_user_stats(user_id, bucket_info, null_yield);
if (ret < 0) {
cerr << "ERROR: could not sync bucket stats: " <<
cpp_strerror(-ret) << std::endl;
return -ret;
}
} else {
int ret = rgw_user_sync_all_stats(store, user_id);
int ret = rgw_user_sync_all_stats(store, user_id, null_yield);
if (ret < 0) {
cerr << "ERROR: could not sync user stats: " <<
cpp_strerror(-ret) << std::endl;
Expand All @@ -7463,7 +7463,9 @@ int main(int argc, const char **argv)
RGWStorageStats stats;
ceph::real_time last_stats_sync;
ceph::real_time last_stats_update;
int ret = store->ctl()->user->read_stats(user_id, &stats, &last_stats_sync, &last_stats_update);
int ret = store->ctl()->user->read_stats(user_id, &stats, null_yield,
&last_stats_sync,
&last_stats_update);
if (ret < 0) {
if (ret == -ENOENT) { /* in case of ENOENT */
cerr << "User has not been initialized or user does not exist" << std::endl;
Expand Down Expand Up @@ -7667,7 +7669,7 @@ int main(int argc, const char **argv)

if (opt_cmd == OPT::MDLOG_AUTOTRIM) {
// need a full history for purging old mdlog periods
store->svc()->mdlog->init_oldest_log_period();
store->svc()->mdlog->init_oldest_log_period(null_yield);

RGWCoroutinesManager crs(store->ctx(), store->getRados()->get_cr_registry());
RGWHTTPManager http(store->ctx(), crs.get_completion_mgr());
Expand Down Expand Up @@ -8630,7 +8632,8 @@ int main(int argc, const char **argv)
return -ret;
}
map<int, string> markers;
ret = store->svc()->bilog_rados->get_log_status(bucket_info, shard_id, &markers);
ret = store->svc()->bilog_rados->get_log_status(bucket_info, shard_id,
&markers, null_yield);
if (ret < 0) {
cerr << "ERROR: get_bi_log_status(): " << cpp_strerror(-ret) << std::endl;
return -ret;
Expand Down Expand Up @@ -8894,7 +8897,7 @@ int main(int argc, const char **argv)
user_info.mfa_ids.insert(totp_serial);
user_op.set_mfa_ids(user_info.mfa_ids);
string err;
ret = user.modify(user_op, &err);
ret = user.modify(user_op, null_yield, &err);
if (ret < 0) {
cerr << "ERROR: failed storing user info, error: " << err << std::endl;
return -ret;
Expand Down Expand Up @@ -8930,7 +8933,7 @@ int main(int argc, const char **argv)
user_info.mfa_ids.erase(totp_serial);
user_op.set_mfa_ids(user_info.mfa_ids);
string err;
ret = user.modify(user_op, &err);
ret = user.modify(user_op, null_yield, &err);
if (ret < 0) {
cerr << "ERROR: failed storing user info, error: " << err << std::endl;
return -ret;
Expand Down
46 changes: 25 additions & 21 deletions src/rgw/rgw_bucket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,11 @@ int rgw_read_user_buckets(rgw::sal::RGWRadosStore * store,
const string& marker,
const string& end_marker,
uint64_t max,
bool need_stats)
bool need_stats,
optional_yield y)
{
rgw::sal::RGWRadosUser user(store, user_id);
return user.list_buckets(marker, end_marker, max, need_stats, buckets);
return user.list_buckets(marker, end_marker, max, need_stats, buckets, y);
}

int rgw_bucket_parse_bucket_instance(const string& bucket_instance, string *bucket_name, string *bucket_id, int *shard_id)
Expand Down Expand Up @@ -258,7 +259,8 @@ static void dump_mulipart_index_results(list<rgw_obj_index_key>& objs_to_unlink,
}

void check_bad_user_bucket_mapping(rgw::sal::RGWRadosStore *store, const rgw_user& user_id,
bool fix)
bool fix,
optional_yield y)
{
rgw::sal::RGWBucketList user_buckets;
rgw::sal::RGWRadosUser user(store, user_id);
Expand All @@ -269,7 +271,7 @@ void check_bad_user_bucket_mapping(rgw::sal::RGWRadosStore *store, const rgw_use
size_t max_entries = cct->_conf->rgw_list_buckets_max_chunk;

do {
int ret = user.list_buckets(marker, string(), max_entries, false, user_buckets);
int ret = user.list_buckets(marker, string(), max_entries, false, user_buckets, y);
if (ret < 0) {
ldout(store->ctx(), 0) << "failed to read user buckets: "
<< cpp_strerror(-ret) << dendl;
Expand Down Expand Up @@ -475,7 +477,7 @@ int rgw_remove_bucket_bypass_gc(rgw::sal::RGWRadosStore *store, rgw_bucket& buck
return ret;
}

ret = store->ctl()->bucket->sync_user_stats(info.owner, info);
ret = store->ctl()->bucket->sync_user_stats(info.owner, info, y);
if (ret < 0) {
dout(1) << "WARNING: failed sync user stats before bucket delete. ret=" << ret << dendl;
}
Expand Down Expand Up @@ -1372,7 +1374,7 @@ static int bucket_stats(rgw::sal::RGWRadosStore *store,
int RGWBucketAdminOp::limit_check(rgw::sal::RGWRadosStore *store,
RGWBucketAdminOpState& op_state,
const std::list<std::string>& user_ids,
RGWFormatterFlusher& flusher,
RGWFormatterFlusher& flusher, optional_yield y,
bool warnings_only)
{
int ret = 0;
Expand Down Expand Up @@ -1403,7 +1405,7 @@ int RGWBucketAdminOp::limit_check(rgw::sal::RGWRadosStore *store,
do {
rgw::sal::RGWRadosUser user(store, rgw_user(user_id));

ret = user.list_buckets(marker, string(), max_entries, false, buckets);
ret = user.list_buckets(marker, string(), max_entries, false, buckets, y);

if (ret < 0)
return ret;
Expand Down Expand Up @@ -1488,7 +1490,8 @@ int RGWBucketAdminOp::limit_check(rgw::sal::RGWRadosStore *store,

int RGWBucketAdminOp::info(rgw::sal::RGWRadosStore *store,
RGWBucketAdminOpState& op_state,
RGWFormatterFlusher& flusher)
RGWFormatterFlusher& flusher,
optional_yield y)
{
RGWBucket bucket;
int ret = 0;
Expand Down Expand Up @@ -1521,7 +1524,7 @@ int RGWBucketAdminOp::info(rgw::sal::RGWRadosStore *store,

do {
ret = user.list_buckets(marker, empty_end_marker, max_entries,
no_need_stats, buckets);
no_need_stats, buckets, y);
if (ret < 0) {
return ret;
}
Expand Down Expand Up @@ -2570,7 +2573,7 @@ int RGWMetadataHandlerPut_BucketInstance::put_check()
bci.info.bucket.tenant = tenant_name;
// if the sync module never writes data, don't require the zone to specify all placement targets
if (bihandler->svc.zone->sync_module_supports_writes()) {
ret = bihandler->svc.zone->select_bucket_location_by_rule(bci.info.placement_rule, &rule_info);
ret = bihandler->svc.zone->select_bucket_location_by_rule(bci.info.placement_rule, &rule_info, y);
if (ret < 0) {
ldout(cct, 0) << "ERROR: select_bucket_placement() returned " << ret << dendl;
return ret;
Expand Down Expand Up @@ -2969,18 +2972,18 @@ int RGWBucketCtl::link_bucket(const rgw_user& user_id,
rgw_ep_info *pinfo)
{
return bm_handler->call([&](RGWSI_Bucket_EP_Ctx& ctx) {
return do_link_bucket(ctx, user_id, bucket, creation_time, y,
update_entrypoint, pinfo);
return do_link_bucket(ctx, user_id, bucket, creation_time,
update_entrypoint, pinfo, y);
});
}

int RGWBucketCtl::do_link_bucket(RGWSI_Bucket_EP_Ctx& ctx,
const rgw_user& user_id,
const rgw_bucket& bucket,
ceph::real_time creation_time,
optional_yield y,
bool update_entrypoint,
rgw_ep_info *pinfo)
rgw_ep_info *pinfo,
optional_yield y)
{
int ret;

Expand Down Expand Up @@ -3009,7 +3012,7 @@ int RGWBucketCtl::do_link_bucket(RGWSI_Bucket_EP_Ctx& ctx,
}
}

ret = ctl.user->add_bucket(user_id, bucket, creation_time);
ret = ctl.user->add_bucket(user_id, bucket, creation_time, y);
if (ret < 0) {
ldout(cct, 0) << "ERROR: error adding bucket to user directory:"
<< " user=" << user_id
Expand All @@ -3033,7 +3036,7 @@ int RGWBucketCtl::do_link_bucket(RGWSI_Bucket_EP_Ctx& ctx,
return 0;

done_err:
int r = do_unlink_bucket(ctx, user_id, bucket, y, true);
int r = do_unlink_bucket(ctx, user_id, bucket, true, y);
if (r < 0) {
ldout(cct, 0) << "ERROR: failed unlinking bucket on error cleanup: "
<< cpp_strerror(-r) << dendl;
Expand All @@ -3044,17 +3047,17 @@ int RGWBucketCtl::do_link_bucket(RGWSI_Bucket_EP_Ctx& ctx,
int RGWBucketCtl::unlink_bucket(const rgw_user& user_id, const rgw_bucket& bucket, optional_yield y, bool update_entrypoint)
{
return bm_handler->call([&](RGWSI_Bucket_EP_Ctx& ctx) {
return do_unlink_bucket(ctx, user_id, bucket, y, update_entrypoint);
return do_unlink_bucket(ctx, user_id, bucket, update_entrypoint, y);
});
}

int RGWBucketCtl::do_unlink_bucket(RGWSI_Bucket_EP_Ctx& ctx,
const rgw_user& user_id,
const rgw_bucket& bucket,
optional_yield y,
bool update_entrypoint)
bool update_entrypoint,
optional_yield y)
{
int ret = ctl.user->remove_bucket(user_id, bucket);
int ret = ctl.user->remove_bucket(user_id, bucket, y);
if (ret < 0) {
ldout(cct, 0) << "ERROR: error removing bucket from directory: "
<< cpp_strerror(-ret)<< dendl;
Expand Down Expand Up @@ -3218,6 +3221,7 @@ int RGWBucketCtl::read_buckets_stats(map<string, RGWBucketEnt>& m,

int RGWBucketCtl::sync_user_stats(const rgw_user& user_id,
const RGWBucketInfo& bucket_info,
optional_yield y,
RGWBucketEnt* pent)
{
RGWBucketEnt ent;
Expand All @@ -3230,7 +3234,7 @@ int RGWBucketCtl::sync_user_stats(const rgw_user& user_id,
return r;
}

return ctl.user->flush_bucket_stats(user_id, *pent);
return ctl.user->flush_bucket_stats(user_id, *pent, y);
}

int RGWBucketCtl::get_sync_policy_handler(std::optional<rgw_zone_id> zone,
Expand Down
Loading

0 comments on commit 4cd42ce

Please sign in to comment.