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: disable sync squash for versioned objects #13426

Closed
wants to merge 2 commits into from

Conversation

yehudasa
Copy link
Member

and also modify mtime reported on unlink_instance operation.

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

@@ -2530,11 +2534,12 @@ int RGWBucketShardIncrementalSyncCR::operate()
marker_tracker.try_update_high_marker(cur_id, 0, entry->timestamp);
continue;
}
if (make_pair<>(entry->timestamp, entry->op) != squash_map[make_pair(entry->object, entry->instance)]) {
if (entry->instance.empty() &&
make_pair<>(entry->timestamp, entry->op) != squash_map[make_pair(entry->object, entry->instance)]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we revert the commit ce7d00a that added instance to the squash_map keys now that we're not using it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cbodley I'm not extremely happy about this fix, would rather understand the exact root cause before. The mtime issue was a problem, but I think there's another issue coming into play (I think it's del squashing unlink_instance), but still need to verify.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that commit removed a special case related to OP_DEL, maybe that's relevant to this issue

@cbodley
Copy link
Contributor

cbodley commented Feb 16, 2017

also Fixes: http://tracker.ceph.com/issues/18885

This is needed so that when squashing entries during data
sync we get a consistent mtime for that object.

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
Fixes: http://tracker.ceph.com/issues/18939

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
@yehudasa
Copy link
Member Author

yehudasa commented Nov 2, 2017

@mattbenjamin @oritwas rebased

@cbodley
Copy link
Contributor

cbodley commented Jan 18, 2018

pulled the mtime change into a separate pr #20016

@yuriw
Copy link
Contributor

yuriw commented Jul 5, 2018

@yehudasa pls rebase

--- pr 13426 --- pulling https://github.com/yehudasa/ceph.git branch wip-18939
remote: Counting objects: 11, done.
remote: Total 11 (delta 9), reused 9 (delta 9), pack-reused 2
Unpacking objects: 100% (11/11), done.
From https://github.com/yehudasa/ceph

  • branch wip-18939 -> FETCH_HEAD
    Auto-merging src/rgw/rgw_data_sync.cc
    CONFLICT (file/directory): There is a directory with name src/dmclock in 9b10bce. Adding src/dmclock as src/dmclock~HEAD
    Adding qa/suites/rgw/verify/frontend
    Adding qa/suites/rgw/tempest/frontend
    Adding qa/suites/rgw/multisite/frontend
    Adding qa/suites/rbd/mirror/cluster
    Adding qa/suites/rbd/mirror/base
    Adding qa/suites/ceph-deploy/basic/distros/ubuntu_latest.yaml
    Adding qa/suites/ceph-deploy/basic/distros/centos_latest.yaml
    Adding qa/suites/ceph-deploy/basic/distros/.qa
    Automatic merge failed; fix conflicts and then commit the result.
    Traceback (most recent call last):
    File "/home/yuriw/wip_master/src/script/build-integration-branch", line 62, in
    assert not r
    AssertionError

@cbodley cbodley removed the needs-qa label Jul 5, 2018
@stale
Copy link

stale bot commented Oct 17, 2018

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 stale bot added the stale label Oct 17, 2018
@stale
Copy link

stale bot commented Apr 22, 2019

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 stale bot closed this Apr 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants