Skip to content
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

fix(NODE-6394): pause message stream when not listening for events #4249

Merged
merged 2 commits into from
Sep 27, 2024

Conversation

baileympearson
Copy link
Contributor

@baileympearson baileympearson commented Sep 25, 2024

Description

What is changing?

The message stream is paused when we're not reading from it, so we do not miss data events.

Is there new documentation needed for these changes?

What is the motivation for this change?

Release Highlight

Fixed indefinite hang bug for high write load scenarios

When performing large and many write operations, the driver will likely encounter buffering at the socket layer. The logic that waited until buffered writes were complete would mistakenly drop 'data' (reading from the socket), causing the driver to hang indefinitely or until a socket timeout. Using pausing and resuming mechanisms exposed by Node streams we have eliminated the possibility for data events to go unhandled.

Shout out to @hunkydoryrepair for debugging and finding this issue!

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@baileympearson baileympearson marked this pull request as draft September 25, 2024 17:00
@baileympearson baileympearson changed the title fix(no-story-drain-repro): add tests demonstrating drain + data behavior fix(NODE-6394): pause message stream when not listening for events Sep 26, 2024
@baileympearson baileympearson marked this pull request as ready for review September 26, 2024 13:28
@nbbeeken nbbeeken self-assigned this Sep 26, 2024
@nbbeeken nbbeeken added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Sep 26, 2024
src/cmap/connection.ts Outdated Show resolved Hide resolved
src/cmap/connection.ts Outdated Show resolved Hide resolved
src/cmap/connection.ts Show resolved Hide resolved
test/unit/cmap/connection.test.ts Show resolved Hide resolved
test/unit/cmap/connection.test.ts Show resolved Hide resolved
@nbbeeken
Copy link
Contributor

Failures unrelated

@nbbeeken nbbeeken merged commit 3f9d243 into main Sep 27, 2024
21 of 30 checks passed
@nbbeeken nbbeeken deleted the no-story-drain-repro branch September 27, 2024 19:35
@hunkydoryrepair
Copy link

any chance of a patch release until 6.10 is out? Maybe a 6.9.1? Especially since 6.10 removes mongo 3.6 support, some won't have option to upgrade.

@alexbevi
Copy link
Contributor

Based on the MongoDB Software Lifecycle Schedules MongoDB 3.6 reached EOL in April 2021.

It's unlikely there'd be enough demand to support this server version to act as a forcing function for a patch release.

@hunkydoryrepair
Copy link

Fair enough. we are stuck on 6.3 since this has been broken for a while and would love to be able to use the optimizations done since then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants