nautilus: rgw: Use correct bucket info when put or get large object with swift. #40106

xijiacun · 2021-03-15T03:08:58Z

backport tracker: https://tracker.ceph.com/issues/50836

backport of #40296

parent tracker: https://tracker.ceph.com/issues/49791

Fixes: https://tracker.ceph.com/issues/49791

Signed-off-by: zhiming zhang zhangzhm1@chinatelecom.cn
Signed-off-by: yupeng chen chenyupeng@chinatelecom.cn

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

bel5f5 · 2021-03-15T03:36:26Z

@yehudasa Please help to review the code.

tchaikov · 2021-03-15T03:43:41Z

@xijiacun @bel5f5 does this issue exist in master or any of the LTS branches? if yes, probably we should fix it in master first. and backport it to nautilus (and other LTS branches)?

xijiacun · 2021-03-17T09:55:40Z

@xijiacun @bel5f5 does this issue exist in master or any of the LTS branches? if yes, probably we should fix it in master first. and backport it to nautilus (and other LTS branches)?

This issue does not exist in master, but exist in N and O branches.

xijiacun · 2021-03-17T09:56:45Z

@xijiacun @bel5f5 does this issue exist in master or any of the LTS branches? if yes, probably we should fix it in master first. and backport it to nautilus (and other LTS branches)?

@tchaikov

xijiacun · 2021-03-18T07:44:18Z

@xijiacun @bel5f5 does this issue exist in master or any of the LTS branches? if yes, probably we should fix it in master first. and backport it to nautilus (and other LTS branches)?

This issue does not exist in master, but exist in N and O branches.

@tchaikov

cbodley · 2021-03-18T14:02:29Z

src/rgw/rgw_rest_swift.cc

@@ -858,8 +859,10 @@ int RGWPutObj_ObjStore_SWIFT::update_slo_segment_size(rgw_slo_entry& entry) {
      return r;
    }
    bucket = bucket_info.bucket;
+    pbucket_info = &bucket_info;


this is storing a pointer to RGWBucketInfo bucket_info; which goes out of scope on the next line

OK, I fixed it in pr to octopus, #40296,

smithfarm · 2021-03-22T10:57:41Z

@xijiacun The standard procedure [1] in this case would be:

open fix targeting octopus - you already did that here: octopus: rgw: Use correct bucket info when put or get large object with swift #40296
wait for the octopus fix to be reviewed, tested and merged
cherry-pick the octopus fix to nautilus using git cherry-pick -x

Would that be OK for you?

[1] https://github.com/ceph/ceph/blob/master/SubmittingPatches-backports.rst

tchaikov · 2021-05-17T02:37:56Z

@xijiacun sorry for the latency. i am not qualified for reviewing RGW changes like your fix. so i'd defer the review to developers who have better understanding of RGW.

@smithfarm i am removing the DNM label, since the octopus fix has been merged.

smithfarm

@xijiacun When I tried cherry-picking b6bf107 to nautilus, I got a conflict. Did you also?

The reason I ask is I noticed the commit message in this PR does not have a "Conflicts" section, which ordinarily means the cherry-pick applied cleanly (?).

When you run git cherry-pick -x and need to resolve conflicts, git generates this "Conflicts" section when it generates the commit message. Unfortunately, as of a certain git version it also started commenting-out the Conflicts section that it generates, so now it's easy to forget to uncomment it.

For more information, see https://github.com/ceph/ceph/blob/master/SubmittingPatches-backports.rst#cherry-picking-rules where it says:

the commit message generated by git cherry-pick -x must not be modified, except to add a "Conflicts" section below the "cherry picked from commit ..." line added by git
the "Conflicts" section must mention all files where changes had to be made manually (not just conflicts flagged by git)
the "Conflicts" section should also describe the manual changes that were made

and https://github.com/ceph/ceph/blob/master/SubmittingPatches-backports.rst#conflict-resolution which presents an example illustrating what a Conflicts section looks like.

xijiacun · 2021-05-18T00:39:46Z

@xijiacun When I tried cherry-picking b6bf107 to nautilus, I got a conflict. Did you also?

The reason I ask is I noticed the commit message in this PR does not have a "Conflicts" section, which ordinarily means the cherry-pick applied cleanly (?).

When you run git cherry-pick -x and need to resolve conflicts, git generates this "Conflicts" section when it generates the commit message. Unfortunately, as of a certain git version it also started commenting-out the Conflicts section that it generates, so now it's easy to forget to uncomment it.

For more information, see https://github.com/ceph/ceph/blob/master/SubmittingPatches-backports.rst#cherry-picking-rules where it says:

the commit message generated by git cherry-pick -x must not be modified, except to add a "Conflicts" section below the "cherry picked from commit ..." line added by git

the "Conflicts" section must mention all files where changes had to be made manually (not just conflicts flagged by git)

the "Conflicts" section should also describe the manual changes that were made

and https://github.com/ceph/ceph/blob/master/SubmittingPatches-backports.rst#conflict-resolution which presents an example illustrating what a Conflicts section looks like.

yes, there is a conflict. I cherry-pick again to add conflicts section. @smithfarm

