-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(NODE-4139): streaming protocol message changes #3256
Conversation
When the monitor detects it is using the streaming protocol we now tell the message stream to only ever emit the last message in the buffer.
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.
just a couple of things on the tests; the implementation looks good to me
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.
Looking to those logs, I wonder if we have another problem (which we could choose to ignore): could this change result in unmatched heartbeat started/succeeded events? Or could that already happen even in the current implementation?
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
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.
just one suggestion to better isolate the condition under test
Description
Only emits the last message from the message stream's buffer when using the streaming protocol.
What is changing?
The monitor will now tell the connection that it's using the streaming protocol, which in turn will
set the flag on the connection's message stream. When the stream is in this mode, it will only ever
emit the message event with the very last message in the buffer pool. So if there's more data in the
buffer, the stream essentially skips its emit.
Is there new documentation needed for these changes?
None
What is the motivation for this change?
NODE-4139
Double check the following
npm run check:lint
script<type>(NODE-xxxx)<!>: <description>