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: clear old zone short ids on period update #13949

Merged
merged 1 commit into from Mar 14, 2017

Conversation

Projects
None yet
2 participants
@cbodley
Contributor

cbodley commented Mar 13, 2017

the short ids of old, removed zones were being kept in the period to
guard against hash collisions with new zones

but for a hash collision to cause a wrong object to sync, that object
would have to be uploaded simultaneously to two different zones that had
the same short id

to avoid this, we just have to prevent the period from containing two
colliding zones at the same time - we don't have to remember old zone
short ids forever

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

rgw: clear old zone short ids on period update
the short ids of old, removed zones were being kept in the period to
guard against hash collisions with new zones

but for a hash collision to cause a wrong object to sync, that object
would have to be uploaded simultaneously to two different zones that had
the same short id

to avoid this, we just have to prevent the period from containing two
colliding zones at the same time - we don't have to remember old zone
short ids forever

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

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley

This comment has been minimized.

Contributor

cbodley commented Mar 13, 2017

tested on a multisite configuration with zonegroup na with two zones na-1 and na-2:

$ radosgw-admin period get
...
        "short_zone_ids": [
            {
                "key": "4d7381b2-d070-4af0-b9bf-780fbd7654d1",
                "val": 482653761
            },
            {
                "key": "5ba6c7bc-20a6-46e2-bb49-07c93f1cd874",
                "val": 215964409
            }
        ]
...
$ radosgw-admin zone create --rgw-zonegroup=na --rgw-zone=na-3
$ radosgw-admin period update --commit
...
        "short_zone_ids": [
            {
                "key": "4d7381b2-d070-4af0-b9bf-780fbd7654d1",
                "val": 482653761
            },
            {
                "key": "54d7fe3a-486a-42bf-96fa-3f8622c1e8e3",
                "val": 3387062521
            },
            {
                "key": "5ba6c7bc-20a6-46e2-bb49-07c93f1cd874",
                "val": 215964409
            }
        ]
...
$ radosgw-admin zonegroup remove --rgw-zonegroup na --rgw-zone na-3
$ radosgw-admin period update --commit
...
        "short_zone_ids": [
            {
                "key": "4d7381b2-d070-4af0-b9bf-780fbd7654d1",
                "val": 482653761
            },
            {
                "key": "5ba6c7bc-20a6-46e2-bb49-07c93f1cd874",
                "val": 215964409
            }
        ]
...
@@ -1302,6 +1302,10 @@ int RGWPeriod::update()
return ret;
}
// clear zone short ids of removed zones. period_map.update() will add the
// remaining zones back
period_map.short_zone_ids.clear();

This comment has been minimized.

@yehudasa

yehudasa Mar 13, 2017

Member

@cbodley the problem here I think is that zones that were already assigned an id after collision will have new id assigned to them. I think we need to make a different change in RGWPeriodMap::update(), where we remove entries off short_zone_id if not found.

This comment has been minimized.

@cbodley

cbodley Mar 13, 2017

Contributor

zones that were already assigned an id after collision will have new id assigned to them

sorry, i'm not quite sure what you mean here

if you get into a situation where you have multiple zone uuids that hash to the same short id, your period update --commit will error out with New zone <name> (<id>) generates the same short_zone_id <short_id> as existing zone id <id>

to resolve that, you would have to delete and recreate the new zone, which would generate a new uuid. then the next period update --commit would calculate a different short id for it, so there would be no collision and RGWPeriodMap::update() would end up with the right short_zone_ids

i don't see why we'd need to detect removed zones and handle them differently

This comment has been minimized.

@yehudasa

yehudasa Mar 13, 2017

Member

@cbodley ah, I misremembered.

@yehudasa

lgtm

@cbodley cbodley merged commit 6ba4858 into ceph:master Mar 14, 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

@cbodley cbodley deleted the cbodley:wip-15618 branch Mar 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment