rgw: Multipart upload may failed when object size is large and near byte quota limit #17960

gaosibei · 2017-09-26T02:54:13Z

When user upload big object using multipart api, they may upload failed. The reason is that the big object is considered to be multiple objects for quota stats

Fixes:http://tracker.ceph.com/issues/21565

Signed-off-by: Sibei Gao gaosb@inspur.com

amitkumar50 · 2017-09-26T15:05:28Z

src/cls/rgw/cls_rgw.cc

@@ -707,10 +707,27 @@ int rgw_bucket_prepare_op(cls_method_context_t hctx, bufferlist *in, bufferlist
  return write_bucket_header(hctx, &header);
 }

+static bool obj_name_filter(string& name)


indentation looks wrong in function

The format problem has been corrected

add tracker issue : http://tracker.ceph.com/issues/21565

Function name should be something more descriptive. "is_multipart_objname" or some such.
name can be const. And this could be inline.

ZVampirEM77 · 2017-09-27T02:17:53Z

@gaosibei It's better to create a tracker issue and add a link to it with Fixes: in your commit message

gaosibei · 2017-09-29T01:42:44Z

@ZVampirEM77

Thanks for taking a look!
I've added the tracker issue ：http://tracker.ceph.com/issues/21565

mdw-at-linuxbox · 2017-10-03T06:31:19Z

src/cls/rgw/cls_rgw.cc

