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: moved OpFinisher logic from OSDOp to OpContext #16617

Merged
merged 2 commits into from Jul 28, 2017

Conversation

Projects
None yet
6 participants
@dillaman
Contributor

dillaman commented Jul 27, 2017

This allows the memory lifetime of the OpFinisher to be tightly
controlled since the OpContext cannot be copied.

Fixes: http://tracker.ceph.com/issues/20783
Signed-off-by: Jason Dillaman dillaman@redhat.com

@dillaman dillaman added this to the luminous milestone Jul 27, 2017

@dillaman

This comment has been minimized.

}
int execute() override {
// instance will be destructed after after this method completes

This comment has been minimized.

@tchaikov

tchaikov Jul 27, 2017

Contributor

s/after after/after/

osd: moved OpFinisher logic from OSDOp to OpContext
This allows the memory lifetime of the OpFinisher to be tightly
controlled since the OpContext cannot be copied.

Fixes: http://tracker.ceph.com/issues/20783
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
@dillaman

This comment has been minimized.

Contributor

dillaman commented Jul 27, 2017

typo fixed

osd: silence coverity warning in async read operation contexts
Coverity doesn't know that the context is deleted via the "complete"
method and the embedded context is therefore also destructed when
it's completed.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
@dillaman

This comment has been minimized.

Contributor

dillaman commented Jul 27, 2017

appended a commit to silence coverity

@jdurgin jdurgin merged commit 0624393 into ceph:master Jul 28, 2017

3 of 4 checks passed

make check (arm64) make check failed
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details

@dillaman dillaman deleted the dillaman:wip-20783 branch Jul 28, 2017

@@ -5007,10 +5005,19 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
dout(10) << "do_osd_op " << soid << " " << ops << dendl;
ctx->current_osd_subop_num = 0;

This comment has been minimized.

@cbodley

cbodley Oct 28, 2017

Contributor

@dillaman it looks like this line is causing a regression in ceph_test_cls_log:

[ RUN      ] cls_rgw.test_log_add_same_time
/home/cbodley/ceph/src/test/cls_log/test_cls_log.cc:144: Failure
      Expected: 10
To be equal to: (int)entries.size()
      Which is: 1
[  FAILED  ] cls_rgw.test_log_add_same_time (1180 ms)

could you please help review #18610, which removes this? that fixes the test, but i'm not sure whether this line belongs somewhere else instead

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