Skip to content
This repository was archived by the owner on May 1, 2025. It is now read-only.

Conversation

@kanchana-mongodb
Copy link
Collaborator

@kanchana-mongodb kanchana-mongodb commented Apr 3, 2024

Copy link

@tdq45gj tdq45gj left a comment

Choose a reason for hiding this comment

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

I left some comments. We swapped step 3 and step 4 in the draft doc on Monday. Sorry if it caused confusion!

Comment on lines 67 to 69
Ensure that the ``mongosync`` process response for the
``commit`` request indicates that the ``mongosync`` state is
``COMMITTING`` or ``COMMITTED``.
Copy link

Choose a reason for hiding this comment

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

Should we clarify in the note that the state is COMMITTING or COMMITTED after sending the commit request?

Comment on lines 54 to 57
Call the :ref:`progress <c2c-api-progress>` endpoint to determine
if ``canWrite`` is ``true``. If ``canWrite`` is ``false``, wait
until :ref:`progress <c2c-api-progress>` shows ``canWrite`` is
``true``.
Copy link

Choose a reason for hiding this comment

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

The purpose of this step (checking canWrite=true) is to check the prerequisite to enable application writes. Enable application writes on the destination cluster should be immediately after this step.


When the ``mongosync`` progress response indicates that the
``mongosync`` state is ``COMMITTED``, the cutover process is
complete. You can now :ref:`reverse <c2c-api-reverse>` the sync
Copy link

Choose a reason for hiding this comment

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

Reverse is not a step of the cutover process. It's an option for users to fall back to use the source after the cutover process. Most users wouldn't need it. Right now it seems to be a required action in step 6. Can we clarify that it's an option after the cutover process, not a necessary step, or remove this information if you feel it doesn't fit into this page?

Copy link

@tdq45gj tdq45gj left a comment

Choose a reason for hiding this comment

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

The current sequence of steps still doesn't seem correct to me. The sequence should be:

  1. Verify the status of the mongosync process.
  2. Stop any write operations to the synced collections on the source
  3. Send a commit request to mongosync.
  4. Wait for canWrite to become true
  5. Enable application writes on the destination cluster.
  6. Call the progress endpoint to determine the status of the mongosync process.

that write operations are stopped for the collections included
by the filter.

.. step:: Call the :ref:`progress <c2c-api-progress>` endpoint.
Copy link

Choose a reason for hiding this comment

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

Suggested change: Wait for canWrite to become true.

@kanchana-mongodb kanchana-mongodb requested a review from tdq45gj April 3, 2024 20:52
Copy link

@tdq45gj tdq45gj left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@jmd-mongo jmd-mongo self-requested a review April 4, 2024 14:06
Copy link
Collaborator

@jmd-mongo jmd-mongo left a comment

Choose a reason for hiding this comment

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

Thank you for this addition, @kanchana-mongodb! I've left some suggestions and a question for your consideration.

Thank you!
Joe


.. step:: Wait until you can perform writes on the destination cluster.

Call the :ref:`progress <c2c-api-progress>` endpoint to determine
Copy link
Collaborator

Choose a reason for hiding this comment

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

[linking suggestion]

Consider using backticks around "progress" instead of linking as this is already linked above.


.. step:: Enable application writes on the destination cluster.

.. step:: Call the :ref:`progress <c2c-api-progress>` endpoint to determine the status of the ``mongosync`` process.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar comment as on line 67.


Call the :ref:`progress <c2c-api-progress>` endpoint to determine
if ``canWrite`` is ``true``. If ``canWrite`` is ``false``, wait
until :ref:`progress <c2c-api-progress>` shows ``canWrite`` is
Copy link
Collaborator

Choose a reason for hiding this comment

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

Simliar comment as on line 67.

until :ref:`progress <c2c-api-progress>` shows ``canWrite`` is
``true``.

.. step:: Enable application writes on the destination cluster.
Copy link
Collaborator

Choose a reason for hiding this comment

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

[question]

Should there be a code example or blurb here showing how to enable writes on the destination cluster?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you!

Copy link
Collaborator

@jmd-mongo jmd-mongo left a comment

Choose a reason for hiding this comment

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

Thank you for these changes, @kanchana-mongodb! This issue LGTM mod a minor code formatting nit.

Thank you!
Joe


.. code-block:: javascript

db.adminCommand(
Copy link
Collaborator

Choose a reason for hiding this comment

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

[code formatting suggestion]

I recommend using formatting similar to the following. It's a minor change to be less indented.

db.adminCommand(
   {
      setUserWriteBlockMode: 1,
      global: false
   }
)

until :ref:`progress <c2c-api-progress>` shows ``canWrite`` is
``true``.

.. step:: Enable application writes on the destination cluster.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you!

@mdb-ashley mdb-ashley merged commit 3074037 into mongodb:master Apr 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants