rgw: The Lifecycle cannot delete objects when using multi-version and setting metadata by RGWRados::set_attrs #28100

xxcs · 2019-05-15T09:11:36Z

rgw: fix bug http://tracker.ceph.com/issues/39368

rgw: When using versioning, "call Rados::set_attrs for a version (for example, set self-defined metadata)" resulting in inconsistency between [BI_BUCKET_OBJS_INDEX: entry.meta.mtime] and [BI_BUCKET_OBJ_INSTANCE_INDEX: entry.meta.mtime], [DATA : mtime].(The current version cannot be correctly deleted in the lifecycle function)

related questions: #17400

Fixes: http://tracker.ceph.com/issues/39368
Signed-off-by: Snow Si silonghu@inspur.com

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

batrick

Please sign your commits. See also [1]. This is required for your PR to be merged.

[1] https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#1-sign-your-work

xxcs · 2019-05-16T01:13:44Z

Please sign your commits. See also [1]. This is required for your PR to be merged.

[1] https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#1-sign-your-work

OK, I have just modified it according to your suggestion, Thank you for your comments

tianshan · 2019-05-16T14:27:34Z

src/cls/rgw/cls_rgw.cc

+      } else {
+        int flags_tmp = instance_list_entry.flags;
+        instance_list_entry = entry;
+        instance_list_entry.flags = flags_tmp;


maybe instance_list_entry.meta.mtime = meta.mtime is more clear

maybe instance_list_entry.meta.mtime = meta.mtime is more clear

Yes, I do it at the beginning, It has almost no risk. But I think the values of [BI_BUCKET_OBJS_INDEX] and [BI_BUCKET_OBJ_INSTANCE_INDEX] should be the same(According to cls_rgw.cc:rgw_bucket_link_olh ---> cls_rgw.cc: write_obj_entries), then it caused another problem(due to "entry.flags = (entry.key.instance.empty() ? 0 : RGW_BUCKET_DIRENT_FLAG_VER);", current version is not visible, I am not sure what this line of code means)

[PS: In fact, at the beginning I want to call set_olh in the set_attrs function, Just like write_meta]

tianshan · 2019-05-16T14:30:34Z

I wonder if tested reput of null version, it will enter here not from set_attr call.

xxcs · 2019-05-17T01:18:11Z

I wonder if tested reput of null version, it will enter here not from set_attr call.

Yes, I have considered this problem, so I call "int ret_tmp = read_index_entry(hctx, instance_list_idx, &instance_list_entry);" first, if "ret_tmp == -ENOENT", then it will not execute any statement

tianshan · 2019-05-18T02:00:07Z

I means versioned object with "null" version id

xxcs · 2019-05-18T11:34:47Z

Yes, I do it

xxcs · 2019-08-09T12:49:55Z

I means versioned object with "null" version id

Hello, are there any other questions?

xxcs · 2019-08-16T09:49:24Z

@batrick @tianshan
Hello, Can you help review the code? Thank you very much.

stale · 2019-10-20T02:51:41Z

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.

cbodley · 2019-11-14T16:22:26Z

jenkins test make check

related questions: ceph#17400 Fixes: http://tracker.ceph.com/issues/39368 Signed-off-by: Snow Si <silonghu@inspur.com>

dang · 2019-11-21T16:06:40Z

src/cls/rgw/cls_rgw.cc

+      get_list_index_key(entry, &instance_list_idx);
+      int ret_tmp = read_index_entry(hctx, instance_list_idx, &instance_list_entry);
+      if (ret_tmp == -ENOENT) {
+      } else if (ret_tmp < 0) {


These should just check for >0, unless you want to actually do something in those cases.

cbodley · 2019-11-21T16:09:33Z

could you please add a s3test case that reproduces this bug?

stale · 2020-01-20T17:06:32Z

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-17T11:40:53Z

can't reproduce the issue in master, can you provide an example using s3cli or so, I did the following as per #17400 (comment) and see the historical versions being expired as expected

xxcs · 2020-03-18T07:55:34Z

can't reproduce the issue in master, can you provide an example using s3cli or so, I did the following as per #17400 (comment) and see the historical versions being expired as expected

At present, the master does not have this problem. It removes "if (state-> mtime! = Obj_iter-> meta.mtime) // Check mtime again to avoid delete a recently update object as much as possible" in rgw_lc.cc

stale · 2020-05-17T14:06: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 · 2020-08-15T23:34:04Z

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!

joke-lee · 2020-12-11T09:01:42Z

@theanalyst @cbodley @dang we can reproduce this way

./bin/radosgw-admin zonegroup placement add --placement-id=default-placement --storage-class=STANDARD_IA

./bin/radosgw-admin zone placement modify --rgw-zone=default --placement-id=default-placement --data-pool default.rgw.buckets.data-ia   --storage-class=STANDARD_IA

zone info as follow

"placement_pools": [
        {
            "key": "default-placement",
            "val": {
                "index_pool": "default.rgw.buckets.index",
                "storage_classes": {
                    "STANDARD": {
                        "data_pool": "default.rgw.buckets.data"
                    },
                    "STANDARD_IA": {
                        "data_pool": "default.rgw.buckets.data-ia"
                    }
                },
                "data_extra_pool": "default.rgw.buckets.non-ec",
                "index_type": 0
            }
        }
    ],

restart rgw

create bucket test1 and enable bucket version and upload 5M file 3time

5M	
revision #: 3 (current)	2020/12/11 16:39:39	"5f363e0e58a95f06cbe9bbc662c5dfb6"	5.00 MB	STANDARD	yly (yly)	09DLq6dsnCJ1vBJ8aP31r.2pUc1SrCo	
revision #: 2	2020/12/11 16:39:37	"5f363e0e58a95f06cbe9bbc662c5dfb6"	5.00 MB	STANDARD	yly (yly)	VysPnoeIh9HPP3IvRVkcNClUh5nSO.v	
revision #: 1	2020/12/11 16:39:35	"5f363e0e58a95f06cbe9bbc662c5dfb6"	5.00 MB	STANDARD	yly (yly)	CuEX0gB2en1QlvO-Xn3E3YONBbAi9JJ	

and putobjecttag on VysPnoeIh9HPP3IvRVkcNClUh5nSO.v version object

from boto3.session import Session
import boto3
import logging

import boto
logging.basicConfig(level=logging.DEBUG)
boto.set_stream_logger('boto')

access_key = "yly"
secret_key = "yly"
session = Session(access_key, secret_key)
url = "http://127.0.0.1:8000"
config = boto3.session.Config(connect_timeout=3000000, read_timeout=3000000, retries={'max_attempts': 0}, signature_version="s3")
s3_client = session.client('s3', endpoint_url=url, config=config)
# s3_client2 = session.client('s3', endpoint_url=url, config=config)
dst_bucket = 'test1'
dst_obj = '5M'

print s3_client.put_object_tagging(Bucket=dst_bucket, Key=dst_obj, Tagging={
        'TagSet': [
            {
                'Key': 'color',
                'Value': 'red'
            },
            {
                'Key': 'location',
                'Value': 'usa4'
            },
        ]
    }, VersionId='VysPnoeIh9HPP3IvRVkcNClUh5nSO.v')

we can find the time in instance and plain is not same

 ./bin/radosgw-admin bi list --bucket test1
...
    {
        "type": "plain",
        "idx": "5M\u0000v912\u0000iVysPnoeIh9HPP3IvRVkcNClUh5nSO.v",
        "entry": {
            "name": "5M",
            "instance": "VysPnoeIh9HPP3IvRVkcNClUh5nSO.v",
            "ver": {
                "pool": 6,
                "epoch": 1
            },
            "locator": "",
            "exists": "true",
            "meta": {
                "category": 1,
                "size": 5242880,
                "mtime": "2020-12-11T08:39:37.414509Z",
                "etag": "5f363e0e58a95f06cbe9bbc662c5dfb6",
                "storage_class": "STANDARD",
                "owner": "yly",
                "owner_display_name": "yly",
                "content_type": "application/octet-stream",
                "accounted_size": 5242880,
                "user_data": "",
                "appendable": "false"
            },
            "tag": "037fe0a6-cf27-4cfd-bd36-4611c036053a.14150.11",
            "flags": 1,
            "pending_map": [],
            "versioned_epoch": 3
        }
    },
...
    {
        "type": "instance",
        "idx": "�1000_5M\u0000iVysPnoeIh9HPP3IvRVkcNClUh5nSO.v",
        "entry": {
            "name": "5M",
            "instance": "VysPnoeIh9HPP3IvRVkcNClUh5nSO.v",
            "ver": {
                "pool": 6,
                "epoch": 2
            },
            "locator": "",
            "exists": "true",
            "meta": {
                "category": 1,
                "size": 5242880,
                "mtime": "2020-12-11T08:41:37.128280Z",
                "etag": "5f363e0e58a95f06cbe9bbc662c5dfb6",
                "storage_class": "STANDARD",
                "owner": "yly",
                "owner_display_name": "yly",
                "content_type": "application/octet-stream",
                "accounted_size": 5242880,
                "user_data": "",
                "appendable": "false"
            },
            "tag": "_-4ve3u7PbGngUp77nyem01KRfzmdFo1",
            "flags": 1,
            "pending_map": [],
            "versioned_epoch": 3
        }
    },
    {
        "type": "olh",
        "idx": "�1001_5M",
        "entry": {
            "key": {
                "name": "5M",
                "instance": "09DLq6dsnCJ1vBJ8aP31r.2pUc1SrCo"
            },
            "delete_marker": "false",
            "epoch": 4,
            "pending_log": [],
            "tag": "5ky2o2s57k019pj6qz80kbgs44pi9bdm",
            "exists": "true",
            "pending_removal": "false"
        }
    }
]

 "mtime": "2020-12-11T08:39:37.414509Z",    time in plain    
 "mtime": "2020-12-11T08:41:37.128280Z",    time in instance

config lc for the bucket and in the rgw log

2020-12-11T16:45:00.298+0800 7f03621d8700  0 lifecycle: ERROR: failed to transition obj :test1[037fe0a6-cf27-4cfd-bd36-4611c036053a.4135.1]):5M[VysPnoeIh9HPP3IvRVkcNClUh5nSO.v] -> STANDARD_IA (125) Operation canceled wp_thrd: 0, 1
2020-12-11T16:45:00.298+0800 7f03621d8700  0 lifecycle: ERROR: remove_expired_obj :test1[037fe0a6-cf27-4cfd-bd36-4611c036053a.4135.1]):5M[VysPnoeIh9HPP3IvRVkcNClUh5nSO.v] (125) Operation canceled wp_thrd: 0, 1

the VysPnoeIh9HPP3IvRVkcNClUh5nSO.v version object will never be transition, the other 2 with not tag set is success transition

ceph/src/rgw/rgw_rados.cc

Lines 4546 to 4549 in 4d6d146

    
           if (read_mtime != mtime) { 
        
             /* raced */ 
        
             return -ECANCELED; 
        
           }

xxcs force-pushed the xxcs branch from 8460c8e to 0ac57ba Compare May 15, 2019 10:52

batrick added needs-review rgw labels May 15, 2019

batrick requested changes May 15, 2019

View reviewed changes

xxcs force-pushed the xxcs branch from 0ac57ba to f2fd77c Compare May 16, 2019 01:08

tianshan reviewed May 16, 2019

View reviewed changes

xxcs closed this Jun 6, 2019

xxcs reopened this Jun 6, 2019

xxcs closed this Jul 2, 2019

xxcs reopened this Jul 2, 2019

xxcs force-pushed the xxcs branch 7 times, most recently from 9064a2f to 3608403 Compare August 7, 2019 01:42

xxcs changed the title ~~rgw: fix bug http://tracker.ceph.com/issues/39368~~ rgw: The Lifecycle cannot delete objects when using multi-version and setting metadata by RGWRados::set_attrs Aug 15, 2019

