DOCSP-34008: Split large change events #374

norareidy · 2024-12-12T17:10:57Z

Pull Request Info

PR Reviewing Guidelines

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

Staging Links

fundamentals/crud/read-operations/change-streams

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?

netlify · 2024-12-12T17:11:13Z

✅ Deploy Preview for mongodb-docs-csharp ready!

Name	Link
🔨 Latest commit	`9a03ab7`
🔍 Latest deploy log	https://app.netlify.com/sites/mongodb-docs-csharp/deploys/676ed0ae7a68590008bc5935
😎 Deploy Preview	https://deploy-preview-374--mongodb-docs-csharp.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

mcmorisi

LGTM with some nits!

mcmorisi · 2024-12-12T20:46:38Z

source/fundamentals/crud/read-operations/change-streams.txt

+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If your application generates change events that exceed 16 MB in size, the
+{+driver-short+} returns an error. To avoid this error, you can use


Nit: Do we know the type of error it returns?

The server error is BSONObjectTooLarge, I'll include that

mcmorisi · 2024-12-12T20:53:56Z

source/includes/code-examples/change-streams/change-streams.cs

+var pipeline = new EmptyPipelineDefinition<ChangeStreamDocument<Restaurant>>()
+    .ChangeStreamSplitLargeEvent();
+
+using (var cursor = await _restaurantsCollection.WatchAsync(pipeline))


Nit: Not limited to this PR but it looks like some examples on this page use collection and some use _restaurantsCollection – should we standardize the name?

mcmorisi · 2024-12-12T20:55:18Z

source/fundamentals/crud/read-operations/change-streams.txt

+   To learn more about splitting large change events, see the
+   :manual:`$changeStreamSplitLargeEvent </reference/operator/aggregation/changeStreamSplitLargeEvent/>`
+   page in the {+mdb-server+} manual.


Nit based on how other Server doc links on this page word this.

Suggested change

To learn more about splitting large change events, see the

:manual:`$changeStreamSplitLargeEvent </reference/operator/aggregation/changeStreamSplitLargeEvent/>`

page in the {+mdb-server+} manual.

To learn more about splitting large change events, see

:manual:`$changeStreamSplitLargeEvent </reference/operator/aggregation/changeStreamSplitLargeEvent/>`

in the {+mdb-server+} manual.

papafe · 2024-12-17T11:39:53Z

source/fundamentals/crud/read-operations/change-streams.txt

+
+If your application generates change events that exceed 16 MB in size, the
+server returns a ``BSONObjectTooLarge`` error. To avoid this error, you can use
+the ``$changeStreamSplitLargeEvent`` pipeline stage to split the events


Here we talk about the $changeStreamSplitLargeEvent, but we do not explicitly say that we have it as a step in the typed aggregation api, called ChangeStreamSplitLargeEvent. We mention this only in the code at the bottom of this section. I think it would be better to make this clear from the start

papafe · 2024-12-17T11:40:33Z

source/fundamentals/crud/read-operations/change-streams.txt

+the ``$changeStreamSplitLargeEvent`` pipeline stage to split the events
+into smaller fragments.
+
+After you receive the change stream event fragments, you can use the


I think it would be better to show how to use ChangeStreamSplitLargeEvent first (as done in the last code section) an then show how to merge fragments, as this is a secondary aspect.

Since I use the helper methods in the ChangeStreamSplitLargeEvent example, I decided to introduce the helper methods first - but the actual split event code example is the focus of the section so it does make sense to include it first. Updated!

papafe · 2024-12-17T11:54:59Z

source/includes/code-examples/change-streams/change-streams.cs

@@ -65,6 +65,115 @@ await cursor.ForEachAsync(change =>
 }
 // end-change-stream-pipeline

