-
Notifications
You must be signed in to change notification settings - Fork 21
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-44422 - Bulk write API #257
Conversation
✅ Deploy Preview for mongodb-docs-csharp 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 solid start!
Consider a scenario in which you want to insert a document into a collection, | ||
update multiple other documents, then delete a document. If you use | ||
individual methods, each operation requires its own database call. This guide | ||
shows you how to use bulk write operations to perform multiple write operations | ||
in a single database call. |
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.
S: Reshuffle sentences so first sentence is more expository:
Consider a scenario in which you want to insert a document into a collection, | |
update multiple other documents, then delete a document. If you use | |
individual methods, each operation requires its own database call. This guide | |
shows you how to use bulk write operations to perform multiple write operations | |
in a single database call. | |
This guide shows you how to use the {+driver-short+} to perform **bulk write operations** | |
that include multiple write operations in a single database call. | |
Consider a scenario in which you want to insert a document into a collection, | |
update multiple other documents, then delete a document. If you use | |
individual methods, each operation requires its own database call. By using bulk write operations, | |
you can reduce the number of individual database calls to improve efficiency, and perform operations | |
on multiple namespaces in your deployment... |
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.
You should also highlight that this is Client
bulkwrite
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 didn't know there was another kind. The server docs imply that bulk writes are always client-initiated
The examples in this guide use ``BsonDocument`` as the type for ``TDocument``. | ||
You can also use a Plain Old CLR Object (POCO) as the type for ``TDocument``. |
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.
S: The phrasing "type for" is a bit vague. Perhaps you could be more descriptive about the relationship
eg use BsonDocument
as the type that implements TDocument
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.
reworded. I think the inheritance relationship will be clear to any dev who would be using this page
To use a POCO, define a class that represents the documents in your collection. | ||
The class must have properties that match the fields in your documents. | ||
For more information, see :ref:`<csharp-poco>`. |
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.
To use a POCO, define a class that represents the documents in your collection. | |
The class must have properties that match the fields in your documents. | |
For more information, see :ref:`<csharp-poco>`. | |
If using a POCO, you must define a class that represents the documents in your collection. | |
The class must have properties that match the fields in your documents. | |
For more information, see :ref:`<csharp-poco>`. |
class. The | ||
``BsonDocument`` object represents a new restaurant named "Mongo's Deli" to be inserted | ||
into the ``sample_restaurants.restaurants`` collection. |
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.
S: try to reduce the double references to document field values and real world things (documents vs restaurants)
class. The | |
``BsonDocument`` object represents a new restaurant named "Mongo's Deli" to be inserted | |
into the ``sample_restaurants.restaurants`` collection. | |
class. The model represents a document in which the name is ``"Mongo's Deli"`` that you want to insert | |
into the ``sample_restaurants.restaurants`` collection. |
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.
Could you elaborate on what to avoid? I'm trying to think of another way to say it, because I'd normally use "model" in the context of data models; i.e., a POCO, or class that mirrors a specific document structure.
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 meant that we might prefer to avoid interchangeably referring to documents/C# types and restaurants.
Looking back on my suggestion, it seems a bit clunky. Maybe something like:
The
BulkWriteInsertOneModel
directs the driver to insert a document in which thename
field value is"Mongo's Deli"
into therestaurants
collection.
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.
That works! I didn't find any other uses of the word "restaurant", but let me know if anything else needs to change.
.. tip:: Troubleshooting Bulk Write Operations | ||
|
||
If any of the write operations fail, the {+driver-short+} raises a | ||
``BulkWriteException`` and does not perform any further operations. You can examine | ||
the properties of the exception to determine which operation failed. |
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.
S: Is this note necessary? It doesn't provide specific guidance on troubleshooting
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.
Maybe "troubleshooting" isn't the best title. the point is really the first sentence.
.. note:: Unordered Bulk Write Operations | ||
|
||
Unordered bulk operations do not guarantee order of execution. The order can | ||
differ from the way you list them to optimize the runtime. | ||
|
||
If any of the write operations in an unordered bulk write fail, the driver | ||
reports the errors only after attempting all operations. |
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.
S: move this note directly under the table
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.
moved some things around
new BulkWriteUpdateManyModel<BsonDocument>( | ||
,collection |
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.
S: something looks off here
new BulkWriteUpdateManyModel<BsonDocument>( | |
,collection | |
new BulkWriteUpdateManyModel<BsonDocument>( | |
collection, |
source/whats-new.txt
Outdated
- Adds an API for bulk write operations. To learn more about bulk write operations, see | ||
:ref:`csharp-bulk-write`. |
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.
- Adds an API for bulk write operations. To learn more about bulk write operations, see | |
:ref:`csharp-bulk-write`. | |
- Adds an API for bulk write operations at the client level. To learn more about bulk write operations, see | |
:ref:`csharp-bulk-write`. |
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.
Same question as above re client bulk writes
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 is a bulk write method at the collection level: https://mongodb.github.io/mongo-csharp-driver/3.0.0/api/MongoDB.Driver/MongoDB.Driver.IMongoCollection-1.BulkWrite.html
I think that v3 introduces "client bulk write", through a method at the client level: https://mongodb.github.io/mongo-csharp-driver/3.0.0/api/MongoDB.Driver/MongoDB.Driver.IMongoClient.BulkWrite.html
This enables you to include writes to multiple namespaces in a deployment, which is not possible in collection bulk write.
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.
Ah, I see--so "client" as opposed to "collection" rather than "server". good callout
good notes. think i got everything, except where discussion still ongoing |
Co-authored-by: Rea Rustagi <[email protected]>
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.
Answered some comments, can take another look when you respond/resolve the threads!
class. The | ||
``BsonDocument`` object represents a new restaurant named "Mongo's Deli" to be inserted | ||
into the ``sample_restaurants.restaurants`` collection. |
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 meant that we might prefer to avoid interchangeably referring to documents/C# types and restaurants.
Looking back on my suggestion, it seems a bit clunky. Maybe something like:
The
BulkWriteInsertOneModel
directs the driver to insert a document in which thename
field value is"Mongo's Deli"
into therestaurants
collection.
source/whats-new.txt
Outdated
- Adds an API for bulk write operations. To learn more about bulk write operations, see | ||
:ref:`csharp-bulk-write`. |
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 is a bulk write method at the collection level: https://mongodb.github.io/mongo-csharp-driver/3.0.0/api/MongoDB.Driver/MongoDB.Driver.IMongoCollection-1.BulkWrite.html
I think that v3 introduces "client bulk write", through a method at the client level: https://mongodb.github.io/mongo-csharp-driver/3.0.0/api/MongoDB.Driver/MongoDB.Driver.IMongoClient.BulkWrite.html
This enables you to include writes to multiple namespaces in a deployment, which is not possible in collection bulk write.
* - Property | ||
- Description | ||
|
||
* - ``Acknowledged`` |
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.
Should we mention that in case of unacknowledged results (Acknowledged == false) all other properties throws?
.. tip:: | ||
|
||
If any of the write operations fail, the {+driver-short+} raises a | ||
``BulkWriteException`` and does not perform any further operations. You can examine |
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 should be ClientBulkWriteException instead.
------------ | ||
|
||
The ``BulkWrite()`` and ``BulkWriteAsync()`` methods return a ``ClientBulkWriteResult`` | ||
object that contains the following properties: |
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.
Should we describe ClientBulkWriteException
so users know how to find error responses/partial results? Based on the spec: https://github.com/mongodb/specifications/blob/master/source/crud/bulk-write.md#exception
) | ||
}; | ||
|
||
var results = client.BulkWriteSync(bulkWriteModels); |
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.
BulkWrite
we do not use Sync suffix here.
Console.WriteLine("Bulk write results: " + results); | ||
// end-bulk-write-sync | ||
} | ||
static async void BulkWriteAsync() |
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 better be: async Task
. Even though async void
is possible it isn't recommended to use.
VerboseResult = true | ||
}; | ||
|
||
var results = client.BulkWriteSync(deleteOneModel, clientBulkWriteOptions); |
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.
BulkWrite
, there is no Sync suffix.
var results = client.BulkWriteSync(deleteOneModel, clientBulkWriteOptions); | ||
// end-bulk-write-options-sync | ||
} | ||
static async void BulkWriteOptionsAsync() |
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.
async Task
instead of async void
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.
LGTM
Co-authored-by: Rea Rustagi <[email protected]> (cherry picked from commit 12fc8ef)
Co-authored-by: Rea Rustagi <[email protected]> (cherry picked from commit 12fc8ef) Co-authored-by: Mike Woofter <[email protected]>
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-44422
Staging Links
Self-Review Checklist