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.

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

Repop and lost-unfound overhaul #7765

Merged
merged 23 commits into from Feb 29, 2016
Merged

Conversation

athanatos
Copy link
Contributor

mark_lost_revert/delete can race dangerously with normal IO due to not being ordered with other repops. This series generalizes the repop ordering machinery and introduces a new message for performing mark_unfound_revert/delete.

@athanatos
Copy link
Contributor Author

qa-suite PR: ceph/ceph-qa-suite#832

Test run: samuelj-2016-02-22_06:57:34-rados-wip-sam-working-distro-basic-smithi

Ready for merge.

RepGather *simple_repop_create(ObjectContextRef obc);
void simple_repop_submit(RepGather *repop);
OpContextUPtr simple_opc_create(ObjectContextRef obc);
void simple_opc_submit(OpContextUPtr ctx);
Copy link
Member

Choose a reason for hiding this comment

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

all of the other methods use 'ctx' to mean OpContext (e.g., finish_ctx).. any specific reason to use opc here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two are sort of external interfaces to the other ctx methods. I could switch it if you feel strongly.

@athanatos athanatos changed the title Repop and lost-unfound overhaul DNM (for the moment): Repop and lost-unfound overhaul Feb 23, 2016
@@ -1415,6 +1417,23 @@ class ObjectStore {
}
data.ops++;
}
void try_rename(coll_t cid, const ghobject_t& oldoid,
const ghobject_t& oid) {
if (use_tbl) {
Copy link
Member

Choose a reason for hiding this comment

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

do we already have a barrier feature prevent "use_tbl" is true?

@athanatos athanatos force-pushed the wip-lost-unfound-repop branch 2 times, most recently from c6c9876 to b3c6117 Compare February 24, 2016 17:44
Signed-off-by: Samuel Just <sjust@redhat.com>
This way, we don't expose the RepGather structure
up into the users (which pretty much exclusively
use the repop->ctx member anyway).  This will pave
the way to removing RepGather::ctx.

Part of this involves generalizing the repop callback
members (queue_snap_trimmer and on_applied) to
on_applied, on_committed, on_success, and on_finish
on both OpContext and RepGather.

Signed-off-by: Samuel Just <sjust@redhat.com>
execute_ctx is the only path which actually has a client op,
let's clarify eval_repop by moving that logic inline there.
This also eliminates most of the repop->ctx users from
eval_repop.

Signed-off-by: Samuel Just <sjust@redhat.com>
We'll need this shortly when we remove do_osd_op_effects from
eval_repop.

Signed-off-by: Samuel Just <sjust@redhat.com>
This is better anyway, do_osd_op_effects on a non-client op was somewhat
silly without an op (even if you filled in the other fields like
watch_connects, it would just silently skip it).  It also eliminates
another dependence of RepGather on OpContext.

Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
…evict

Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
…saction

submit_transaction takes ownership of the transaction implicitely.  Make
this implicit using an rvalue ref to force usage of std::move and move
constructor.

Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
…opcontext

This gets us one step closer to removing OpContext from Repop.

Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
We want these to be processed ahead of new client ops since
there are resources being held.

Fixes: 14313
Backport: hammer
Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
This way, we can get proper logging within static functions.

Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
This should not change behavior at all.  It'll be useful in
the next few commits.

Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
@athanatos athanatos force-pushed the wip-lost-unfound-repop branch 2 times, most recently from 502fbe5 to 8938eaa Compare February 25, 2016 19:10
Similar to collection_move_rename, except we ignore ENOENT and
don't allow different collections.

Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
…e safely

Using a MOSDPGLog was unsafe since it is not ordered with
respect to repops.  Instead, use a new message sent through
the same paths as repops.

Signed-off-by: Samuel Just <sjust@redhat.com>
@athanatos athanatos changed the title DNM (for the moment): Repop and lost-unfound overhaul Repop and lost-unfound overhaul Feb 29, 2016
@athanatos
Copy link
Contributor Author

Passed tests after rebase:
sjust@teuthology:/a/samuelj-2016-02-27_00:36:36-rados-wip-sam-working-distro-basic-smithi

@liewegas Ok to merge?

@liewegas
Copy link
Member

yep!

On Mon, 29 Feb 2016, Samuel Just wrote:

Passed tests after rebase:
sjust@teuthology:/a/samuelj-2016-02-27_00:36:36-rados-wip-sam-working-distro-basic-smithi

@liewegas Ok to merge?


Reply to this email directly or view it on GitHub.[AAabhwZlARaqU6Ztq9GV4U5ESZOiupWdks5pow5GgaJpZM4Hg8iZ.gif]

pinfo.last_update = info.last_update;
pinfo.stats.stats_invalid = true;
}
for (auto &&i: entries) {
Copy link
Contributor

Choose a reason for hiding this comment

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

&& does not make i a rvalue reference.

athanatos added a commit that referenced this pull request Feb 29, 2016
Repop and lost-unfound overhaul

Reviewed-by: Sage Weil <sage@redhat.com>
@athanatos athanatos merged commit 306c7bf into ceph:master Feb 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants