-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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-47795 change batchsize descriptions #6292
base: master
Are you sure you want to change the base?
DOCSP-47795 change batchsize descriptions #6292
Conversation
✅ Deploy Preview for mongodb-docs 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.
Thanks for these updates, @shuangela! I've left a few questions and suggestions for your consideration.
Thanks!
Joe
source/reference/command/find.txt
Outdated
limit than 16 MiB, but not a larger one. If you set ``batchSize`` to a limit that results | ||
in batches larger than 16 MiB, this option has no effect. | ||
` | ||
A batchSize of 0 means that the cursor will be established, but no documents |
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.
[tense suggestion]
Consider updating this to use the present tense. Spitballing:
A ``batchsize`` of ``0`` means that the cursor is established, but no documents
are returned in the first batch.
Thanks!
source/reference/command/getMore.txt
Outdated
If ``batchSize`` is not set, ``getMore`` returns up to 16 | ||
mebibytes of data. If ``batchSize`` is set, ``getMore`` returns | ||
the smaller of 16 mebibytes of data or ``batchSize`` documents. | ||
If batchSize is not set, getMore returns up to ``101`` documents in the first batch |
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.
[style suggestion]
Consider wrapping "batchSize" and "getMore" in backticks here on line 77.
source/reference/command/find.txt
Outdated
By default, the ``find`` command has an initial batch size of ``101`` documents and a maximum | ||
size of 16 mebibytes (MiB) for each subsequent batch. This option can enforce a smaller | ||
limit than 16 MiB, but not a larger one. If you set ``batchSize`` to a limit that results | ||
in batches larger than 16 MiB, this option has no effect. |
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.
[behavior question]
Can we clarify what we mean by "no effect" here? I take it that means it uses the default. Thanks!
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.
Yes, it means it will use the default, will clarify!
limit than 16 MiB, but not a larger one. If you set ``batchSize`` to a limit that results | ||
in batches larger than 16 MiB, this option has no effect. | ||
|
||
A batchSize of 0 means that the cursor will be established, but no documents |
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.
[tense suggestion]
Consider updating this to use the present tense. Spitballing:
A ``batchsize`` of ``0`` means that the cursor is established, but no documents
are returned in the first batch.
By default, :method:`~db.collection.watch()` has an initial batch size of ``101`` documents and a maximum size | ||
of 16 mebibytes (MiB) for each subsequent batch. This option can enforce a smaller | ||
limit than 16 MiB, but not a larger one. If you set ``batchSize`` to a limit that | ||
results in batches larger than 16 MiB, this option has no effect. |
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.
[behavior question]
Can we clarify what we mean by "no effect" here? I take it that means it uses the default.
Thanks for the review, @jmd-mongo! Addressed your feedback and applied it to other pages as well, back to 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.
Thanks for these updates, @shuangela! Would you mind wrapping some of these lines to a shorter width? After that, this one LGTM.
Thanks!
Joe
source/reference/command/find.txt
Outdated
- Optional. The number of documents to return in the first batch. | ||
Defaults to 101. A batchSize of 0 means that the cursor will be | ||
established, but no documents will be returned in the first batch. | ||
- Optional. The maximum number of documents within each batch returned in a |
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.
Would you mind wrapping these lines? Thank you! This applies throughout.
@ChrisHarris2012 Thank you for offering to 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.
Why are there different wordings/approaches in the various places? I thought part of the feedback in the Slack thread was that we'd like to be more consistent in our presentation of the information regardless of where it is being presented/consumed?
source/reference/command/getMore.txt
Outdated
If ``batchSize`` is not set, ``getMore`` returns up to ``101`` documents | ||
in the first batch and up to 16 mebibytes (MiB) of data in all |
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.
When a user issues a query (either a find
or an aggregate
) it first is received by the server as an instance of that particular command (eg find). If the full result set is not returned by the server in that initial response (eg the "initial batch") then the server opens up a cursor and tells the client about that as well when responding with the initial results. If the client wants to read more results from that cursor then that is when it issues a getMore command.
All this to say that the text here about "101 results in the initial batch of a getmore" is not correct/relevant
source/reference/command/find.txt
Outdated
established, but no documents will be returned in the first batch. | ||
- Optional. The maximum number of documents within each batch returned | ||
in a query result. By default, the ``find`` command has an initial | ||
batch size of ``101`` documents and a maximum size of 16 mebibytes (MiB) |
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 16MiB (Eng should confirm if MB or MiB, I can't recall) applies to all batches. In other words, by default (assuming more than 1 batch):
- The initial batch is the lesser of 101 docs or 16MiB worth of docs
- Subsequent batches are 16MiB worth of docs
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.
Good catch, thanks!
@ChrisHarris2012 Back to you with the changes! I've tried to make the wording more consistent, however it is not 100% the same across each page because I tried to keep as much of the original content on each doc page as possible, such as keeping the note on the |
DESCRIPTION
Updates five pages in manual/reference to clarify how BatchSize works. No new sections were added, but descriptions of BatchSize were changed.
Wording taken from already updated PHP driver docs which was approved by a PHP engineer.
STAGING
https://deploy-preview-6292--mongodb-docs.netlify.app/reference/method/cursor.batchSize/
https://deploy-preview-6292--mongodb-docs.netlify.app/reference/command/getMore/
https://deploy-preview-6292--mongodb-docs.netlify.app/reference/command/find/
https://deploy-preview-6292--mongodb-docs.netlify.app/reference/method/db.collection.watch/
https://deploy-preview-6292--mongodb-docs.netlify.app/reference/operator/aggregation/queryStats/#output
JIRA
DOCSP-47795
SELF-REVIEW CHECKLIST
See the taxonomy tagging instructions and this example PR
EXTERNAL REVIEW REQUIREMENTS
What's expected of an external reviewer?