-
Notifications
You must be signed in to change notification settings - Fork 35
DOCSP-48515-Source-destination-balancing #678
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
DOCSP-48515-Source-destination-balancing #678
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.
one suggestion/question
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!
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.
Left one comment–curious what Maria thinks since she filed the ticket (cc mvankeulen94)
@@ -113,6 +113,13 @@ destination cluster. | |||
Chunk Distribution | |||
'''''''''''''''''' | |||
|
|||
.. important:: | |||
|
|||
Even if ``mongosync`` balances the source cluster, it does not |
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 this language might be a little confusing–mongosync itself doesn't balance the source cluster, it's really that the source cluster was balanced prior to mongosync. This might read better if use the language from the jira ticket like "preserve chunk distribution from the source to the destination", because I think what we're really saying is that even if the source is balanced before mongosync, mongosync doesn't guarantee the destination cluster will be balanced.
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.
thanks, agree with Nick
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.
The distinction we want to draw is, even if the source cluster is itself already properly balanced prior to mongosync being called, this pre-existing balance does not help the destination cluster get balanced because mongosync does not preserve the sharding config
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! I left one small suggestion but it's up to you. Thanks for your work on this Maya!
.. important:: | ||
|
||
Even if the source cluster is balanced, ``mongosync`` does not | ||
ensure balance of the destination cluster. To balance the destination |
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.
Since Sharding operations are not permitted during the execution of mongosync (see Limitations), you must wait until it is safe to accept writes in order to rebalance the destination cluster. See Sharded Cluster Balancer for guidance on how to rebalance the cluster.
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.
Suggest including the above instead of the sentence "To balance the destination [...]"
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.
once this last comment is addressed I am good to approve, thanks!
Even if the source cluster is balanced, ``mongosync`` does not | ||
ensure balance of the destination cluster. Because ``mongosync`` | ||
does not support the execution of sharding operations during | ||
migration, you must wait until it is safe to accept writes |
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 add the link to Limitations from this comment
once that change is added I am good to approve, thank you!
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 with a couple non-blocking nits!
@@ -113,6 +113,16 @@ destination cluster. | |||
Chunk Distribution | |||
'''''''''''''''''' | |||
|
|||
.. important:: | |||
|
|||
Even if the source cluster is balanced, ``mongosync`` does not |
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.
[nit] We can use contractions in our docs for simplicity!
Even if the source cluster is balanced, ``mongosync`` does not | |
Even if the source cluster is balanced, ``mongosync`` doesn't |
Same goes for the instance below!
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, thanks!
* commit * feedback * feedbaclk * feedback * feedback * link * contractions and ref (cherry picked from commit c1b61ee)
* commit * feedback * feedbaclk * feedback * feedback * link * contractions and ref (cherry picked from commit c1b61ee)
* commit * feedback * feedbaclk * feedback * feedback * link * contractions and ref (cherry picked from commit c1b61ee)
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
* commit * feedback * feedbaclk * feedback * feedback * link * contractions and ref (cherry picked from commit c1b61ee)
* commit * feedback * feedbaclk * feedback * feedback * link * contractions and ref (cherry picked from commit c1b61ee)
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
* commit * feedback * feedbaclk * feedback * feedback * link * contractions and ref (cherry picked from commit c1b61ee)
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
https://jira.mongodb.org/browse/DOCSP-48515
Staging: https://deploy-preview-678--docs-cluster-to-cluster-sync.netlify.app/reference/mongosync/mongosync-behavior/#chunk-distribution