Conversation
Contributor
There was a problem hiding this comment.
The UUID-idempotent dispatch in applyDDLChange is the right shape for the SERVER-120349 problem and the table-driven tests cover the relevant branches. Two things worth a second look:
- The mock's
SetCollectionUUIDdiscards the UUID argument, so the phantom-create test asserts only that the call happened, not that the catalog was updated with the new event UUID. The whole point of the phantom-create branch is that the stored UUID changes; capturing it in the mock would lock that contract in. - The second
Createswitch case is logically redundant with the first (!uuidEqual(...)is already implied by falling out of the equality case) and quietly classifiescatalogUUID == nil && eventUUID != nil && existsas a phantom create. That state is only reachable after a prior nil-event-UUID create storednilin the catalog, but it's worth confirming the intent — "real create" might be the safer fallback for an entry with no recorded UUID.
Collaborator
Author
|
Closing in favor of the consolidated PR: #222 |
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
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.
Problem
PCSM treated every change-stream
createanddropas real DDL. That is unsafe during a shardedmovePrimary, because MongoDB can emit phantom DDL events for the moved collection without tagging themfromMigrate(SERVER-120349).The visible failure is data loss on the target: PCSM drops the collection and recreates it empty. The reported symptom was a 10-document source collection landing as 0 documents on the target.
Solution
Compare the change event UUID against the catalog UUID before replaying a create or drop.
For
create:movePrimarycreate, so update the catalog UUID and keep the target dataFor
drop:Time-series create handling stays unchanged. This is the core data-preservation part of the PCSM-249 fix.
Testing
TestApplyCreateDDLChangecovers same-UUID no-op, different-UUID phantom create (catalog UUID update only), nil event UUID real create, missing catalog entry real create, and the timeseries skipTestApplyDropDDLChangecovers UUID-mismatch suppression, UUID-match real drop, missing catalog entry real drop, and nil-UUID real dropNotes
Part of PCSM-249. Depends on #242 (catalog UUID reader and movePrimary marker scaffolding). This PR is stacked on that branch and will retarget to
mainonce #242 merges.