stale bot added the stale label Oct 20, 2019

cbodley requested a review from mattbenjamin November 14, 2019 16:21

stale bot removed the stale label Nov 14, 2019

rgw: fix bug http://tracker.ceph.com/issues/39368

ae45c6c

related questions: ceph#17400 Fixes: http://tracker.ceph.com/issues/39368 Signed-off-by: Snow Si <silonghu@inspur.com>

xxcs force-pushed the xxcs branch from 3608403 to ae45c6c Compare November 15, 2019 01:44

dang reviewed Nov 21, 2019

View reviewed changes

cbodley added the needs-test label Nov 21, 2019

stale bot added the stale label Jan 20, 2020

dang added needs-qa and removed needs-test labels Feb 4, 2020

stale bot removed the stale label Feb 4, 2020

dang added needs-test and removed needs-qa labels Feb 4, 2020

stale bot added the stale label May 17, 2020

stale bot closed this Aug 15, 2020

mattbenjamin requested a review from ivancich December 11, 2020 14:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rgw: The Lifecycle cannot delete objects when using multi-version and setting metadata by RGWRados::set_attrs #28100

rgw: The Lifecycle cannot delete objects when using multi-version and setting metadata by RGWRados::set_attrs #28100

xxcs commented May 15, 2019 •

edited

