rgw: add the support for X-History-Location in swift API by Jing-Scott · Pull Request #15020 · ceph/ceph

Jing-Scott · 2017-05-10T02:22:00Z

The feature is described at https://github.com/openstack/swift/blob/2.12.0/swift/common/middleware/versioned_writes.py#L23-L30

Signed-off-by: Jing Wenjun [email protected]

Jing-Scott · 2017-05-10T15:27:46Z

Hi, @rzarzynski , can you help to have a look about this feature?

Jing-Scott · 2017-05-11T06:09:13Z

src/rgw/rgw_rest_swift.cc

-  if (s->info.env->exists("HTTP_X_VERSIONS_LOCATION")) {
+  bool ver_exists = s->info.env->exists("HTTP_X_VERSIONS_LOCATION");
+  bool his_exists = s->info.env->exists("HTTP_X_HISTORY_LOCATION");
+  if (ver_exists && his_exists) {


add this case

rzarzynski · 2017-06-12T12:57:57Z

src/rgw/rgw_common.h

       ::decode(swift_versioning, bl);
       if (swift_versioning) {
         ::decode(swift_ver_location, bl);
+         ::decode(swift_his_location, bl);


I think this should be performed only for the version 18 and above.

@Jing-Scott swift_ver_location shouldn't require version 18, right?

Signed-off-by: Jing Wenjun <[email protected]>

The feature is described at https://github.com/openstack/swift/blob/2.12.0/swift/common/middleware/versioned_writes.py#L23-L30 Signed-off-by: Jing Wenjun <[email protected]>

Jing-Scott · 2017-06-13T16:49:24Z

jekins test this please

Jing-Scott · 2017-06-14T07:07:46Z

@rzarzynski i have rebased and changed to version 18.

rzarzynski · 2017-06-25T10:22:58Z

src/rgw/rgw_common.h

     ::encode(swift_versioning, bl);
     if (swift_versioning) {
       ::encode(swift_ver_location, bl);
+       ::encode(swift_his_location, bl);


To preserve compatibility with older struct versions, we should encode new fields at the end, not in the middle. This is required during e.g. cluster upgrades when many instances of RadosGW, possibly from two different releases, may coexist in a single cluster.

@rzarzynski seems to be fixed now?

rzarzynski · 2017-06-25T10:25:12Z

src/rgw/rgw_common.h

+       ::decode(mdsearch_config, bl);
       if (swift_versioning) {
         ::decode(swift_ver_location, bl);
+         ::decode(swift_his_location, bl);


This field comes with version 20th, not 18th.

stale · 2018-10-18T10:54:22Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

mattbenjamin

@Jing-Scott I would like to encorporate these improvements, but I think there are problems remaining, e.g., the struct versioning decoding issues @rzarzynski pointed out; if you are willing to update, that would be great!

mattbenjamin · 2017-05-31T20:43:10Z

src/rgw/rgw_rados.cc

+  const string& src_name = src_obj.get_oid();
+  char buf[src_name.size() + 32];
+  struct timespec ts = ceph::real_clock::to_timespec(ceph::real_clock::now());
+  snprintf(buf, sizeof(buf), "%03x%s/%lld.%06ld", (int)src_name.size(),


could we use boost::format() boost::str() idioms here?

mattbenjamin · 2018-11-01T15:37:40Z

src/rgw/rgw_common.h

     ::encode(swift_versioning, bl);
     if (swift_versioning) {
       ::encode(swift_ver_location, bl);
+       ::encode(swift_his_location, bl);


@rzarzynski seems to be fixed now?

mattbenjamin · 2018-11-01T15:39:09Z

src/rgw/rgw_common.h

       ::decode(swift_versioning, bl);
       if (swift_versioning) {
         ::decode(swift_ver_location, bl);
+         ::decode(swift_his_location, bl);


@Jing-Scott swift_ver_location shouldn't require version 18, right?

rzarzynski · 2018-11-03T15:01:17Z

src/rgw/rgw_common.h

     swift_ver_location.clear();
-     if (struct_v >= 16) {
+     swift_his_location.clear();
+     if (struct_v >= 18) {


Something weird happened here. IIUC swift_versioning and swift_ver_location can be present since struct_v 16th but now we expect it only since 18th. I believe we shouldn't touch this but instead introduce:

swift_his_location.clear(); if (struct_v >= 20 && swift_versioning) { ::decode(swift_his_location, bl); }

just before DECODE_FINISH(bl).

thanks, @rzarzynski !

stale · 2019-02-11T09:19:56Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

stale · 2019-05-12T10:13:57Z

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

mattbenjamin · 2019-05-12T19:03:59Z

reopen for re-review

stale · 2019-07-11T19:40:40Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

stale · 2019-10-09T20:01:28Z

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

tchaikov added feature rgw labels May 10, 2017

rzarzynski self-assigned this May 10, 2017

Jing-Scott force-pushed the wip-add-x-his-location branch from 8c7c940 to 29b7951 Compare May 11, 2017 06:05

Jing-Scott commented May 11, 2017

View reviewed changes

rzarzynski reviewed Jun 12, 2017

View reviewed changes

Jing-Scott added 3 commits June 13, 2017 16:51

rgw: add the variable swift_his_location to be as archive bucket

59e9d87

Signed-off-by: Jing Wenjun <[email protected]>

rgw: implement RGWRados::swift_versioning_history

8347bc0

Signed-off-by: Jing Wenjun <[email protected]>

Jing-Scott force-pushed the wip-add-x-his-location branch from 29b7951 to bd226b6 Compare June 13, 2017 08:52

rzarzynski requested changes Jun 25, 2017

View reviewed changes

stale bot added the stale label Oct 18, 2018

mattbenjamin requested changes Nov 1, 2018

View reviewed changes

stale bot removed stale labels Nov 1, 2018

rzarzynski reviewed Nov 3, 2018

View reviewed changes

fghaas mentioned this pull request Nov 21, 2018

doc: Improve OpenStack integration and multitenancy docs for radosgw #25056

Merged

LenzGr added the needs-rebase label Dec 13, 2018

stale bot added the stale label Feb 11, 2019

stale bot closed this May 12, 2019

mattbenjamin reopened this May 12, 2019

stale bot removed the stale label May 12, 2019

stale bot added the stale label Jul 11, 2019

stale bot closed this Oct 9, 2019

Conversation

Jing-Scott commented May 10, 2017

Uh oh!

Jing-Scott commented May 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jing-Scott commented Jun 13, 2017

Uh oh!

Jing-Scott commented Jun 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stale bot commented Oct 18, 2018

Uh oh!

mattbenjamin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rzarzynski Nov 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stale bot commented Feb 11, 2019

Uh oh!

stale bot commented May 12, 2019

Uh oh!

mattbenjamin commented May 12, 2019

Uh oh!

stale bot commented Jul 11, 2019

Uh oh!

stale bot commented Oct 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rzarzynski Nov 3, 2018 •

edited

Loading