Skip to content
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-33862-mongorestore-example #146

Conversation

@jmd-mongo jmd-mongo self-requested a review April 16, 2024 18:12
Copy link
Contributor

@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.

Thanks for this update, @jason-price-mongodb! I've left a non-blocking for your consideration, but otherwise this LGTM.

Thanks!
Joe

source/mongorestore/mongorestore-examples.txt Outdated Show resolved Hide resolved
Copy link
Contributor Author

@jason-price-mongodb jason-price-mongodb left a comment

Choose a reason for hiding this comment

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

@jmd-mongo please re-review. Thanks.

source/mongorestore/mongorestore-examples.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@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.

Thanks, @jason-price-mongodb! This one LGTM.

Thanks,
Joe

@ohassanmongodb
Copy link

Hi there,
I’ve noticed a bit of confusion regarding the Restore Point in Time (RPIT) functionality as mentioned in our documentation. We’ve been getting some feedback from customers using mongorestore. They seem to think that it allows for a point-in-time restore, but actually, mongorestore only supports this feature while mongodump is running the backup. This has caused some misunderstandings because it doesn’t let users restore data from, say, two days ago. I believe this could be misleading, and it might be a good idea to update our documentation to make this clearer.
Thanks for looking into this!

Copy link
Contributor Author

@jason-price-mongodb jason-price-mongodb left a comment

Choose a reason for hiding this comment

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

Updates per ext review.

@jason-price-mongodb
Copy link
Contributor Author

@ohassanmongodb please re-review. Thanks!

@jason-price-mongodb
Copy link
Contributor Author

jason-price-mongodb commented Apr 19, 2024

@mvankeulen94 please review. Thanks!

Copy link

@mvankeulen94 mvankeulen94 left a comment

Choose a reason for hiding this comment

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

I've left a few comments, will flag this so engineering can weigh in as well

To back up a database at a specific point in time to an :term:`oplog`
file, use :option:`mongodump --oplog`. To restore the database at a
point in time with an oplog file, use :option:`mongorestore
--oplogReplay`. For more details, see :ref:`backup-restore-oplogreplay`.

Choose a reason for hiding this comment

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

We should add a note that there are restrictions on when --oplog can be called.
https://jira.mongodb.org/browse/DOCSP-38779 is in progress, which adds additional corrections on when --oplog can safely be called. Ideally we can complete that ticket prior to pushing this.

source/mongorestore/mongorestore-examples.txt Outdated Show resolved Hide resolved
source/mongorestore/mongorestore-examples.txt Outdated Show resolved Hide resolved
source/mongorestore/mongorestore-examples.txt Outdated Show resolved Hide resolved
source/mongorestore/mongorestore-examples.txt Show resolved Hide resolved
@mvankeulen94
Copy link

The root issue with the confusion is we are labeling mongorestore as "point in time", and so users assume that they can choose the specific point in time to which they restore. They cannot do that safely with the DB Tools, so we'd prefer to not advertise this as a recommended use case.

I discussed with the team, and we are in favor of instead updating all these references to "point in time" to state that, unless a user pauses writes for the duration of their mongodump, the only way to get a consistent backup is to use --oplog mode.
It'd also be good to clarify that by "consistent" we mean that the dump is a reflection of the state of the data at a single point in time.

Copy link
Contributor Author

@jason-price-mongodb jason-price-mongodb left a comment

Choose a reason for hiding this comment

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

Updated per feedback.

source/mongorestore/mongorestore-examples.txt Show resolved Hide resolved
source/mongorestore/mongorestore-examples.txt Outdated Show resolved Hide resolved
source/mongorestore/mongorestore-examples.txt Show resolved Hide resolved
@jason-price-mongodb
Copy link
Contributor Author

@mvankeulen94 please re-review. I'll make the various changes to the server docs repo in a different PR. Thanks.

Copy link

@mvankeulen94 mvankeulen94 left a comment

Choose a reason for hiding this comment

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

thanks for handling these changes! I have a few points of clarification

source/mongodump.txt Outdated Show resolved Hide resolved
source/mongodump.txt Outdated Show resolved Hide resolved
source/mongodump.txt Outdated Show resolved Hide resolved
source/mongodump.txt Show resolved Hide resolved
source/mongorestore.txt Outdated Show resolved Hide resolved
source/mongorestore.txt Outdated Show resolved Hide resolved
source/mongorestore/mongorestore-examples.txt Show resolved Hide resolved
Copy link
Contributor Author

@jason-price-mongodb jason-price-mongodb left a comment

Choose a reason for hiding this comment

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

Updates per external review.

source/mongodump.txt Outdated Show resolved Hide resolved
source/mongodump.txt Outdated Show resolved Hide resolved
source/mongodump.txt Outdated Show resolved Hide resolved
source/mongodump.txt Show resolved Hide resolved
source/mongorestore.txt Outdated Show resolved Hide resolved
source/mongorestore.txt Outdated Show resolved Hide resolved
source/mongorestore/mongorestore-examples.txt Show resolved Hide resolved
@jason-price-mongodb
Copy link
Contributor Author

@mvankeulen94 please re-review. Thanks.

Copy link

@mvankeulen94 mvankeulen94 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! This is looking great. I just have a couple more changes - I'll also share the PR with TAR engineering

source/mongodump.txt Outdated Show resolved Hide resolved
source/mongorestore/mongorestore-examples.txt Outdated Show resolved Hide resolved
Copy link
Member

@tfogo tfogo left a comment

Choose a reason for hiding this comment

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

Hey, I think we're overindexing on the specific misunderstanding the user in DOCSP-33862 had. It feels like we're trying to treat the symptom (the misunderstanding in DOCSP-33862) instead of treating the cause (we don't explain what mongodump does clearly enough).

I think that understanding how mongodump will dump inconsistent data without using --oplog is integral to understanding the tools. We should probably explain how this works more fully and it should probably be up front in the behavior section and even the main page.

Could we do more of a larger rewrite here? I think as it stands this PR is adding complexity and not tackling the root of the problem.

source/mongorestore.txt Outdated Show resolved Hide resolved
Copy link
Contributor Author

@jason-price-mongodb jason-price-mongodb left a comment

Choose a reason for hiding this comment

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

Updates per external review

source/mongodump.txt Outdated Show resolved Hide resolved
source/mongorestore.txt Outdated Show resolved Hide resolved
source/mongorestore/mongorestore-examples.txt Outdated Show resolved Hide resolved
@jason-price-mongodb
Copy link
Contributor Author

@mvankeulen94 @tfogo please re-review. Thanks.

Copy link
Member

@tfogo tfogo left a comment

Choose a reason for hiding this comment

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

LGTM as long as @mvankeulen94 is happy.

I can file the tickets to make the larger changes to the dump/restore docs.

Copy link

@mvankeulen94 mvankeulen94 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! Per Slack conversation, if we could wait on publishing these until https://jira.mongodb.org/browse/DOCSP-37996 is complete, that would be great.

@jason-price-mongodb jason-price-mongodb merged commit 7c428ab into mongodb:master May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants