Skip to content

stream: fix merge() idle source draining - #63567

Closed
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:stream-iter-merge-drain-fast-sources
Closed

trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:stream-iter-merge-drain-fast-sources

Conversation

@trivikr

@trivikr trivikr commented May 25, 2026

Copy link
Copy Markdown
Member

This updates stream/iter merge() so fast sources are not pulled to
completion while the merged consumer is idle.

Previously, after a source produced a batch, merge() immediately scheduled
another next() for that same source. If the merged iterator yielded once and
then the consumer paused, fast sources could continue draining in the
background.

The multi-source merge path now waits until the consumer requests the next
merged item before pulling again from the source that produced the previous
item.

Fixes: #63566


Assisted-by: openai:gpt-5.5

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to Node.js streams. labels May 25, 2026
@codecov

codecov Bot commented May 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.28571% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.33%. Comparing base (37779fd) to head (d4d7730).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/streams/iter/consumers.js 89.28% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63567      +/-   ##
==========================================
- Coverage   90.34%   90.33%   -0.02%     
==========================================
  Files         732      732              
  Lines      236689   236754      +65     
  Branches    44586    44610      +24     
==========================================
+ Hits       213839   213870      +31     
- Misses      14550    14602      +52     
+ Partials     8300     8282      -18     
Files with missing lines Coverage Δ
lib/internal/streams/iter/consumers.js 96.48% <89.28%> (-0.14%) ⬇️

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trivikr
trivikr force-pushed the stream-iter-merge-drain-fast-sources branch from 1d1b87f to 2894332 Compare May 27, 2026 01:14
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label May 28, 2026
@trivikr
trivikr force-pushed the stream-iter-merge-drain-fast-sources branch 2 times, most recently from 37e6bae to d4d7730 Compare June 2, 2026 03:44
Defer pulling a merged source again until the consumer requests the
next merged item. This prevents fast sources from being drained while
the merged iterator is idle.

Fixes: nodejs#63566

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@trivikr
trivikr force-pushed the stream-iter-merge-drain-fast-sources branch from d4d7730 to 4b053cb Compare June 7, 2026 05:44
@trivikr trivikr closed this Jun 18, 2026
@trivikr
trivikr deleted the stream-iter-merge-drain-fast-sources branch June 18, 2026 04:18
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. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. stream Issues and PRs related to Node.js streams.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stream/iter: merge() proactively drains fast sources while merged consumer is idle

2 participants