Fixes: https://tracker.ceph.com/issues/49791 Signed-off-by: zhiming zhang <zhangzhm1@chinatelecom.cn> Signed-off-by: yupeng chen <chenyupeng@chinatelecom.cn> (cherry picked from commit bdd0635) Conflicts: src/rgw/rgw_op.cc src/rgw/rgw_rest_swift.cc -In octopus: - RGWRados::Object op_target(store->getRados(), ...) -In nautilus: - RGWRados::Object op_target(store, ...)

The commit message looks good to me now, thank you!

github-actions bot added this to the nautilus milestone Mar 15, 2021

xijiacun closed this Mar 17, 2021

xijiacun deleted the nautilus branch March 17, 2021 09:56

xijiacun restored the nautilus branch March 17, 2021 09:57

xijiacun reopened this Mar 17, 2021

xijiacun closed this Mar 18, 2021

xijiacun reopened this Mar 18, 2021

xijiacun closed this Mar 18, 2021

xijiacun reopened this Mar 18, 2021

cbodley reviewed Mar 18, 2021

View reviewed changes

smithfarm changed the title ~~rgw: Use correct bucket info when put or get large object with swift.~~ nautilus: rgw: Use correct bucket info when put or get large object with swift. Mar 22, 2021

smithfarm added the rgw label Mar 22, 2021

smithfarm added the DNM label Mar 22, 2021

jdurgin changed the base branch from nautilus to nautilus-saved March 31, 2021 00:14

jdurgin changed the base branch from nautilus-saved to nautilus March 31, 2021 00:14

yuriw force-pushed the nautilus branch from 533a855 to 40278cb Compare April 12, 2021 17:28

jdurgin force-pushed the nautilus branch from 40278cb to 56c38c6 Compare April 12, 2021 18:41

idryomov force-pushed the nautilus branch from 533a855 to ef654b4 Compare April 14, 2021 12:42

idryomov changed the base branch from nautilus to nautilus-saved April 20, 2021 18:23

idryomov changed the base branch from nautilus-saved to nautilus April 20, 2021 18:23

liewegas force-pushed the nautilus branch from 98b8bc5 to 71b14f0 Compare May 13, 2021 15:15

jdurgin changed the base branch from nautilus to nautilus-saved May 14, 2021 21:59

jdurgin changed the base branch from nautilus-saved to nautilus May 14, 2021 21:59

xijiacun closed this May 17, 2021

xijiacun force-pushed the nautilus branch from e1b7527 to 0be5ed5 Compare May 17, 2021 02:10

xijiacun reopened this May 17, 2021

tchaikov added nautilus-batch-1 nautilus point releases needs-qa and removed DNM labels May 17, 2021

smithfarm removed nautilus-batch-1 nautilus point releases needs-qa labels May 17, 2021

smithfarm previously requested changes May 17, 2021

View reviewed changes

xijiacun closed this May 18, 2021

xijiacun force-pushed the nautilus branch from b6bf107 to 0be5ed5 Compare May 18, 2021 00:44

xijiacun reopened this May 18, 2021

xijiacun closed this May 18, 2021

xijiacun reopened this May 18, 2021

smithfarm added nautilus-batch-1 nautilus point releases needs-qa labels May 18, 2021

yuriw added the wip-yuri3-testing label May 19, 2021

cbodley approved these changes May 20, 2021

View reviewed changes

yuriw merged commit 833da4c into ceph:nautilus May 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nautilus: rgw: Use correct bucket info when put or get large object with swift. #40106

nautilus: rgw: Use correct bucket info when put or get large object with swift. #40106

xijiacun commented Mar 15, 2021 •

edited by smithfarm

Loading

bel5f5 commented Mar 15, 2021

tchaikov commented Mar 15, 2021 •

edited

Loading

xijiacun commented Mar 17, 2021

xijiacun commented Mar 17, 2021

xijiacun commented Mar 18, 2021

cbodley Mar 18, 2021

xijiacun Mar 22, 2021

smithfarm commented Mar 22, 2021

tchaikov commented May 17, 2021

smithfarm left a comment •

edited

Loading

xijiacun commented May 18, 2021

nautilus: rgw: Use correct bucket info when put or get large object with swift. #40106

nautilus: rgw: Use correct bucket info when put or get large object with swift. #40106

Conversation

xijiacun commented Mar 15, 2021 • edited by smithfarm Loading

Checklist

bel5f5 commented Mar 15, 2021

tchaikov commented Mar 15, 2021 • edited Loading

xijiacun commented Mar 17, 2021

xijiacun commented Mar 17, 2021

xijiacun commented Mar 18, 2021

cbodley Mar 18, 2021

Choose a reason for hiding this comment

xijiacun Mar 22, 2021

Choose a reason for hiding this comment

smithfarm commented Mar 22, 2021

tchaikov commented May 17, 2021

smithfarm left a comment • edited Loading

Choose a reason for hiding this comment

xijiacun commented May 18, 2021

xijiacun commented Mar 15, 2021 •

edited by smithfarm

Loading

tchaikov commented Mar 15, 2021 •

edited

Loading

smithfarm left a comment •

edited

Loading