Skip to content

Comments

rgw/multisite: fix object sync on versioned and versioning-suspended buckets#62594

Closed
smanjara wants to merge 1 commit intoceph:mainfrom
smanjara:wip-fix-object-sync-bucket-versioning
Closed

rgw/multisite: fix object sync on versioned and versioning-suspended buckets#62594
smanjara wants to merge 1 commit intoceph:mainfrom
smanjara:wip-fix-object-sync-bucket-versioning

Conversation

@smanjara
Copy link
Contributor

@smanjara smanjara commented Apr 1, 2025

fixes regression tracked in: https://tracker.ceph.com/issues/70728

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

rgw/multisite: non-versioned objects from source should still sync to…
009575c 
… destination

while versioning is suspended on dest bucket. also, choosing to sync a new object
based on versioning should only apply to ADD or LINK_OLH ops.

Signed-off-by: Shilpa Jagannath <[email protected]>
@smanjara smanjara requested a review from a team as a code owner April 1, 2025 00:55
@github-actions github-actions bot added the rgw label Apr 1, 2025
@smanjara smanjara mentioned this pull request Apr 1, 2025
Merged
@smanjara
Copy link
Contributor Author

smanjara commented Apr 1, 2025

the two tests that were failing in tracker pass with this change.

@cbodley cbodley requested a review from clwluvw April 1, 2025 13:10
@clwluvw
Copy link
Member

clwluvw commented Apr 1, 2025 •
edited
Loading

I guess we should perhaps have this clause moved upper (so instance would not be empty for null versioned), but still not sure why this only is set for CLS_RGW_OP_UNLINK_INSTANCE || CLS_RGW_OP_DEL initially

ceph/src/rgw/driver/rados/rgw_data_sync.cc

Lines 4566 to 4568 in f75bfa4

if (null_verid) {
key.instance = "null";
}

@clwluvw
Copy link
Member

clwluvw commented Apr 1, 2025

but i can't find setting RGW_BILOG_NULL_VERSION in _do_write_meta() for bilog_flags so that might not work properly for replicating suspended objects.
could it make sense to integrate RGW_BILOG_NULL_VERSION in _do_write_meta() as well?

@smanjara
Copy link
Contributor Author

smanjara commented Apr 1, 2025

but i can't find setting RGW_BILOG_NULL_VERSION in _do_write_meta() for bilog_flags so that might not work properly for replicating suspended objects. could it make sense to integrate RGW_BILOG_NULL_VERSION in _do_write_meta() as well?

RGW_BILOG_NULL_VERSION was a special flag we added to allow deleted null-versioned objects to be synced (58133) because delete_obj would clear the "null" in

ceph/src/rgw/driver/rados/rgw_rados.cc

Line 6057 in f75bfa4

if (instance == "null") {
which would then end up leaving a delete marker on the destination instead of deleting the "null" versioned object. so we pass the special flag down to cls and read it on the destination, add the "null" back to key.instance so that destination properly deletes the object.
I'm not sure we want to add that complexity here to fix this case.

clwluvw
clwluvw reviewed Apr 1, 2025
View reviewed changes
clwluvw
clwluvw reviewed Apr 2, 2025
View reviewed changes
@smanjara smanjara closed this Apr 2, 2025
@smanjara
Copy link
Contributor Author

smanjara commented Apr 2, 2025

closing in favor of 62637

@clwluvw clwluvw mentioned this pull request Apr 23, 2025
Closed
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cbodley cbodley cbodley left review comments

@clwluvw clwluvw clwluvw left review comments

Assignees

No one assigned

Labels

rgw

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@smanjara @clwluvw @cbodley