+// start-split-event-helpers-sync
+// Fetches the next complete change stream event
+private static ChangeStreamDocument<TDocument> GetNextChangeStreamEvent<TDocument>(


@BorisDog, can you take a look at the code examples? It seems you've worked on the implementation of this so you can better spot errors

papafe · 2024-12-17T11:55:23Z

source/includes/code-examples/change-streams/change-streams.cs

+
+using (var cursor = collection.Watch(pipeline))
+{
+    using (var enumerator = cursor.ToEnumerable().GetEnumerator())


@BorisDog do we need to do all of this here?

Short answer: unfortunately yes.
We just demonstrate how to merge the events if the user chooses to. Our spec does not prescribe any merging functionality, so we did not implement a shortcut for that in our API. We also didn't have any requests for that yet.

BorisDog · 2024-12-21T00:28:15Z

source/includes/code-examples/change-streams/change-streams.cs

+
+using (var cursor = collection.Watch(pipeline))
+{
+    using (var enumerator = cursor.ToEnumerable().GetEnumerator())


Short answer: unfortunately yes.
We just demonstrate how to merge the events if the user chooses to. Our spec does not prescribe any merging functionality, so we did not implement a shortcut for that in our API. We also didn't have any requests for that yet.

BorisDog · 2024-12-21T00:29:50Z

source/fundamentals/crud/read-operations/change-streams.txt

+change events that exceed the 16 MB limit. The code prints the
+change document for each event and calls helper methods to
+reassemble any event fragments:
+


We probably should mention that fragments reassembling is optional (but probably needed), and the split events can be watched as usual events as they arrive.

BorisDog · 2024-12-21T00:42:12Z

source/includes/code-examples/change-streams/change-streams.cs

+private static async Task<ChangeStreamDocument<TDocument>> GetNextChangeStreamEvent<TDocument>(
+    IAsyncCursor<ChangeStreamDocument<TDocument>> changeStreamCursor)
+{
+    var changeStreamEvent = changeStreamCursor.Current.First();


The Async path needs to be enumerated differently,
The whole batch returned by Current needs to be inspected. There are few alternatives to do so, I'll share an example later if you don't find one beforehand.

Shared the code sample in slack.

BorisDog

Some changes requested

BorisDog

LGTM! + minor formatting comment.

BorisDog · 2024-12-26T21:44:00Z

source/includes/code-examples/change-streams/change-streams.cs

+    .ChangeStreamSplitLargeEvent();
+
+using var cursor = collection.Watch(pipeline);
+ foreach (var completeEvent in GetNextChangeStreamEvent(cursor.ToEnumerable().GetEnumerator()))


Unnecessary one space indentation in foreach block.

BorisDog · 2024-12-26T21:44:04Z

source/includes/code-examples/change-streams/change-streams.cs

+    .ChangeStreamSplitLargeEvent();
+
+using var cursor = await collection.WatchAsync(pipeline);
+ await foreach (var completeEvent in GetNextChangeStreamEventAsync(cursor))


Unnecessary one space indentation in foreach block.

* DOCSP-34008: Split large change events * fixes * edits * fixes to async code * fixes to sync code * wording * admonition * small fix * MM feedback * FP feedback * tech feedback * edits * final tech feedback (cherry picked from commit e721d24)

* DOCSP-34008: Split large change events * fixes * edits * fixes to async code * fixes to sync code * wording * admonition * small fix * MM feedback * FP feedback * tech feedback * edits * final tech feedback

DOCSP-34008: Split large change events

811ec32

norareidy added 7 commits December 12, 2024 13:28

fixes

a9c6e48

edits

9904225

fixes to async code

b840209

fixes to sync code

88dcab3

wording

2cdf355

admonition

78209e3

small fix

7b6e0c7

mcmorisi approved these changes Dec 12, 2024

View reviewed changes

MM feedback

db64098

norareidy requested review from a team and papafe and removed request for a team December 12, 2024 22:02

papafe suggested changes Dec 17, 2024

View reviewed changes

papafe reviewed Dec 17, 2024

View reviewed changes

FP feedback

5764b75

norareidy requested review from BorisDog and papafe December 19, 2024 15:20

BorisDog reviewed Dec 21, 2024

View reviewed changes

BorisDog requested changes Dec 24, 2024

View reviewed changes

norareidy added 2 commits December 26, 2024 13:58

tech feedback

949557d

edits

a1666a0

norareidy requested a review from BorisDog December 26, 2024 19:49

BorisDog approved these changes Dec 26, 2024

View reviewed changes

final tech feedback

9a03ab7

norareidy merged commit e721d24 into mongodb:master Dec 27, 2024
6 checks passed

norareidy deleted the DOCSP-34008-split-event branch December 27, 2024 16:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOCSP-34008: Split large change events #374

DOCSP-34008: Split large change events #374

norareidy commented Dec 12, 2024 •

edited

Loading

netlify bot commented Dec 12, 2024 •

edited

Loading

mcmorisi left a comment

mcmorisi Dec 12, 2024

norareidy Dec 12, 2024

mcmorisi Dec 12, 2024

mcmorisi Dec 12, 2024

papafe Dec 17, 2024

papafe Dec 17, 2024

norareidy Dec 17, 2024

papafe Dec 17, 2024

papafe Dec 17, 2024

BorisDog Dec 21, 2024

BorisDog Dec 21, 2024

BorisDog Dec 21, 2024

BorisDog Dec 21, 2024

BorisDog Dec 24, 2024

BorisDog left a comment

BorisDog left a comment

BorisDog Dec 26, 2024

BorisDog Dec 26, 2024

DOCSP-34008: Split large change events #374

DOCSP-34008: Split large change events #374

Conversation

norareidy commented Dec 12, 2024 • edited Loading

Pull Request Info

Staging Links

Self-Review Checklist

netlify bot commented Dec 12, 2024 • edited Loading

✅ Deploy Preview for mongodb-docs-csharp ready!

mcmorisi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BorisDog left a comment

Choose a reason for hiding this comment

BorisDog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

norareidy commented Dec 12, 2024 •

edited

Loading

netlify bot commented Dec 12, 2024 •

edited

Loading