stream: resume stream on drain #41848

ronag · 2022-02-04T09:38:27Z

Previously we would just resume "flowing" the stream without
reseting the "paused" state. Fixes this by properly using
pause/resume methods for .pipe.

Fixes: #41785

Previously we would just resume "flowing" the stream without reseting the "paused" state. Fixes this by properly using pause/resume methods for .pipe. Fixes: nodejs#41785

nodejs-github-bot · 2022-02-04T09:38:31Z

Review requested:

@nodejs/streams

nodejs-github-bot · 2022-02-04T09:54:15Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42344/

mcollina

lgtm

nodejs-github-bot · 2022-02-05T10:30:48Z

Commit Queue failed

- Loading data for nodejs/node/pull/41848
✔  Done loading data for nodejs/node/pull/41848
----------------------------------- PR info ------------------------------------
Title      stream: resume stream on drain (#41848)
Author     Robert Nagy  (@ronag)
Branch     ronag:pipe-resume -> nodejs:master
Labels     stream, author ready, lts-watch-v14.x, lts-watch-v16.x
Commits    2
 - stream: resume stream on drain
 - fixup
Committers 1
 - Robert Nagy 
PR-URL: https://github.com/nodejs/node/pull/41848
Fixes: https://github.com/nodejs/node/issues/41785
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41848
Fixes: https://github.com/nodejs/node/issues/41785
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 04 Feb 2022 09:38:27 GMT
   ✔  Approvals: 3
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/41848#pullrequestreview-872871213
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/41848#pullrequestreview-872917307
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/41848#pullrequestreview-873588555
   ✖  This PR needs to wait 23 more hours to land
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-02-04T09:54:15Z: https://ci.nodejs.org/job/node-test-pull-request/42344/
- Querying data for job/node-test-pull-request/42344/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/1799092796

nodejs-github-bot · 2022-02-05T10:31:11Z

Commit Queue failed

- Loading data for nodejs/node/pull/41848
✔  Done loading data for nodejs/node/pull/41848
----------------------------------- PR info ------------------------------------
Title      stream: resume stream on drain (#41848)
Author     Robert Nagy  (@ronag)
Branch     ronag:pipe-resume -> nodejs:master
Labels     stream, author ready, commit-queue-failed, lts-watch-v14.x, lts-watch-v16.x
Commits    2
 - stream: resume stream on drain
 - fixup
Committers 1
 - Robert Nagy 
PR-URL: https://github.com/nodejs/node/pull/41848
Fixes: https://github.com/nodejs/node/issues/41785
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41848
Fixes: https://github.com/nodejs/node/issues/41785
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 04 Feb 2022 09:38:27 GMT
   ✔  Approvals: 3
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/41848#pullrequestreview-872871213
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/41848#pullrequestreview-872917307
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/41848#pullrequestreview-873588555
   ✖  This PR needs to wait 23 more hours to land
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-02-05T10:30:48Z: https://ci.nodejs.org/job/node-test-pull-request/42344/
- Querying data for job/node-test-pull-request/42344/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/1799113734

nodejs-github-bot · 2022-02-05T10:31:29Z

Commit Queue failed

- Loading data for nodejs/node/pull/41848
✔  Done loading data for nodejs/node/pull/41848
----------------------------------- PR info ------------------------------------
Title      stream: resume stream on drain (#41848)
Author     Robert Nagy  (@ronag)
Branch     ronag:pipe-resume -> nodejs:master
Labels     stream, author ready, commit-queue-failed, lts-watch-v14.x, lts-watch-v16.x
Commits    2
 - stream: resume stream on drain
 - fixup
Committers 1
 - Robert Nagy 
PR-URL: https://github.com/nodejs/node/pull/41848
Fixes: https://github.com/nodejs/node/issues/41785
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41848
Fixes: https://github.com/nodejs/node/issues/41785
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 04 Feb 2022 09:38:27 GMT
   ✔  Approvals: 3
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/41848#pullrequestreview-872871213
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/41848#pullrequestreview-872917307
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/41848#pullrequestreview-873588555
   ✖  This PR needs to wait 23 more hours to land
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-02-05T10:31:11Z: https://ci.nodejs.org/job/node-test-pull-request/42344/
- Querying data for job/node-test-pull-request/42344/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/1799130209

nodejs-github-bot · 2022-02-06T09:28:47Z

Commit Queue failed

- Loading data for nodejs/node/pull/41848
✔  Done loading data for nodejs/node/pull/41848
----------------------------------- PR info ------------------------------------
Title      stream: resume stream on drain (#41848)
Author     Robert Nagy  (@ronag)
Branch     ronag:pipe-resume -> nodejs:master
Labels     stream, author ready, lts-watch-v14.x, lts-watch-v16.x
Commits    2
 - stream: resume stream on drain
 - fixup
Committers 1
 - Robert Nagy 
PR-URL: https://github.com/nodejs/node/pull/41848
Fixes: https://github.com/nodejs/node/issues/41785
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
Reviewed-By: James M Snell 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41848
Fixes: https://github.com/nodejs/node/issues/41785
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
Reviewed-By: James M Snell 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 04 Feb 2022 09:38:27 GMT
   ✔  Approvals: 4
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/41848#pullrequestreview-872871213
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/41848#pullrequestreview-872917307
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/41848#pullrequestreview-873588555
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/41848#pullrequestreview-873948143
   ✖  This PR needs to wait 9 more minutes to land
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-02-05T10:31:29Z: https://ci.nodejs.org/job/node-test-pull-request/42344/
- Querying data for job/node-test-pull-request/42344/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/1801919364

nodejs-github-bot · 2022-02-06T09:50:36Z

Commit Queue failed

- Loading data for nodejs/node/pull/41848 ✔ Done loading data for nodejs/node/pull/41848 ----------------------------------- PR info ------------------------------------ Title stream: resume stream on drain (#41848) Author Robert Nagy (@ronag) Branch ronag:pipe-resume -> nodejs:master Labels stream, author ready, lts-watch-v14.x, lts-watch-v16.x Commits 2 - stream: resume stream on drain - fixup Committers 1 - Robert Nagy PR-URL: https://github.com/nodejs/node/pull/41848 Fixes: https://github.com/nodejs/node/issues/41785 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca Reviewed-By: James M Snell ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/41848 Fixes: https://github.com/nodejs/node/issues/41785 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca Reviewed-By: James M Snell -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 04 Feb 2022 09:38:27 GMT ✔ Approvals: 4 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/41848#pullrequestreview-872871213 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/41848#pullrequestreview-872917307 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/41848#pullrequestreview-873588555 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/41848#pullrequestreview-873948143 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2022-02-06T09:28:47Z: https://ci.nodejs.org/job/node-test-pull-request/42344/ - Querying data for job/node-test-pull-request/42344/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/master up to date... From https://github.com/nodejs/node * branch master -> FETCH_HEAD ✔ origin/master is now up-to-date - Downloading patch for 41848 From https://github.com/nodejs/node * branch refs/pull/41848/merge -> FETCH_HEAD ✔ Fetched commits as 1c7a74e6f769..4916f0cde3fb -------------------------------------------------------------------------------- [master 7e509e5088] stream: resume stream on drain Author: Robert Nagy Date: Fri Feb 4 10:36:58 2022 +0100 2 files changed, 18 insertions(+), 2 deletions(-) [master f73beb6c3d] fixup Author: Robert Nagy Date: Fri Feb 4 10:47:25 2022 +0100 1 file changed, 1 insertion(+), 2 deletions(-) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. Rebasing (2/4)

Executing: git node land --amend --yes
⚠ Found Fixes: #41785, skipping..
--------------------------------- New Message ----------------------------------
stream: resume stream on drain

Previously we would just resume "flowing" the stream without
reseting the "paused" state. Fixes this by properly using
pause/resume methods for .pipe.

Fixes: #41785

PR-URL: #41848
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Luigi Pinca [email protected]
Reviewed-By: James M Snell [email protected]

[detached HEAD dfe7ed6c58] stream: resume stream on drain
Author: Robert Nagy [email protected]
Date: Fri Feb 4 10:36:58 2022 +0100
2 files changed, 18 insertions(+), 2 deletions(-)
Rebasing (3/4)
Rebasing (4/4)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
fixup

PR-URL: #41848
Fixes: #41785
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Luigi Pinca [email protected]
Reviewed-By: James M Snell [email protected]

[detached HEAD 16ebaf30d7] fixup
Author: Robert Nagy [email protected]
Date: Fri Feb 4 10:47:25 2022 +0100
1 file changed, 1 insertion(+), 2 deletions(-)

Successfully rebased and updated refs/heads/master.

ℹ Use --fixupAll option, squash the PR manually or land the PR from the command line.

https://github.com/nodejs/node/actions/runs/1801962481

nodejs-github-bot · 2022-02-06T10:05:53Z

Landed in 224b78f

Previously we would just resume "flowing" the stream without reseting the "paused" state. Fixes this by properly using pause/resume methods for .pipe. Fixes: #41785 PR-URL: #41848 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>

Previously we would just resume "flowing" the stream without reseting the "paused" state. Fixes this by properly using pause/resume methods for .pipe. Fixes: nodejs#41785 PR-URL: nodejs#41848 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>

Previously we would just resume "flowing" the stream without reseting the "paused" state. Fixes this by properly using pause/resume methods for .pipe. Fixes: #41785 PR-URL: #41848 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>

stream: resume stream on drain

a2c5fcc

Previously we would just resume "flowing" the stream without reseting the "paused" state. Fixes this by properly using pause/resume methods for .pipe. Fixes: nodejs#41785

ronag requested a review from mcollina February 4, 2022 09:38

ronag force-pushed the pipe-resume branch from 216bb20 to a2c5fcc Compare February 4, 2022 09:38

ronag requested a review from benjamingr February 4, 2022 09:38

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Feb 4, 2022

ronag added request-ci Add this label to start a Jenkins CI on a PR. stream Issues and PRs related to the stream subsystem. and removed needs-ci PRs that need a full CI run. labels Feb 4, 2022

benjamingr approved these changes Feb 4, 2022

View reviewed changes

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 4, 2022

This comment was marked as outdated.

Sign in to view

fixup

4916f0c

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 4, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 4, 2022

mcollina approved these changes Feb 4, 2022

View reviewed changes

mcollina added lts-watch-v14.x labels Feb 4, 2022

ronag added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 4, 2022

lpinca approved these changes Feb 4, 2022

View reviewed changes

mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 5, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 5, 2022

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Feb 5, 2022

mcollina removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Feb 5, 2022

jasnell approved these changes Feb 5, 2022

View reviewed changes

github-actions bot mentioned this pull request Feb 6, 2022

CI Reliability 2022-02-06 nodejs/reliability#194

Open

27 tasks

mcollina added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Feb 6, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 6, 2022

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Feb 6, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 6, 2022

nodejs-github-bot merged commit 224b78f into nodejs:master Feb 6, 2022

This was referenced Feb 7, 2022

CI Reliability 2022-02-07 nodejs/reliability#195

Open

CI Reliability 2022-02-08 nodejs/reliability#196

Open

ruyadorno mentioned this pull request Feb 8, 2022

v17.5.0 release proposal #41897

Merged

github-actions bot mentioned this pull request Feb 9, 2022

CI Reliability 2022-02-09 nodejs/reliability#197

Open

24 tasks

danielleadams mentioned this pull request Mar 3, 2022

v16.14.1 release proposal #42200

Merged

juanarbol mentioned this pull request Apr 28, 2022

V14.19.2 proposal #42899

Merged

richardlau added backported-to-v14.x and removed lts-watch-v14.x labels May 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stream: resume stream on drain #41848

stream: resume stream on drain #41848

ronag commented Feb 4, 2022

nodejs-github-bot commented Feb 4, 2022

This comment was marked as outdated.

nodejs-github-bot commented Feb 4, 2022

mcollina left a comment

nodejs-github-bot commented Feb 5, 2022

nodejs-github-bot commented Feb 5, 2022

nodejs-github-bot commented Feb 5, 2022

nodejs-github-bot commented Feb 6, 2022

nodejs-github-bot commented Feb 6, 2022

PR-URL: #41848
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Luigi Pinca [email protected]
Reviewed-By: James M Snell [email protected]

PR-URL: #41848
Fixes: #41785
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Luigi Pinca [email protected]
Reviewed-By: James M Snell [email protected]

Successfully rebased and updated refs/heads/master.

nodejs-github-bot commented Feb 6, 2022

stream: resume stream on drain #41848

stream: resume stream on drain #41848

Conversation

ronag commented Feb 4, 2022

nodejs-github-bot commented Feb 4, 2022

This comment was marked as outdated.

nodejs-github-bot commented Feb 4, 2022

mcollina left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Feb 5, 2022

nodejs-github-bot commented Feb 5, 2022

nodejs-github-bot commented Feb 5, 2022

nodejs-github-bot commented Feb 6, 2022

nodejs-github-bot commented Feb 6, 2022

PR-URL: #41848 Reviewed-By: Benjamin Gruenbaum [email protected] Reviewed-By: Matteo Collina [email protected] Reviewed-By: Luigi Pinca [email protected] Reviewed-By: James M Snell [email protected]

PR-URL: #41848 Fixes: #41785 Reviewed-By: Benjamin Gruenbaum [email protected] Reviewed-By: Matteo Collina [email protected] Reviewed-By: Luigi Pinca [email protected] Reviewed-By: James M Snell [email protected]

Successfully rebased and updated refs/heads/master.

nodejs-github-bot commented Feb 6, 2022

PR-URL: #41848
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Luigi Pinca [email protected]
Reviewed-By: James M Snell [email protected]

PR-URL: #41848
Fixes: #41785
Reviewed-By: Benjamin Gruenbaum [email protected]
Reviewed-By: Matteo Collina [email protected]
Reviewed-By: Luigi Pinca [email protected]
Reviewed-By: James M Snell [email protected]