Skip to content

Commit

Permalink
Merge pull request #9017 from yehudasa/wip-15625-jewel
Browse files Browse the repository at this point in the history 
jewel: rgw: segfault at RGWAsyncGetSystemObj
  • Loading branch information
@yehudasa
yehudasa committed May 10, 2016
2 parents 6f23218 + c08e90e commit 4ead0f9
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 78 deletions.
1 change: 1 addition & 0 deletions src/cls/rgw/cls_rgw.cc
View file
Expand Up @@ -870,6 +870,7 @@ int rgw_bucket_complete_op(cls_method_context_t hctx, bufferlist *in, bufferlist
entry.ver = op.ver;
switch ((int)op.op) {
case CLS_RGW_OP_DEL:
entry.meta = op.meta;
if (ondisk) {
if (!entry.pending_map.size()) {
int ret = cls_cxx_map_remove_key(hctx, idx);
Expand Down
10 changes: 4 additions & 6 deletions src/rgw/rgw_cr_rados.h
View file
Expand Up @@ -174,7 +174,7 @@ template <class T>
class RGWSimpleRadosReadCR : public RGWSimpleCoroutine {
RGWAsyncRadosProcessor *async_rados;
RGWRados *store;
RGWObjectCtx& obj_ctx;
RGWObjectCtx obj_ctx;
bufferlist bl;

rgw_bucket pool;
Expand All @@ -188,11 +188,10 @@ class RGWSimpleRadosReadCR : public RGWSimpleCoroutine {

public:
RGWSimpleRadosReadCR(RGWAsyncRadosProcessor *_async_rados, RGWRados *_store,
RGWObjectCtx& _obj_ctx,
const rgw_bucket& _pool, const string& _oid,
T *_result) : RGWSimpleCoroutine(_store->ctx()),
async_rados(_async_rados), store(_store),
obj_ctx(_obj_ctx),
obj_ctx(store),
pool(_pool), oid(_oid),
pattrs(NULL),
result(_result),
Expand Down Expand Up @@ -252,7 +251,7 @@ int RGWSimpleRadosReadCR<T>::request_complete()
class RGWSimpleRadosReadAttrsCR : public RGWSimpleCoroutine {
RGWAsyncRadosProcessor *async_rados;
RGWRados *store;
RGWObjectCtx& obj_ctx;
RGWObjectCtx obj_ctx;
bufferlist bl;

rgw_bucket pool;
Expand All @@ -264,11 +263,10 @@ class RGWSimpleRadosReadAttrsCR : public RGWSimpleCoroutine {

public:
RGWSimpleRadosReadAttrsCR(RGWAsyncRadosProcessor *_async_rados, RGWRados *_store,
RGWObjectCtx& _obj_ctx,
rgw_bucket& _pool, const string& _oid,
map<string, bufferlist> *_pattrs) : RGWSimpleCoroutine(_store->ctx()),
async_rados(_async_rados), store(_store),
obj_ctx(_obj_ctx),
obj_ctx(store),
pool(_pool), oid(_oid),
pattrs(_pattrs),
req(NULL) { }
Expand Down
65 changes: 25 additions & 40 deletions src/rgw/rgw_data_sync.cc
View file
Expand Up @@ -46,18 +46,15 @@ void rgw_datalog_shard_data::decode_json(JSONObj *obj) {
class RGWReadDataSyncStatusCoroutine : public RGWSimpleRadosReadCR<rgw_data_sync_info> {
RGWDataSyncEnv *sync_env;

RGWObjectCtx& obj_ctx;

rgw_data_sync_status *sync_status;

public:
RGWReadDataSyncStatusCoroutine(RGWDataSyncEnv *_sync_env, RGWObjectCtx& _obj_ctx,
rgw_data_sync_status *_status) : RGWSimpleRadosReadCR(_sync_env->async_rados, _sync_env->store, _obj_ctx,
RGWReadDataSyncStatusCoroutine(RGWDataSyncEnv *_sync_env,
rgw_data_sync_status *_status) : RGWSimpleRadosReadCR(_sync_env->async_rados, _sync_env->store,
_sync_env->store->get_zone_params().log_pool,
RGWDataSyncStatusManager::sync_status_oid(_sync_env->source_zone),
&_status->sync_info),
sync_env(_sync_env),
obj_ctx(_obj_ctx),
sync_status(_status) {}

int handle_data(rgw_data_sync_info& data);
Expand All @@ -72,7 +69,7 @@ int RGWReadDataSyncStatusCoroutine::handle_data(rgw_data_sync_info& data)
map<uint32_t, rgw_data_sync_marker>& markers = sync_status->sync_markers;
RGWRados *store = sync_env->store;
for (int i = 0; i < (int)data.num_shards; i++) {
spawn(new RGWSimpleRadosReadCR<rgw_data_sync_marker>(sync_env->async_rados, store, obj_ctx, store->get_zone_params().log_pool,
spawn(new RGWSimpleRadosReadCR<rgw_data_sync_marker>(sync_env->async_rados, store, store->get_zone_params().log_pool,
RGWDataSyncStatusManager::shard_obj_name(sync_env->source_zone, i), &markers[i]), true);
}
return 0;
Expand Down Expand Up @@ -344,7 +341,6 @@ class RGWInitDataSyncStatusCoroutine : public RGWCoroutine {
RGWDataSyncEnv *sync_env;

RGWRados *store;
RGWObjectCtx& obj_ctx;

string sync_status_oid;

Expand All @@ -354,9 +350,8 @@ class RGWInitDataSyncStatusCoroutine : public RGWCoroutine {
map<int, RGWDataChangesLogInfo> shards_info;
public:
RGWInitDataSyncStatusCoroutine(RGWDataSyncEnv *_sync_env,
RGWObjectCtx& _obj_ctx, uint32_t _num_shards) : RGWCoroutine(_sync_env->cct),
sync_env(_sync_env), store(sync_env->store),
obj_ctx(_obj_ctx) {
uint32_t _num_shards) : RGWCoroutine(_sync_env->cct),
sync_env(_sync_env), store(sync_env->store) {
lock_name = "sync_lock";
status.num_shards = _num_shards;

Expand Down Expand Up @@ -527,8 +522,7 @@ int RGWRemoteDataLog::get_shard_info(int shard_id)

int RGWRemoteDataLog::read_sync_status(rgw_data_sync_status *sync_status)
{
RGWObjectCtx obj_ctx(store, NULL);
int r = run(new RGWReadDataSyncStatusCoroutine(&sync_env, obj_ctx, sync_status));
int r = run(new RGWReadDataSyncStatusCoroutine(&sync_env, sync_status));
if (r == -ENOENT) {
r = 0;
}
Expand All @@ -537,8 +531,7 @@ int RGWRemoteDataLog::read_sync_status(rgw_data_sync_status *sync_status)

int RGWRemoteDataLog::init_sync_status(int num_shards)
{
RGWObjectCtx obj_ctx(store, NULL);
return run(new RGWInitDataSyncStatusCoroutine(&sync_env, obj_ctx, num_shards));
return run(new RGWInitDataSyncStatusCoroutine(&sync_env, num_shards));
}

static string full_data_sync_index_shard_oid(const string& source_zone, int shard_id)
Expand Down Expand Up @@ -1139,8 +1132,7 @@ class RGWDataSyncShardControlCR : public RGWBackoffControlCR {

RGWCoroutine *alloc_finisher_cr() {
RGWRados *store = sync_env->store;
RGWObjectCtx obj_ctx(store, NULL);
return new RGWSimpleRadosReadCR<rgw_data_sync_marker>(sync_env->async_rados, store, obj_ctx, store->get_zone_params().log_pool,
return new RGWSimpleRadosReadCR<rgw_data_sync_marker>(sync_env->async_rados, store, store->get_zone_params().log_pool,
RGWDataSyncStatusManager::shard_obj_name(sync_env->source_zone, shard_id), &sync_marker);
}

Expand All @@ -1160,8 +1152,6 @@ class RGWDataSyncCR : public RGWCoroutine {
RGWDataSyncEnv *sync_env;
uint32_t num_shards;

RGWObjectCtx obj_ctx;

rgw_data_sync_status sync_status;

RGWDataSyncShardMarkerTrack *marker_tracker;
Expand All @@ -1175,7 +1165,6 @@ class RGWDataSyncCR : public RGWCoroutine {
RGWDataSyncCR(RGWDataSyncEnv *_sync_env, uint32_t _num_shards, bool *_reset_backoff) : RGWCoroutine(_sync_env->cct),
sync_env(_sync_env),
num_shards(_num_shards),
obj_ctx(sync_env->store),
marker_tracker(NULL),
shard_crs_lock("RGWDataSyncCR::shard_crs_lock"),
reset_backoff(_reset_backoff) {
Expand All @@ -1191,7 +1180,7 @@ class RGWDataSyncCR : public RGWCoroutine {
reenter(this) {

/* read sync status */
yield call(new RGWReadDataSyncStatusCoroutine(sync_env, obj_ctx, &sync_status));
yield call(new RGWReadDataSyncStatusCoroutine(sync_env, &sync_status));

if (retcode == -ENOENT) {
sync_status.sync_info.num_shards = num_shards;
Expand All @@ -1203,7 +1192,7 @@ class RGWDataSyncCR : public RGWCoroutine {
/* state: init status */
if ((rgw_data_sync_info::SyncState)sync_status.sync_info.state == rgw_data_sync_info::StateInit) {
ldout(sync_env->cct, 20) << __func__ << "(): init" << dendl;
yield call(new RGWInitDataSyncStatusCoroutine(sync_env, obj_ctx, sync_status.sync_info.num_shards));
yield call(new RGWInitDataSyncStatusCoroutine(sync_env, sync_status.sync_info.num_shards));
if (retcode < 0) {
ldout(sync_env->cct, 0) << "ERROR: failed to init sync, retcode=" << retcode << dendl;
return set_cr_error(retcode);
Expand Down Expand Up @@ -1320,9 +1309,7 @@ void RGWRemoteDataLog::wakeup(int shard_id, set<string>& keys) {

int RGWRemoteDataLog::run_sync(int num_shards, rgw_data_sync_status& sync_status)
{
RGWObjectCtx obj_ctx(store, NULL);

int r = run(new RGWReadDataSyncStatusCoroutine(&sync_env, obj_ctx, &sync_status));
int r = run(new RGWReadDataSyncStatusCoroutine(&sync_env, &sync_status));
if (r < 0 && r != -ENOENT) {
ldout(store->ctx(), 0) << "ERROR: failed to read sync status from source_zone=" << sync_env.source_zone << " r=" << r << dendl;
return r;
Expand Down Expand Up @@ -1604,7 +1591,6 @@ void rgw_bucket_shard_inc_sync_marker::encode_attr(map<string, bufferlist>& attr

class RGWReadBucketSyncStatusCoroutine : public RGWCoroutine {
RGWDataSyncEnv *sync_env;
RGWObjectCtx obj_ctx;
string oid;
rgw_bucket_shard_sync_info *status;

Expand All @@ -1614,7 +1600,6 @@ class RGWReadBucketSyncStatusCoroutine : public RGWCoroutine {
const string& _bucket_name, const string _bucket_id, int _shard_id,
rgw_bucket_shard_sync_info *_status) : RGWCoroutine(_sync_env->cct),
sync_env(_sync_env),
obj_ctx(sync_env->store),
oid(RGWBucketSyncStatusManager::status_oid(sync_env->source_zone, _bucket_name, _bucket_id, _shard_id)),
status(_status) {}
int operate();
Expand All @@ -1623,7 +1608,7 @@ class RGWReadBucketSyncStatusCoroutine : public RGWCoroutine {
int RGWReadBucketSyncStatusCoroutine::operate()
{
reenter(this) {
yield call(new RGWSimpleRadosReadAttrsCR(sync_env->async_rados, sync_env->store, obj_ctx,
yield call(new RGWSimpleRadosReadAttrsCR(sync_env->async_rados, sync_env->store,
sync_env->store->get_zone_params().log_pool,
oid,
&attrs));
Expand Down Expand Up @@ -1863,7 +1848,7 @@ class RGWBucketIncSyncShardMarkerTrack : public RGWSyncShardMarkerTrack<string,
string marker_oid;
rgw_bucket_shard_inc_sync_marker sync_marker;

map<rgw_obj_key, pair<RGWModifyOp, string> > key_to_marker;
map<rgw_obj_key, string> key_to_marker;
map<string, rgw_obj_key> marker_to_key;

void handle_finish(const string& marker) {
Expand Down Expand Up @@ -1906,23 +1891,18 @@ class RGWBucketIncSyncShardMarkerTrack : public RGWSyncShardMarkerTrack<string,
* Also, we should make sure that we don't run concurrent operations on the same key with
* different ops.
*/
bool index_key_to_marker(const rgw_obj_key& key, RGWModifyOp op, const string& marker) {
bool index_key_to_marker(const rgw_obj_key& key, const string& marker) {
if (key_to_marker.find(key) != key_to_marker.end()) {
set_need_retry(key);
return false;
}
key_to_marker[key] = make_pair<>(op, marker);
key_to_marker[key] = marker;
marker_to_key[marker] = key;
return true;
}

bool can_do_op(const rgw_obj_key& key, RGWModifyOp op) {
auto i = key_to_marker.find(key);
if (i == key_to_marker.end()) {
return true;
}

return (i->second.first == op);
bool can_do_op(const rgw_obj_key& key) {
return (key_to_marker.find(key) == key_to_marker.end());
}
};

Expand Down Expand Up @@ -2313,16 +2293,21 @@ int RGWBucketShardIncrementalSyncCR::operate()
}
ldout(sync_env->cct, 20) << "[inc sync] syncing object: " << bucket_name << ":" << bucket_id << ":" << shard_id << "/" << key << dendl;
updated_status = false;
while (!marker_tracker->can_do_op(key, entry->op)) {
while (!marker_tracker->can_do_op(key)) {
if (!updated_status) {
set_status() << "can't do op, conflicting inflight operation";
updated_status = true;
}
ldout(sync_env->cct, 5) << *this << ": [inc sync] can't do op on key=" << key << " need to wait for conflicting operation to complete" << dendl;
yield wait_for_child();

while (collect(&ret)) {
if (ret < 0) {
ldout(sync_env->cct, 0) << "ERROR: a child operation returned error (ret=" << ret << ")" << dendl;
/* we have reported this error */
}
}
}
if (!marker_tracker->index_key_to_marker(key, entry->op, cur_id)) {
if (!marker_tracker->index_key_to_marker(key, cur_id)) {
set_status() << "can't do op, sync already in progress for object";
ldout(sync_env->cct, 20) << __func__ << ": skipping sync of entry: " << cur_id << ":" << key << " sync already in progress for object" << dendl;
marker_tracker->try_update_high_marker(cur_id, 0, entry->timestamp);
Expand Down
20 changes: 12 additions & 8 deletions src/rgw/rgw_rados.cc
View file
Expand Up @@ -7491,12 +7491,15 @@ int RGWRados::Object::Delete::delete_obj()
}
}

if (!state->exists) {
target->invalidate_state();
return -ENOENT;
}

r = target->prepare_atomic_modification(op, false, NULL, NULL, NULL, true);
if (r < 0)
return r;

bool ret_not_existed = (!state->exists);

RGWBucketInfo& bucket_info = target->get_bucket_info();

RGWRados::Bucket bop(store, bucket_info);
Expand Down Expand Up @@ -7525,7 +7528,7 @@ int RGWRados::Object::Delete::delete_obj()

int64_t poolid = ref.ioctx.get_id();
if (r >= 0) {
r = index_op.complete_del(poolid, ref.ioctx.get_last_version(), params.remove_objs);
r = index_op.complete_del(poolid, ref.ioctx.get_last_version(), state->mtime, params.remove_objs);
} else {
int ret = index_op.cancel();
if (ret < 0) {
Expand All @@ -7547,9 +7550,6 @@ int RGWRados::Object::Delete::delete_obj()
if (r < 0)
return r;

if (ret_not_existed)
return -ENOENT;

/* update quota cache */
store->quota_handler->update_stats(params.bucket_owner, bucket, -1, 0, obj_size);

Expand Down Expand Up @@ -7615,7 +7615,8 @@ int RGWRados::delete_obj_index(rgw_obj& obj)
RGWRados::Bucket bop(this, bucket_info);
RGWRados::Bucket::UpdateIndex index_op(&bop, obj, NULL);

int r = index_op.complete_del(-1 /* pool */, 0, NULL);
real_time removed_mtime;
int r = index_op.complete_del(-1 /* pool */, 0, removed_mtime, NULL);

return r;
}
Expand Down Expand Up @@ -8555,6 +8556,7 @@ int RGWRados::Bucket::UpdateIndex::complete(int64_t poolid, uint64_t epoch, uint
}

int RGWRados::Bucket::UpdateIndex::complete_del(int64_t poolid, uint64_t epoch,
real_time& removed_mtime,
list<rgw_obj_key> *remove_objs)
{
if (blind) {
Expand All @@ -8568,7 +8570,7 @@ int RGWRados::Bucket::UpdateIndex::complete_del(int64_t poolid, uint64_t epoch,
return ret;
}

ret = store->cls_obj_complete_del(*bs, optag, poolid, epoch, obj, remove_objs, bilog_flags);
ret = store->cls_obj_complete_del(*bs, optag, poolid, epoch, obj, removed_mtime, remove_objs, bilog_flags);

int r = store->data_log->add_entry(bs->bucket, bs->shard_id);
if (r < 0) {
Expand Down Expand Up @@ -11039,10 +11041,12 @@ int RGWRados::cls_obj_complete_add(BucketShard& bs, string& tag,
int RGWRados::cls_obj_complete_del(BucketShard& bs, string& tag,
int64_t pool, uint64_t epoch,
rgw_obj& obj,
real_time& removed_mtime,
list<rgw_obj_key> *remove_objs,
uint16_t bilog_flags)
{
RGWObjEnt ent;
ent.mtime = removed_mtime;
obj.get_index_key(&ent.key);
return cls_obj_complete_op(bs, CLS_RGW_OP_DEL, tag, pool, epoch, ent, RGW_OBJ_CATEGORY_NONE, remove_objs, bilog_flags);
}
Expand Down
3 changes: 2 additions & 1 deletion src/rgw/rgw_rados.h
View file
Expand Up @@ -2374,6 +2374,7 @@ class RGWRados
bufferlist *acl_bl, RGWObjCategory category,
list<rgw_obj_key> *remove_objs);
int complete_del(int64_t poolid, uint64_t epoch,
ceph::real_time& removed_mtime, /* mtime of removed object */
list<rgw_obj_key> *remove_objs);
int cancel();
};
Expand Down Expand Up @@ -2753,7 +2754,7 @@ class RGWRados
int cls_obj_complete_add(BucketShard& bs, string& tag, int64_t pool, uint64_t epoch, RGWObjEnt& ent,
RGWObjCategory category, list<rgw_obj_key> *remove_objs, uint16_t bilog_flags);
int cls_obj_complete_del(BucketShard& bs, string& tag, int64_t pool, uint64_t epoch, rgw_obj& obj,
list<rgw_obj_key> *remove_objs, uint16_t bilog_flags);
ceph::real_time& removed_mtime, list<rgw_obj_key> *remove_objs, uint16_t bilog_flags);
int cls_obj_complete_cancel(BucketShard& bs, string& tag, rgw_obj& obj, uint16_t bilog_flags);
int cls_obj_set_bucket_tag_timeout(rgw_bucket& bucket, uint64_t timeout);
int cls_bucket_list(rgw_bucket& bucket, int shard_id, rgw_obj_key& start, const string& prefix,
Expand Down
2 changes: 1 addition & 1 deletion src/rgw/rgw_rest_client.cc
View file
Expand Up @@ -692,7 +692,7 @@ int RGWRESTStreamRWRequest::get_resource(RGWAccessKey& key, map<string, string>&
int RGWRESTStreamRWRequest::complete(string& etag, real_time *mtime, map<string, string>& attrs)
{
set_str_from_headers(out_headers, "ETAG", etag);
if (status > 0 && mtime) {
if (status >= 0 && mtime) {
string mtime_str;
set_str_from_headers(out_headers, "RGWX_MTIME", mtime_str);
if (!mtime_str.empty()) {
Expand Down

0 comments on commit 4ead0f9

Please sign in to comment.