Skip to content

Conversation

@FGasper
Copy link
Collaborator

@FGasper FGasper commented Oct 31, 2025

This applies the new batch cursor from PR #148 to the change stream.

@FGasper FGasper changed the title Rep 6766 batch cursor change stream REP-6766 Use a batch cursor to parse the change stream Oct 31, 2025
FGasper added a commit that referenced this pull request Oct 31, 2025
This alters document comparison to use a batch cursor instead of the Go driver’s cursor. The advantage here is that, instead of creating giant slices of documents (which are copied from the driver), we’ll read the raw documents directly from the server response buffer. This minimizes GC pressure.

Pprof analysis shows that before this change, slices.Clone() allocated more heap than any other functions. With this change, those allocations are gone.

PR #153 will make the change stream use the same batch cursor.
@FGasper FGasper requested a review from tdq45gj October 31, 2025 15:08
@FGasper FGasper marked this pull request as ready for review October 31, 2025 15:08
@FGasper FGasper marked this pull request as draft October 31, 2025 15:09
@FGasper FGasper marked this pull request as ready for review October 31, 2025 16:15
Copy link
Collaborator

@tdq45gj tdq45gj left a comment

Choose a reason for hiding this comment

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

Left two small suggestions. LGTM.

err = csr.readAndHandleOneChangeEventBatch(ctx, ri, cs, sess)
err = csr.readAndHandleOneChangeEventBatch(ctx, ri, csCursor)

if err := csCursor.GetNext(ctx); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer a different variable name for the GetNext error, especially since we haven't finished with the original err.

SetMaxAwaitTime(maxChangeStreamAwaitTime)
) (*cursor.BatchCursor, bson.Timestamp, error) {

changeStreamStage := bson.D{
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could use the BSON builder.

@FGasper FGasper closed this Nov 4, 2025
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.

2 participants