Loading

batrick left a comment

xxcs commented May 16, 2019

tianshan May 16, 2019

xxcs May 17, 2019

tianshan commented May 16, 2019

xxcs commented May 17, 2019

tianshan commented May 18, 2019

xxcs commented May 18, 2019

xxcs commented Aug 9, 2019

xxcs commented Aug 16, 2019 •

edited

Loading

stale bot commented Oct 20, 2019

cbodley commented Nov 14, 2019

dang Nov 21, 2019

cbodley commented Nov 21, 2019

stale bot commented Jan 20, 2020

theanalyst commented Mar 17, 2020

xxcs commented Mar 18, 2020

stale bot commented May 17, 2020

stale bot commented Aug 15, 2020

joke-lee commented Dec 11, 2020 •

edited

Loading

rgw: The Lifecycle cannot delete objects when using multi-version and setting metadata by RGWRados::set_attrs #28100

rgw: The Lifecycle cannot delete objects when using multi-version and setting metadata by RGWRados::set_attrs #28100

Conversation

xxcs commented May 15, 2019 • edited Loading

batrick left a comment

Choose a reason for hiding this comment

xxcs commented May 16, 2019

tianshan May 16, 2019

Choose a reason for hiding this comment

xxcs May 17, 2019

Choose a reason for hiding this comment

tianshan commented May 16, 2019

xxcs commented May 17, 2019

tianshan commented May 18, 2019

xxcs commented May 18, 2019

xxcs commented Aug 9, 2019

xxcs commented Aug 16, 2019 • edited Loading

stale bot commented Oct 20, 2019

cbodley commented Nov 14, 2019

dang Nov 21, 2019

Choose a reason for hiding this comment

cbodley commented Nov 21, 2019

stale bot commented Jan 20, 2020

theanalyst commented Mar 17, 2020

xxcs commented Mar 18, 2020

stale bot commented May 17, 2020

stale bot commented Aug 15, 2020

joke-lee commented Dec 11, 2020 • edited Loading

xxcs commented May 15, 2019 •

edited

Loading

xxcs commented Aug 16, 2019 •

edited

Loading

joke-lee commented Dec 11, 2020 •

edited

Loading