+  int ret = true;
+  if (name.substr(0,10) == MP_MULTI_SUFFIX) {
+    int len = name.size();
+    size_t pos = name.find(MP_META_SUFFIX, len - 5);


using "find" here is suboptimal. Use boost::algorithm::ends_with or perhaps name.compare.
Also, -5 is magic - not good to bury knowledge of length of MP_META_SUFFIX like this.

mdw-at-linuxbox · 2017-10-03T06:32:35Z

src/cls/rgw/cls_rgw.cc

+static bool obj_name_filter(string& name)
+{
+  int ret = true;
+  if (name.substr(0,10) == MP_MULTI_SUFFIX) {


Like below "10" is magic, ugh. also substr makes a temp copy - better to use compare. Or better yet use "boost::algorithm::starts_with".

mdw-at-linuxbox · 2017-10-03T06:35:15Z

src/cls/rgw/cls_rgw_types.h

@@ -11,6 +11,8 @@
 #define CEPH_RGW_TAG_TIMEOUT 120
 #define CEPH_RGW_DIR_SUGGEST_LOG_OP  0x80
 #define CEPH_RGW_DIR_SUGGEST_OP_MASK 0x7f
+#define MP_META_SUFFIX ".meta"
+#define MP_MULTI_SUFFIX "_multipart"


This isn't really a suffix is it? This looks like it's supposed to match "RGW_OBJ_NS_MULTIPART" - however that omits the '_' which is inconvenient.

mdw-at-linuxbox · 2017-10-03T06:41:15Z

I don't have a good way to fix this, but I wish there were some way to share more of the oid/ns code in src/rgw with this logic. This logic here is basically duplicating a small subset of that logic with code that doesn't obviously match it, which is not an ideal style choice.

gaosibei · 2017-10-03T13:02:26Z

@mdw-at-linuxbox
Thanks for your suggestion.

According to your review, I have modified the code with boost. But, I'm not sure this change is a good way to fix this bug.

If this change is s`till not a good method, I would parse oid and ns from entry.key.name which like the function: parse_raw_oid in src/rgw/rgw_common.h.

mdw-at-linuxbox · 2017-10-03T21:30:11Z

"parse_raw_oid" looks intriguing, so that's probably the "right" way to do this. If you can try it and see if it works, that would be wonderful. If it turns out to be a complicated change touching lots of files, can you push it as a separate PR? I think we might want to keep this change for jewel/backports, and use "parse_raw_oid" going forward in master. Thanks!

mdw-at-linuxbox · 2017-10-05T07:33:22Z

There were some impatient people around, so I created another PR with a revised copy of this change.
#18123
It uses parse_raw_oid .

gaosibei · 2017-10-05T11:39:25Z

@mdw-at-linuxbox
Tanks for your quickly amend with parse_raw_oid, it looks good.

According to the PR#18123, I think the function name should be changed to "is_not_multipart_objname". When the return value is
true, it means that the object name is not multipart.

The another way is changed the return value from false to true, when the object name is multipart.
And the code: "if (is_multipart_objname(entry.key.name)) " should be changed to
"if (!(is_multipart_objname(entry.key.name))) ".

gaosibei · 2017-10-06T14:14:05Z

@mattbenjamin @mdw-at-linuxbox
Now, I give a new method to fix this bug which don't need parse object names. This method use "multipart" to identify the full operation being performed is completing a multipart upload

mattbenjamin · 2017-10-06T14:23:24Z

@gaosibei this new approach looks appealing, thanks very much!

yehudasa

@gaosibei need to verify that s3-tests still pass with this one. One thing to check is whether the behavior of deleting a bucket with incomplete multipart upload changed.

yehudasa · 2017-10-06T16:12:49Z

src/cls/rgw/cls_rgw_types.h

@@ -97,12 +97,13 @@ struct rgw_bucket_dir_entry_meta {
  string content_type;
  uint64_t accounted_size;
  string user_data;
+  bool multipart;


this shouldn't be called multipart, should be something like accounted_entry. Should default to true, and the logic should be flipped.

yehudasa · 2017-10-06T16:16:25Z

src/rgw/rgw_rados.cc

@@ -6897,7 +6897,7 @@ int RGWRados::Object::Write::_do_write_meta(uint64_t size, uint64_t accounted_si
  uint64_t orig_size;

  if (!reset_obj) {    //Multipart upload, it has immutable head. 
-    orig_exists = false;
+    orig_exists = true;


should comment why this is true

gaosibei · 2017-10-16T02:14:44Z

@mattbenjamin @mdw-at-linuxbox @yehudasa

can you take a look again?

gaosibei · 2017-11-09T09:49:47Z

@mattbenjamin

can you take a look again?

oritwas · 2018-02-15T08:39:42Z

src/cls/rgw/cls_rgw_types.h

@@ -130,6 +132,8 @@ struct rgw_bucket_dir_entry_meta {
      accounted_size = size;
    if (struct_v >= 5)
      ::decode(user_data, bl);
+    if (struct_v >= 6)


you need to set accounted_entry as true for older version which don't have this flag.
otherwise when you delete them you won't remove them from the accounting (which there were already accounted for)

oritwas · 2018-02-15T08:41:48Z

This pr needs upgrade tests to make sure we don't break the accounting on them

stale · 2018-10-18T03:54:07Z

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-04-22T19:34:48Z

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!

stale · 2019-06-22T02:23:19Z

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-09-20T03:19:25Z

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 · 2019-09-20T03:35:09Z

@cbodley is this change still relevant ?

cbodley · 2019-11-14T15:59:47Z

@gaosibei can you please rebase this?

stale · 2020-01-13T16:43:52Z

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.

theanalyst · 2020-03-05T15:06:45Z

needs rebase

stale · 2020-05-04T23:30:28Z

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 · 2020-05-06T01:45:05Z

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 · 2020-08-06T08:46:35Z

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!

gaosibei force-pushed the branch3 branch 4 times, most recently from 227e867 to ad079d1 Compare September 26, 2017 06:40

amitkumar50 reviewed Sep 26, 2017

View reviewed changes

mattbenjamin added bug-fix rgw labels Sep 26, 2017

mattbenjamin assigned mdw-at-linuxbox Sep 26, 2017

gaosibei force-pushed the branch3 branch from ad079d1 to 199a947 Compare September 27, 2017 02:17

gaosibei force-pushed the branch3 branch from 199a947 to 6ebc6a7 Compare September 29, 2017 00:55

gaosibei force-pushed the branch3 branch from 6ebc6a7 to 6e26123 Compare October 1, 2017 03:42

mdw-at-linuxbox reviewed Oct 3, 2017

View reviewed changes

gaosibei force-pushed the branch3 branch from 6e26123 to 8327626 Compare October 3, 2017 10:24

mattbenjamin changed the title ~~RGW: Multipart upload may failed when object size is large~~ RGW: Multipart upload may failed when object size is large and near byte quota limit Oct 5, 2017

gaosibei force-pushed the branch3 branch 3 times, most recently from f5c1809 to 2121cfb Compare October 6, 2017 13:25

gaosibei mentioned this pull request Oct 6, 2017

rgw: Multipart upload may failed when object size is large #18123

Closed

yehudasa reviewed Oct 6, 2017

View reviewed changes

Merge branch 'master' into branch3

9114861

liewegas changed the title ~~RGW: Multipart upload may failed when object size is large and near byte quota limit~~ rgw: Multipart upload may failed when object size is large and near byte quota limit Nov 17, 2017

oritwas reviewed Feb 15, 2018

View reviewed changes

stale bot added the stale label Oct 18, 2018

stale bot closed this Apr 22, 2019

tchaikov reopened this Apr 23, 2019

stale bot removed the stale label Apr 23, 2019

stale bot added the stale label Jun 22, 2019

stale bot closed this Sep 20, 2019

tchaikov reopened this Sep 20, 2019

stale bot removed the stale label Sep 20, 2019

ivancich added the needs-rebase label Nov 21, 2019

stale bot added the stale label Jan 13, 2020

stale bot removed the stale label Mar 5, 2020

stale bot added the stale label May 4, 2020

stale bot closed this Aug 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rgw: Multipart upload may failed when object size is large and near byte quota limit #17960

rgw: Multipart upload may failed when object size is large and near byte quota limit #17960

gaosibei commented Sep 26, 2017 •

edited

amitkumar50 Sep 26, 2017

gaosibei Sep 27, 2017

gaosibei Sep 27, 2017

mdw-at-linuxbox Oct 3, 2017

ZVampirEM77 commented Sep 27, 2017

gaosibei commented Sep 29, 2017

mdw-at-linuxbox Oct 3, 2017

mdw-at-linuxbox Oct 3, 2017

mdw-at-linuxbox Oct 3, 2017

mdw-at-linuxbox commented Oct 3, 2017

gaosibei commented Oct 3, 2017 •

edited

mdw-at-linuxbox commented Oct 3, 2017

mdw-at-linuxbox commented Oct 5, 2017

gaosibei commented Oct 5, 2017 •

edited

gaosibei commented Oct 6, 2017 •

edited

mattbenjamin commented Oct 6, 2017

yehudasa left a comment

yehudasa Oct 6, 2017

yehudasa Oct 6, 2017

gaosibei commented Oct 16, 2017

gaosibei commented Nov 9, 2017

oritwas Feb 15, 2018

oritwas commented Feb 15, 2018

stale bot commented Oct 18, 2018

stale bot commented Apr 22, 2019

stale bot commented Jun 22, 2019

stale bot commented Sep 20, 2019

tchaikov commented Sep 20, 2019

cbodley commented Nov 14, 2019

stale bot commented Jan 13, 2020

theanalyst commented Mar 5, 2020

stale bot commented May 4, 2020

stale bot commented May 6, 2020

stale bot commented Aug 6, 2020

rgw: Multipart upload may failed when object size is large and near byte quota limit #17960

rgw: Multipart upload may failed when object size is large and near byte quota limit #17960

Conversation

gaosibei commented Sep 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZVampirEM77 commented Sep 27, 2017

gaosibei commented Sep 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdw-at-linuxbox commented Oct 3, 2017

gaosibei commented Oct 3, 2017 • edited

mdw-at-linuxbox commented Oct 3, 2017

mdw-at-linuxbox commented Oct 5, 2017

gaosibei commented Oct 5, 2017 • edited

gaosibei commented Oct 6, 2017 • edited

mattbenjamin commented Oct 6, 2017

yehudasa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaosibei commented Oct 16, 2017

gaosibei commented Nov 9, 2017

Choose a reason for hiding this comment

oritwas commented Feb 15, 2018

stale bot commented Oct 18, 2018

stale bot commented Apr 22, 2019

stale bot commented Jun 22, 2019

stale bot commented Sep 20, 2019

tchaikov commented Sep 20, 2019

cbodley commented Nov 14, 2019

stale bot commented Jan 13, 2020

theanalyst commented Mar 5, 2020

stale bot commented May 4, 2020

stale bot commented May 6, 2020

stale bot commented Aug 6, 2020

gaosibei commented Sep 26, 2017 •

edited

gaosibei commented Oct 3, 2017 •

edited

gaosibei commented Oct 5, 2017 •

edited

gaosibei commented Oct 6, 2017 •

edited