rgw: add missing RGWPeriod::reflect() based on new atomic update_latest_epoch()#14915
Merged
yuriw merged 5 commits intoceph:masterfrom Jul 14, 2017
Merged
rgw: add missing RGWPeriod::reflect() based on new atomic update_latest_epoch()#14915yuriw merged 5 commits intoceph:masterfrom
yuriw merged 5 commits intoceph:masterfrom
Conversation
4 tasks
Contributor
Author
|
turns out this doesn't actually fix my issue in multisite-for-teuthology, but the change is still valid and passed the rgw suite: http://pulpito.ceph.com/cbodley-2017-05-02_14:01:34-rgw-wip-19817---basic-smithi/ (2 known s3test failures due to apache, the rest were valgrind failures in the mon) |
Member
|
jenkins test this please |
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
update_latest_epoch() uses RGWObjVersionTracker to implement atomic updates to the period's latest_epoch, returning -EEXIST if we already have an epoch >= the one given Fixes: http://tracker.ceph.com/issues/19816 Signed-off-by: Casey Bodley <cbodley@redhat.com>
split the latest_epoch update out of RGWPeriod::store_info(), so callers that need to call the atomic update_latest_epoch() can do so and interpret its result separately Signed-off-by: Casey Bodley <cbodley@redhat.com>
when updating the period, callers use the atomic result of update_latest_epoch() to determine whether they need to call RGWPeriod::reflect() and RGWRealm::notify_new_period() this adds a missing call to RGWPeriod::reflect() to RGWPeriodPuller, which was previously not safe to do without atomic updates to latest_epoch Fixes: http://tracker.ceph.com/issues/19817 Signed-off-by: Casey Bodley <cbodley@redhat.com>
Contributor
Author
|
rebased |
oritwas
approved these changes
Jul 13, 2017
Contributor
|
I guess the answer is "yes" because the tests in #14688 found a real bug that is being addressed by this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tests of #14688 were failing because a
period update --commitintest_multi_period_incremental_sync()was mistakenly removing another zone from the period. this was happening becauseRGWPeriodPullerhad store the latest period without also callingRGWPeriod::reflect()to update the local zonegroup object. so thezone modify --masterandperiod update --commitwere operating on an old version of the zonegroupRGWPeriodPullercould not safely callRGWPeriod::reflect(), because it couldn't determine whether it had the latest epoch of that period (calling reflect() on an older epoch would overwrite the more recent zonegroup object). this was explained by the comment// XXX: if this is a newer epoch, we should overwrite the existing latest_epoch. but there's no way to do that atomicallythis patch set adds a new
RGWPeriod::update_latest_epoch()to provide atomic updates to the period's latest_epoch object, which callers can use to safely determine whether to callRGWPeriod::reflect()andRGWRealm::notify_new_period()Fixes: http://tracker.ceph.com/issues/19816
Fixes: http://tracker.ceph.com/issues/19817