-
Notifications
You must be signed in to change notification settings - Fork 39
DOCSP-45863-embedded-verifier-migrations #557
DOCSP-45863-embedded-verifier-migrations #557
Conversation
✅ Deploy Preview for docs-cluster-to-cluster-sync ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, thanks Ali!
Hey @ajayvijayakumar123 and @mvankeulen 👋 Whenever you get the chance, could you both review this docs PR for noting verifier changes for pre-6.0 migrations? TIA! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions!
|
||
.. include:: /includes/fact-verifier-unsupported | ||
|
||
.. note:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we also include this note:
“Mongosync preserves document field-order based on what it reads using read preference primary from the source cluster. The embedded verifier will verify documents based on the source cluster’s primary node. It will not be able to verify discrepancies among the different source cluster nodes. In rare cases, discrepancies among the different source cluster’s nodes will cause the embedded verifier to fail a migration.”
at the top of the page? This applies to all migrations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm % changing In rare cases, discrepancies among the different source cluster’s nodes will cause the embedded verifier to fail a migration
to In rare cases, discrepancies in document field order among the source cluster’s nodes will cause the embedded verifier to fail a migration even if mongosync copied documents correctly
It may also make sense to mention that the only case where this can happen is if an election occurs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we note that this issue can only occur in rare cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to double check my understanding is it that any time an election occurs this can happen or just a small portion of elections? Mainly want to make sure that we're clearing on distinguishing that if we're mentioning "rare cases" @ajayvijayakumar123 @rohshar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discrepancies among different source cluster nodes only happen in very rare cases. Even if there is a discrepancy, an election may not even surface the issue and cause the verifier to fail. I think we can just say that this issue is rare in general.
Thanks for the review @ajayvijayakumar123. Just finished adding in the changes you suggested. For the note that applies to all migrations, I tweaked the wording a bit, but let me know if it's changed the meaning too much. Thanks again! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a couple more suggestions!
|
||
.. include:: /includes/fact-verifier-unsupported | ||
|
||
.. note:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we note that this issue can only occur in rare cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! @rohshar Can you be the second reviewer for this PR? (since Maria's out sick)
``mongosync`` reads using :readmode:`primary` read preference, so it | ||
preserves document field order from the source cluster's primary node. The | ||
embedded verifier also checks documents based on the source cluster’s primary | ||
node. Because of this read preference, the verifier cannot verify | ||
discrepancies between the different source cluster nodes. In rare cases, | ||
discrepancies in document field order between the source cluster’s nodes will | ||
cause the embedded verifier to fail the migration, even if ``mongosync`` | ||
copied the documents correctly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
``mongosync`` reads using :readmode:`primary` read preference, so it | |
preserves document field order from the source cluster's primary node. The | |
embedded verifier also checks documents based on the source cluster’s primary | |
node. Because of this read preference, the verifier cannot verify | |
discrepancies between the different source cluster nodes. In rare cases, | |
discrepancies in document field order between the source cluster’s nodes will | |
cause the embedded verifier to fail the migration, even if ``mongosync`` | |
copied the documents correctly. | |
``mongosync`` reads using :readmode:`primary` read preference, so it | |
preserves document field order from the source cluster's primary node. The | |
embedded verifier also checks documents based on the source cluster’s primary | |
node, but at a different time than mongosync reads them. Because of this, in rare cases, | |
discrepancies in document field order between the source cluster’s nodes will | |
cause the embedded verifier to fail the migration, even if ``mongosync`` | |
copied the documents correctly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think about this change @ajayvijayakumar123
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup that sounds good. should we also note that discrepancies among cluster nodes themselves are rare?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's already implied in the suggested text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, thanks both. Updated.
59070e0
into
mongodb:feature-older-version-support
* DOCSP-45860-older-version-limitations (#522) * DOCSP-45860-older-version-limitations * * * JA feedback * * * * * * * AV feedback * MvK feedback * DOCSP-45859 Version Compatibility (#518) * DOCSP-45859 Version Compatibility * * * * * * * AV feedback * swap list order * MvK feedback * DOCSP-45861 Older Version Support Task Page (#533) * DOCSP-45861 Older Version Support Task Page * add prerequisites & limitations * * * * * procedures * * * rn * * * remove extra step * nit * AV feedback * remove task page * * * * * * * DOCSP-45862 User permissions older version (#552) * DOCSP-45862-user-permissions * * * JD feedback * AV feedback * DOCSP-45863-embedded-verifier-migrations (#557) * DOCSP-45863-embedded-verifier-migrations * * * * * AV feedback * wording * add ddl limitation * * * RS feedback * * * remove merge conflict artifact * fix link title
DESCRIPTION
Adds a note that during pre-6.0 migrations, the verifier checks metadata and indexes only for DDL events.
STAGING
https://deploy-preview-557--docs-cluster-to-cluster-sync.netlify.app/reference/verification/embedded/#unsupported-verification-checks
JIRA
https://jira.mongodb.org/browse/DOCSP-45863