Skip to content

RGW: Pass and check destination zone trace when fetching object#51715

Merged
cbodley merged 1 commit intoceph:mainfrom
awojno-bloomberg:replication-race-condition-fix
Jun 9, 2023
Merged

RGW: Pass and check destination zone trace when fetching object#51715
cbodley merged 1 commit intoceph:mainfrom
awojno-bloomberg:replication-race-condition-fix

Conversation

@awojno-bloomberg
Copy link
Copy Markdown
Contributor

@awojno-bloomberg awojno-bloomberg commented May 23, 2023 •
edited
Loading

The proposed fix for a race condition in multisite during full sync involving deletes. A custom header was added passing the destination zone's trace string and is then checked against the object's replication trace. If there is a match, a 304 response is returned which will prevent full sync from overwriting a deleted object. Please see the tracker for more details.

Contribution Guidelines

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
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@github-actions github-actions bot added the rgw label May 23, 2023
@awojno-bloomberg awojno-bloomberg force-pushed the replication-race-condition-fix branch from d8c4255 to ca2bdeb Compare May 23, 2023 16:01
@awojno-bloomberg awojno-bloomberg marked this pull request as ready for review May 24, 2023 14:01
@awojno-bloomberg awojno-bloomberg requested a review from a team as a code owner May 24, 2023 14:01
@mattbenjamin mattbenjamin requested a review from mkogan1 May 24, 2023 14:38
cbodley
cbodley reviewed May 24, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@cbodley cbodley left a comment

Choose a reason for hiding this comment

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

looks great!

@awojno-bloomberg awojno-bloomberg force-pushed the replication-race-condition-fix branch from ca2bdeb to 201d7e5 Compare May 24, 2023 18:58
cbodley
cbodley reviewed May 24, 2023
View reviewed changes
@awojno-bloomberg awojno-bloomberg force-pushed the replication-race-condition-fix branch from 201d7e5 to e70fcf9 Compare May 24, 2023 20:21
@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Jun 1, 2023

jenkins test api

@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Jun 1, 2023

jenkins test make check

@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Jun 1, 2023

@awojno-bloomberg looks like all of the required checks have passed 👍

@mkogan1
Copy link
Copy Markdown
Contributor

mkogan1 commented Jun 5, 2023

@mattbenjamin mattbenjamin requested a review from mkogan1 2 weeks ago

sorry still testing, the inconsistent state between zones keeps occuring with this PR on one test setup:
(but OK on other - still bisecting to find what is the difference between environments)

# main + this PR commit cherry-pick:
git branch -vv
* main                  2bc45cd9ef1 [origin/main: ahead 1] rgw: Pass and check destination zone trace when fetching object

❯ glo | grep -E 'rgw: Pass and check destination zone trace when fetching object|4023a15|9ec4fd7'
28ea361f867 (HEAD -> main) rgw: Pass and check destination zone trace when fetching object
9ec4fd7e497 rgw: fetch_remote_obj() adds x-rgw-replicated-from headers
4023a157add rgw/rados: pass source zone trace into fetch_remote_obj()

image
although the metadata and data are caught up, the object number in the data pool is not the same -
left - primary zone, right secondary zone:

mkogan1
mkogan1 approved these changes Jun 7, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@mkogan1 mkogan1 left a comment

Choose a reason for hiding this comment

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

image
updating following more testing, that the rados df stats mismatch is an intermittent temporary rados stats artifact which in above screenshot case corrected itself after 3 hours.

switched to radosgw-admin bucket list ... methodology for object count validation and result are immediate, consistent and correct.

LGTM

@awojno-bloomberg
Copy link
Copy Markdown
Contributor Author

awojno-bloomberg commented Jun 7, 2023

image updating following more testing, that the rados df stats mismatch is an intermittent temporary rados stats artifact which in above screenshot case corrected itself after 3 hours.

switched to radosgw-admin bucket list ... methodology for object count validation and result are immediate, consistent and correct.

LGTM

Great! Thanks for all the testing!

@mkogan1
Copy link
Copy Markdown
Contributor

mkogan1 commented Jun 7, 2023

small addendum, found that the cause of the temporary rados df object count discrepancy was gc's objects, resolved by radosgw-admin gc process --include-all
image

@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Jun 9, 2023

passed qa in https://pulpito.ceph.com/cbodley-2023-06-08_19:52:02-rgw-wip-cbodley-testing-distro-default-smithi/

@cbodley cbodley merged commit e6550c1 into ceph:main Jun 9, 2023
@mkogan1 mkogan1 mentioned this pull request Jun 18, 2023
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cbodley cbodley cbodley approved these changes

@mkogan1 mkogan1 mkogan1 approved these changes

Assignees

No one assigned

Labels

bug-fix needs-qa rgw wip-cbodley-testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@awojno-bloomberg @cbodley @mkogan1