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

osd: don't use ORDERSNAP for flush; always request/send ondisk ack #13570

Merged
merged 2 commits into from Feb 24, 2017

Conversation

Projects
None yet
4 participants

@athanatos athanatos requested review from liewegas and jdurgin Feb 21, 2017

@liewegas liewegas changed the title from Wip 18937 to osd: don't use ORDERSNAP for flush; always request/send ondisk ack Feb 22, 2017

@jdurgin jdurgin added the needs-qa label Feb 24, 2017

athanatos added some commits Feb 15, 2017

PrimaryLogPG::start_flush: don't use ORDERSNAP, eliminate the second …
…delete

I think that whole thing was a misguided attempt to avoid deleting head
if it exists in the base tier (in reality it doesn't matter since head
would have to be logically dirty and anything we actually care about
would be preserved by sending a new enough seq to cause a clone).

Introduced in 4843fd5, but the real
logical error happened in f3df501.

I suggest never backporting this patch.  If you want to try, keep in
mind that the last version didn't turn up as busted for 2 years.

Fixes: f3df501
Signed-off-by: Samuel Just <sjust@redhat.com>
osd,osdc: eliminate FLAG_ONDISK and helpers
The objecter actually always needs to get a response in order to
be able to not continually resend ops (even if the caller didn't
provide a callback).  Thus, it makes no sense for an MOSDOp to
ever not have FLAG_ONDISK set.  Therefore, we'll just remove the
helper and assume it's always there (it's safe to send a response
the client didn't ask for, the error paths already do that).  On
the Objecter side, we'll just unconditionally fill in ONDISK for
the benefit of pre-luminous OSDs.

Fixes: http://tracker.ceph.com/issues/18961
Signed-off-by: Samuel Just <sjust@redhat.com>

@athanatos athanatos merged commit 4f856fe into ceph:master Feb 24, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
// Nothing checks this any longer, but needed for compatibility with
// pre-luminous osds
flags |= CEPH_OSD_FLAG_ONDISK;

This comment has been minimized.

@liupan1111

liupan1111 Mar 26, 2017

Contributor

Could you talk more about this change? for read ops, is it neccesary to wait for ondisk ack?

This comment has been minimized.

@liewegas

liewegas Mar 26, 2017

Member
  1. there is now only one kind of reply (there used to be 'ack' and 'ondisk' replies)
  2. this changes makes it so we always request a reply for an operation. this particular bug was that we didn't request an ack, which meant that the client would never close it out and we would resend the request later when the pg moved.
  3. the other patches simplify cache flushing, which was originally more complicated because it was working around the bug (before we understood what was going on).
@liupan1111

This comment has been minimized.

Contributor

liupan1111 commented Mar 26, 2017

@athanatos @jdurgin Could you give me some info?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment