Skip to content

stream: fix drop-newest behavior in share()#64417

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:stream-iter-share-drop-newest
Open

stream: fix drop-newest behavior in share()#64417
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:stream-iter-share-drop-newest

Conversation

@trivikr

@trivikr trivikr commented Jul 11, 2026

Copy link
Copy Markdown
Member

Fixes: #64416

Fixes share() with backpressure: 'drop-newest' so upstream pull
results are discarded when the shared buffer reaches highWaterMark.

Previously, #waitForBufferSpace() returned true for drop-newest,
causing the pulled batch to be appended to the full buffer. The buffer
could therefore grow beyond highWaterMark, and stalled consumers
eventually received every batch.

The async implementation now pulls and discards new results while the
buffer is full. The existing test has been updated to verify that the
buffer remains bounded and dropped batches are not delivered.

@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 the stream subsystem. labels Jul 11, 2026
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 11, 2026
@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.24%. Comparing base (608112a) to head (5fadff0).

Files with missing lines Patch % Lines
lib/internal/streams/iter/share.js 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64417      +/-   ##
==========================================
+ Coverage   90.21%   90.24%   +0.03%     
==========================================
  Files         739      739              
  Lines      241650   241650              
  Branches    45542    45551       +9     
==========================================
+ Hits       218013   218086      +73     
+ Misses      15155    15100      -55     
+ Partials     8482     8464      -18     
Files with missing lines Coverage Δ
lib/internal/streams/iter/share.js 84.99% <71.42%> (+0.26%) ⬆️

... and 33 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.

Discard upstream pull results when the shared buffer has reached its
highWaterMark instead of allowing the buffer to grow beyond its limit.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
@trivikr trivikr force-pushed the stream-iter-share-drop-newest branch from baae906 to 5fadff0 Compare July 16, 2026 03:48
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. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stream/iter: share() does not discard upstream results with "drop-newest"

2 participants