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.

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

rgw: add the remove-x-delete feature to cancel swift object expiration #13621

Merged
merged 1 commit into from Apr 3, 2017

Conversation

Projects
None yet
4 participants
@Jing-Scott
Copy link
Contributor

Jing-Scott commented Feb 24, 2017

In openstack swift, it also support the feature to cancel the object expiration, which could be found at last point in https://docs.openstack.org/user-guide/cli-swift-set-object-expiration.html. we can remove the object expiration by set 'X-Remove-Delete-At:'.

This patch also could fix the bug that when we set the object expiration and then upload the same object to the container again. The previous object expiration also works, which is not compatible with the openstack swift.

Fixes: http://tracker.ceph.com/issues/19074
Signed-off-by: Jing Wenjun jingwenjun@cmss.chinamobile.com

@Jing-Scott Jing-Scott force-pushed the Jing-Scott:add-remov-x-delete-feature branch from f6d6c6c to 25ad980 Feb 24, 2017

@Jing-Scott

This comment has been minimized.

Copy link
Contributor Author

Jing-Scott commented Feb 27, 2017

Hi, @rzarzynski
Could you help me to take a look? Thanks!

@rzarzynski rzarzynski self-assigned this Feb 27, 2017

{
if (real_clock::is_zero(delete_at)) {

This comment has been minimized.

Copy link
@rzarzynski

rzarzynski Feb 28, 2017

Contributor

It looks we started treating0 in delete_at as special value saying "expiration hint revoked". Does the expirer part is OK with that?

This comment has been minimized.

Copy link
@Jing-Scott

Jing-Scott Mar 1, 2017

Author Contributor

Yeah. @rzarzynski : In this change, if we want to set the object expiration time. The var set_delete_at will be true and the delete_at updated relatively.

In int RGWObjectExpirer::garbage_single_object(objexp_hint_entry& hint) it will call delete_obj()
In int RGWRados::Object::Delete::delete_obj(), you will find these code:

if (!real_clock::is_zero(params.expiration_time)) {
    bufferlist bl;
    real_time delete_at;

    if (state->get_attr(RGW_ATTR_DELETE_AT, bl)) {
      try {
        bufferlist::iterator iter = bl.begin();
        ::decode(delete_at, iter);
      } catch (buffer::error& err) {
        ldout(store->ctx(), 0) << "ERROR: couldn't decode RGW_ATTR_DELETE_AT" << dendl;
	return -EIO;
      }

      if (params.expiration_time != delete_at) {
        return -ERR_PRECONDITION_FAILED;
      }
    }
  }

In if (params.expiration_time != delete_at) { :
if the expiration time of object is updated, the delete_at is the current expiration time in bufferlist and the params,expiration_time is the non-current expiration time in the list of cls_timeindex_entry.

So, that means if the two expriation time is not consistent, the object will not be deleted.
If we intend to cancel the object expiration time, the delete_at will be 0 and the object will never be deleted because of if (!real_clock::is_zero(params.expiration_time)) { in upper code.

This comment has been minimized.

Copy link
@rzarzynski

rzarzynski Mar 2, 2017

Contributor

Thanks for the very detailed explanation! Conceptually everything looks good.

{
if (real_clock::is_zero(delete_at)) {
map<string, bufferlist>::iterator iter = attrs.find(RGW_ATTR_DELETE_AT);
if (!set_delete_at && iter != attrs.end()) {

This comment has been minimized.

Copy link
@Jing-Scott

Jing-Scott Mar 1, 2017

Author Contributor

@rzarzynski in this change, the delete_at will be encoded only in two situations:

  1. the object was upload in first time
  2. the object expiration time was updated(including remove the expiration time)

This comment has been minimized.

Copy link
@rzarzynski

rzarzynski Mar 2, 2017

Contributor

What do you think about switching to boost::optional<ceph::real_time> instead of introducing theset_delete_at flag? Optional seems to be intended for handling cases exactly like this one.

@Jing-Scott Jing-Scott force-pushed the Jing-Scott:add-remov-x-delete-feature branch from 25ad980 to 8c6753e Mar 6, 2017

@Jing-Scott

This comment has been minimized.

Copy link
Contributor Author

Jing-Scott commented Mar 6, 2017

@rzarzynski Hi, thanks for your suggestion! I've updated them with boost::optional<ceph::real_time> delete_at instead of ceph::real_time delete_at.

@Jing-Scott Jing-Scott force-pushed the Jing-Scott:add-remov-x-delete-feature branch from 8c6753e to 2a9da73 Mar 6, 2017

@@ -1145,7 +1145,7 @@ namespace rgw {
emplace_attr(RGW_ATTR_SLO_UINDICATOR, std::move(slo_userindicator_bl));
}

op_ret = processor->complete(s->obj_size, etag, &mtime, real_time(), attrs, delete_at,
op_ret = processor->complete(s->obj_size, etag, &mtime, real_time(), attrs, *delete_at,

This comment has been minimized.

Copy link
@rzarzynski

rzarzynski Mar 8, 2017

Contributor

It looks we might be dereferencing an empty boost::optional here. If that's true, following check should help:

delete_at ? *delete_at : 0
@@ -3343,7 +3343,7 @@ void RGWPostObj::execute()
emplace_attr(RGW_ATTR_COMPRESSION, std::move(tmp));
}

op_ret = processor.complete(s->obj_size, etag, NULL, real_time(), attrs, delete_at);
op_ret = processor.complete(s->obj_size, etag, NULL, real_time(), attrs, *delete_at);

This comment has been minimized.

Copy link
@rzarzynski

rzarzynski Mar 8, 2017

Contributor

The same here.

if (real_clock::is_zero(delete_at)) {
return;
map<string, bufferlist>::iterator iter = attrs.find(RGW_ATTR_DELETE_AT);
if (delete_at == boost::none) {

This comment has been minimized.

Copy link
@rzarzynski

rzarzynski Mar 8, 2017

Contributor

This is much more readable now. Thanks!

This comment has been minimized.

Copy link
@rzarzynski

rzarzynski Mar 8, 2017

Contributor

Hmm, I wonder whether we really need to have the RGW_ATTR_DELETE_AT set for all objects (including those created through the S3 API). Maybe the check solely for boost::none would be enough? What do you think?

This comment has been minimized.

Copy link
@Jing-Scott

Jing-Scott Mar 9, 2017

Author Contributor

@rzarzynski ,I've updated them. I considered the http://tracker.ceph.com/issues/19074, so I set RGW_ATTR_DELETE_AT to all objects before. But now there is another solution instead which is described below.

@Jing-Scott Jing-Scott force-pushed the Jing-Scott:add-remov-x-delete-feature branch 6 times, most recently from 1347007 to e0ade73 Mar 8, 2017

rgw: add the remove-x-delete feature to cancel swift object expiration
In openstack swift, it also support the feature to cancel the object expiration,
which could be found at last point in
https://docs.openstack.org/user-guide/cli-swift-set-object-expiration.html. we
can remove the object expiration by set 'X-Remove-Delete-At:'.

This patch also could fix the bug that when we set the object expiration and
then upload the same object to the container again. The previous object expiration
also works, which is not compatible with the openstack swift.

Fixes: http://tracker.ceph.com/issues/19074
Signed-off-by: Jing Wenjun <jingwenjun@cmss.chinamobile.com>

@Jing-Scott Jing-Scott force-pushed the Jing-Scott:add-remov-x-delete-feature branch from e0ade73 to 230429e Mar 9, 2017

@@ -8328,6 +8328,8 @@ int RGWRados::Object::Delete::delete_obj()
if (params.expiration_time != delete_at) {
return -ERR_PRECONDITION_FAILED;
}
} else {

This comment has been minimized.

Copy link
@Jing-Scott

Jing-Scott Mar 9, 2017

Author Contributor

This comment has been minimized.

Copy link
@rzarzynski

rzarzynski Apr 3, 2017

Contributor

OK, RGWRados::delete_obj takes expiration_time which is by default ceph::real_time():

  /** Delete an object.*/
  int delete_obj(RGWObjectCtx& obj_ctx,
                         const RGWBucketInfo& bucket_owner,
                         const rgw_obj& src_obj,
                         int versioning_status,
                         uint16_t bilog_flags = 0,
                         const ceph::real_time& expiration_time = ceph::real_time());
@cbodley

This comment has been minimized.

Copy link
Contributor

cbodley commented Mar 24, 2017

@rzarzynski
Copy link
Contributor

rzarzynski left a comment

Looks good. Thanks, @Jing-Scott!

op_ret = processor->complete(s->obj_size, etag, &mtime, real_time(), attrs, delete_at,
if_match, if_nomatch);
op_ret = processor->complete(s->obj_size, etag, &mtime, real_time(), attrs,
(delete_at ? *delete_at : real_time()), if_match, if_nomatch);

This comment has been minimized.

Copy link
@rzarzynski

rzarzynski Apr 3, 2017

Contributor

If delete_at is empty, then we supply an instance of real_time. ceph::real_time is an alias over std::chrono::time_point<ceph::time_detail::real_clock>. Its default-constructed instance represents logical zero --real_clock::is_zero(ceph::real_time()) == true. OK.

@@ -8328,6 +8328,8 @@ int RGWRados::Object::Delete::delete_obj()
if (params.expiration_time != delete_at) {
return -ERR_PRECONDITION_FAILED;
}
} else {

This comment has been minimized.

Copy link
@rzarzynski

rzarzynski Apr 3, 2017

Contributor

OK, RGWRados::delete_obj takes expiration_time which is by default ceph::real_time():

  /** Delete an object.*/
  int delete_obj(RGWObjectCtx& obj_ctx,
                         const RGWBucketInfo& bucket_owner,
                         const rgw_obj& src_obj,
                         int versioning_status,
                         uint16_t bilog_flags = 0,
                         const ceph::real_time& expiration_time = ceph::real_time());

@rzarzynski rzarzynski merged commit 51dd349 into ceph:master Apr 3, 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

@Jing-Scott Jing-Scott deleted the Jing-Scott:add-remov-x-delete-feature branch Apr 5, 2017

@Jing-Scott Jing-Scott restored the Jing-Scott:add-remov-x-delete-feature branch Jun 8, 2017

@Jing-Scott Jing-Scott deleted the Jing-Scott:add-remov-x-delete-feature branch Jun 8, 2017

dongbula added a commit to dongbula/ceph that referenced this pull request Jul 22, 2017

Merge pull request ceph#13621 from Jing-Scott/add-remov-x-delete-feature
rgw: add the remove-x-delete feature to cancel swift object expiration

Reviewed-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.