Skip to content

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

Merged
merged 12 commits into from
Mar 31, 2025
Merged

Conversation

rustagir
Copy link
Contributor

@rustagir rustagir commented Mar 20, 2025

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-35923

Staging Links

main changes pages:

Self-Review Checklist

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?
  • Are all the links working?
  • Are the facets and meta keywords accurate?

Copy link

netlify bot commented Mar 20, 2025

Deploy Preview for docs-java ready!

Name Link
🔨 Latest commit a5d5edc
🔍 Latest deploy log https://app.netlify.com/sites/docs-java/deploys/67eaa5104c3d1d00085762ba
😎 Deploy Preview https://deploy-preview-657--docs-java.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@rustagir rustagir marked this pull request as ready for review March 20, 2025 17:19
@rustagir rustagir requested a review from a team as a code owner March 20, 2025 17:19
@rustagir rustagir requested review from vbabanin and katcharov and removed request for a team and vbabanin March 20, 2025 17:19
Copy link
Contributor

@mongoKart mongoKart left a 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

Copy link
Collaborator

@katcharov katcharov left a comment

Choose a reason for hiding this comment

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

.

@rustagir rustagir requested a review from katcharov March 24, 2025 18:11
Copy link
Collaborator

@katcharov katcharov left a 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)
Copy link
Collaborator

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?

Copy link
Contributor Author

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?

Copy link
Collaborator

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.

Copy link
Contributor Author

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rustagir rustagir requested a review from katcharov March 26, 2025 21:23
@rustagir rustagir merged commit 133da16 into mongodb:master Mar 31, 2025
5 checks passed
rustagir added a commit that referenced this pull request Mar 31, 2025
* 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)
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.

3 participants