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: fix handling of ENOENT in RGWRadosGetOmapKeysCR #19878

Merged
merged 2 commits into from
Jan 16, 2018
Merged

rgw: fix handling of ENOENT in RGWRadosGetOmapKeysCR #19878

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
2f94f63
rgw: RGWRadosGetOmapKeysCR uses completion return code
cbodley Jan 9, 2018
0ccf50f
rgw: RGWRadosGetOmapKeysCR uses omap_get_keys2
cbodley Jan 9, 2018
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
15 changes: 12 additions & 3 deletions src/rgw/rgw_cr_rados.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,10 @@ int RGWRadosSetOmapKeysCR::request_complete()
RGWRadosGetOmapKeysCR::RGWRadosGetOmapKeysCR(RGWRados *_store,
const rgw_raw_obj& _obj,
const string& _marker,
map<string, bufferlist> *_entries, int _max_entries) : RGWSimpleCoroutine(_store->ctx()),
std::set<std::string> *_entries, int _max_entries) : RGWSimpleCoroutine(_store->ctx()),
store(_store),
marker(_marker),
entries(_entries), max_entries(_max_entries), rval(0),
entries(_entries), max_entries(_max_entries),
obj(_obj), cn(NULL)
{
set_description() << "get omap keys dest=" << obj << " marker=" << marker;
Expand All @@ -268,12 +268,21 @@ int RGWRadosGetOmapKeysCR::send_request() {
set_status() << "send request";

librados::ObjectReadOperation op;
op.omap_get_vals2(marker, max_entries, entries, nullptr, &rval);
op.omap_get_keys2(marker, max_entries, entries, nullptr, nullptr);

cn = stack->create_completion_notifier();
return ref.ioctx.aio_operate(ref.oid, cn->completion(), &op, NULL);
}

int RGWRadosGetOmapKeysCR::request_complete()
{
int r = cn->completion()->get_return_value();

set_status() << "request complete; ret=" << r;
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM, I have searched aio_operate in RGW code to check whether there is similar problem. It seems that all async rados CR(RGWRadosSetOmapKeysCR, RGWRadosRemoveOmapKeysCR, RGWRadosRemoveCR, RGWRadosBILogTrimCR) already use cn->completion()->get_return_value().

maybe we should create a base class, avoid those duplicated code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe we should create a base class, avoid those duplicated code?

that sounds reasonable, yeah. it would help distinguish the ops that use aio_operate vs async_rados


return r;
}

RGWRadosRemoveOmapKeysCR::RGWRadosRemoveOmapKeysCR(RGWRados *_store,
const rgw_raw_obj& _obj,
const set<string>& _keys) : RGWSimpleCoroutine(_store->ctx()),
Expand Down
10 changes: 3 additions & 7 deletions src/rgw/rgw_cr_rados.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,9 @@ class RGWRadosGetOmapKeysCR : public RGWSimpleCoroutine {
RGWRados *store;

string marker;
map<string, bufferlist> *entries;
std::set<std::string> *entries;
int max_entries;

int rval;
rgw_rados_ref ref;

rgw_raw_obj obj;
Expand All @@ -424,13 +423,10 @@ class RGWRadosGetOmapKeysCR : public RGWSimpleCoroutine {
RGWRadosGetOmapKeysCR(RGWRados *_store,
const rgw_raw_obj& _obj,
const string& _marker,
map<string, bufferlist> *_entries, int _max_entries);
std::set<std::string> *_entries, int _max_entries);

int send_request() override;

int request_complete() override {
return rval;
}
int request_complete() override;
};

class RGWRadosRemoveOmapKeysCR : public RGWSimpleCoroutine {
Expand Down
22 changes: 11 additions & 11 deletions src/rgw/rgw_data_sync.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1093,8 +1093,8 @@ class RGWDataSyncShardCR : public RGWCoroutine {
uint32_t shard_id;
rgw_data_sync_marker sync_marker;

map<string, bufferlist> entries;
map<string, bufferlist>::iterator iter;
std::set<std::string> entries;
std::set<std::string>::iterator iter;

string oid;

Expand Down Expand Up @@ -1135,7 +1135,7 @@ class RGWDataSyncShardCR : public RGWCoroutine {

string error_oid;
RGWOmapAppend *error_repo;
map<string, bufferlist> error_entries;
std::set<std::string> error_entries;
string error_marker;
int max_error_entries;

Expand Down Expand Up @@ -1267,20 +1267,20 @@ class RGWDataSyncShardCR : public RGWCoroutine {
tn->log(20, SSTR("retrieved " << entries.size() << " entries to sync"));
iter = entries.begin();
for (; iter != entries.end(); ++iter) {
tn->log(20, SSTR("full sync: " << iter->first));
tn->log(20, SSTR("full sync: " << *iter));
total_entries++;
if (!marker_tracker->start(iter->first, total_entries, real_time())) {
tn->log(0, SSTR("ERROR: cannot start syncing " << iter->first << ". Duplicate entry?"));
if (!marker_tracker->start(*iter, total_entries, real_time())) {
tn->log(0, SSTR("ERROR: cannot start syncing " << *iter << ". Duplicate entry?"));
} else {
// fetch remote and write locally
yield spawn(new RGWDataSyncSingleEntryCR(sync_env, iter->first, iter->first, marker_tracker, error_repo, false, tn), false);
yield spawn(new RGWDataSyncSingleEntryCR(sync_env, *iter, *iter, marker_tracker, error_repo, false, tn), false);
if (retcode < 0) {
lease_cr->go_down();
drain_all();
return set_cr_error(retcode);
}
}
sync_marker.marker = iter->first;
sync_marker.marker = *iter;
}
} while ((int)entries.size() == max_entries);

Expand Down Expand Up @@ -1353,9 +1353,9 @@ class RGWDataSyncShardCR : public RGWCoroutine {
tn->log(20, SSTR("read error repo, got " << error_entries.size() << " entries"));
iter = error_entries.begin();
for (; iter != error_entries.end(); ++iter) {
tn->log(20, SSTR("handle error entry: " << iter->first));
spawn(new RGWDataSyncSingleEntryCR(sync_env, iter->first, iter->first, nullptr /* no marker tracker */, error_repo, true, tn), false);
error_marker = iter->first;
error_marker = *iter;
tn->log(20, SSTR("handle error entry: " << error_marker));
spawn(new RGWDataSyncSingleEntryCR(sync_env, error_marker, error_marker, nullptr /* no marker tracker */, error_repo, true, tn), false);
}
if ((int)error_entries.size() != max_error_entries) {
if (error_marker.empty() && error_entries.empty()) {
Expand Down
18 changes: 9 additions & 9 deletions src/rgw/rgw_sync.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1360,8 +1360,8 @@ class RGWMetaSyncShardCR : public RGWCoroutine {
string max_marker;
const std::string& period_marker; //< max marker stored in next period

map<string, bufferlist> entries;
map<string, bufferlist>::iterator iter;
std::set<std::string> entries;
std::set<std::string>::iterator iter;

string oid;

Expand Down Expand Up @@ -1562,20 +1562,20 @@ class RGWMetaSyncShardCR : public RGWCoroutine {
}
iter = entries.begin();
for (; iter != entries.end(); ++iter) {
tn->log(20, SSTR("full sync: " << iter->first));
marker = *iter;
tn->log(20, SSTR("full sync: " << marker));
total_entries++;
if (!marker_tracker->start(iter->first, total_entries, real_time())) {
tn->log(0, SSTR("ERROR: cannot start syncing " << iter->first << ". Duplicate entry?"));
if (!marker_tracker->start(marker, total_entries, real_time())) {
tn->log(0, SSTR("ERROR: cannot start syncing " << marker << ". Duplicate entry?"));
} else {
// fetch remote and write locally
yield {
RGWCoroutinesStack *stack = spawn(new RGWMetaSyncSingleEntryCR(sync_env, iter->first, iter->first, MDLOG_STATUS_COMPLETE, marker_tracker, tn), false);
RGWCoroutinesStack *stack = spawn(new RGWMetaSyncSingleEntryCR(sync_env, marker, marker, MDLOG_STATUS_COMPLETE, marker_tracker, tn), false);
// stack_to_pos holds a reference to the stack
stack_to_pos[stack] = iter->first;
pos_to_prev[iter->first] = marker;
stack_to_pos[stack] = marker;
pos_to_prev[marker] = marker;
}
}
marker = iter->first;
}
collect_children();
} while ((int)entries.size() == max_entries && can_adjust_marker);
Expand Down