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 ECANCELED handling in rgw_get_system_obj() #12225

Closed
wants to merge 1 commit into from
Closed

rgw: fix ECANCELED handling in rgw_get_system_obj() #12225

wants to merge 1 commit into from

Conversation

yehudasa
Copy link
Member

@yehudasa yehudasa commented Nov 29, 2016 •
edited

@yehudasa yehudasa mentioned this pull request Nov 29, 2016
Closed
@theanalyst
Copy link
Member

there is an accidental submodule change in the pr

@robbat2
Copy link
Contributor

robbat2 commented Mar 30, 2017

@yehudasa DreamObjects has been running this patch on top of Jewel without the submodule piece for several months now, please included in master & 10.2.7.

@theanalyst
Copy link
Member

@yehudasa ping

amitkumar50
amitkumar50 reviewed Aug 23, 2017
View reviewed changes
src/rgw/rgw_tools.cc
@@ -47,6 +47,11 @@ int rgw_get_system_obj(RGWRados *rgwstore, RGWObjectCtx& obj_ctx, rgw_bucket& bu
int request_len = READ_CHUNK_LEN;
rgw_obj obj(bucket, key);

obj_version original_readv;
Copy link

Choose a reason for hiding this comment

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

  1. Let's initialize original_readv.

  2. if (!original_readv.empty())) //String is empty=> .empty() return 0
    {
    string empty; return;
    }

    Just thinking, Why can't we return from
    if (objv_tracker && !objv_tracker->read_version.empty()) {
    original_readv = objv_tracker->read_version;
    <<<<return (value)>>>>
    }
    This will save processing code inside do-while before rop.read().

@robbat2
Copy link
Contributor

robbat2 commented Oct 6, 2017

@yehudasa ping, please merge! Dreamhost has been running this for many months.

@oritwas
Copy link
Member

oritwas commented Oct 7, 2017

jenkins test this please

@oritwas
Copy link
Member

oritwas commented Oct 9, 2017

@yehudasa , @robbat2 this was merged to master as part of #11485

@robbat2
Copy link
Contributor

robbat2 commented Oct 9, 2017

@oritwas clarification: I'm waiting for it to merge to Jewel

@yehudasa yehudasa changed the base branch from master to jewel October 9, 2017 17:26
@yehudasa yehudasa changed the base branch from jewel to master October 9, 2017 17:26
@yehudasa
Copy link
Member Author

yehudasa commented Oct 9, 2017

@robbat2 can you send a PR against jewel? rebasing this one doesn't work.

robbat2 pushed a commit to dreamhost/ceph that referenced this pull request Oct 11, 2017
@yehudasa
rgw: fix ECANCELED handling in rgw_get_system_obj()
72d8d9e 
Fixes: http://tracker.ceph.com/issues/17996
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
See-Also: ceph#12225 (comment)
(cherry picked from commit d3d64ed)
@robbat2 robbat2 mentioned this pull request Oct 11, 2017
Closed
@tchaikov
Copy link
Contributor

retest this please.

@tchaikov
Copy link
Contributor 

In file included from /home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos7/DIST/centos7/MACHINE_SIZE/huge/release/13.0.0-3542-gd33628d/rpm/el7/BUILD/ceph-13.0.0-3542-gd33628d/src/test/rgw/test_rgw_common.h:18:0,
                 from /home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos7/DIST/centos7/MACHINE_SIZE/huge/release/13.0.0-3542-gd33628d/rpm/el7/BUILD/ceph-13.0.0-3542-gd33628d/src/test/rgw/test_rgw_common.cc:1:
/home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos7/DIST/centos7/MACHINE_SIZE/huge/release/13.0.0-3542-gd33628d/rpm/el7/BUILD/ceph-13.0.0-3542-gd33628d/src/rgw/rgw_rados.h:2670:10: error: 'void RGWRados::SystemObject::invalidate_state()' cannot be overloaded
     void invalidate_state();
          ^
/home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos7/DIST/centos7/MACHINE_SIZE/huge/release/13.0.0-3542-gd33628d/rpm/el7/BUILD/ceph-13.0.0-3542-gd33628d/src/rgw/rgw_rados.h:2668:10: error: with 'void RGWRados::SystemObject::invalidate_state()'
     void invalidate_state();
          ^

this PR does not build

@tchaikov
Copy link
Contributor

@oritwas @yehudasa so it has been merge as e67a3b5 , shall we close this PR ?

@liewegas liewegas closed this Dec 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amitkumar50 amitkumar50 amitkumar50 left review comments

@oritwas oritwas oritwas approved these changes

Assignees
No one assigned
Labels
bug-fix rgw
Projects
None yet
Milestone
No milestone
7 participants
@yehudasa @theanalyst @robbat2 @oritwas @tchaikov @amitkumar50 @liewegas