-
Notifications
You must be signed in to change notification settings - Fork 43
DOCSP-35923: csot page #657
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-35923: csot page #657
Conversation
✅ Deploy Preview for docs-java 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.
really nice job 💪 lots of small suggestions and fixes, but i trust your judgment. lmk if you want another review
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.
.
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.
It's looking good! Just a question and a minor suggestion.
// start-operation-timeout | ||
MongoClientSettings settings = MongoClientSettings.builder() | ||
.applyConnectionString(new ConnectionString("<connection string>")) | ||
.timeout(5L, SECONDS) |
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.
5 seconds seems a bit long for an insertOne. Do the docs here, or elsewhere, advise on how to properly calibrate various timeouts?
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.
There isn't much I could find in the Server manual or the MongoDB spec re: timeouts. Would something on the order of milliseconds be more appropriate? such as 200 MS or something?
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 guess any value is arbitrary if we don't get into details, so any (including 5s and 200ms) is fine.
We could consider adding documentation (not in this PR, but elsewhere) on properly configuring timeouts. This would not be Java-specific. For example, we might suggest using the defaults, except in certain kinds of cases, which we might give examples of. And suggest that when configuring an operation timeout, one should account for retries and failover, and so on. The goal would be to ensure that customers do not avoid using them when they should use them, and do not use them without considering various implications. But, this is outside of the scope of this PR.
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.
Sure, I can create a ticket for Server docs to add such guidance. I agree that its a good idea!
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.
* DOCSP-35923: csot page * small fix * api docs + fixes * txn code examples * deprecations * list fix * fix * MW + MK PR fixes * MK tech review 2 (cherry picked from commit 133da16)
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-35923
Staging Links
main changes pages:
Self-Review Checklist