Skip to content

Conversation

kanongil
Copy link
Contributor

@kanongil kanongil commented Jul 1, 2022

This undoes a patch from #40947, and reworks it to continue handling backpressure when highWaterMark: 0, as described in #42457 (comment).

The new test fails on node releases with the patch from #40947, like v16.4, and passes on node v14 and node v16.3 from before it was introduced. It is also designed, so that it will fail if highWaterMark: 1 is set instead.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina requested a review from ronag July 1, 2022 23:03
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 5, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 5, 2022
@mcollina mcollina requested a review from ShogunPanda July 5, 2022 08:21
@ronag
Copy link
Member

ronag commented Jul 5, 2022

Looking at this now. I agree the hwm === 0 check is incorrect. Not sure about the fix though. Digging into it.

Copy link
Contributor

@ShogunPanda ShogunPanda left a comment

Choose a reason for hiding this comment

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

LGTM!

@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 5, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 5, 2022
@ronag
Copy link
Member

ronag commented Jul 6, 2022

@kanongil Could you rename this PR or create a new one for lazy reading?

@aduh95 aduh95 removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 9, 2022
@aduh95
Copy link
Contributor

aduh95 commented Jul 9, 2022

Removing author ready as it looks #43648 (comment) needs to be addressed. @ronag please re-add the label if I misinterpreted your comment.

@aduh95 aduh95 added the stalled Issues and PRs that are stalled. label May 11, 2024
Copy link
Contributor

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

Copy link
Contributor

Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions.

@github-actions github-actions bot closed this Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. stalled Issues and PRs that are stalled.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants