Skip to content
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

[legacy-framework] Maybe fix the stuck at _manifest issue: Increase stream highwatermark #697

Merged
merged 2 commits into from
Jun 17, 2020

Conversation

ryardley
Copy link
Collaborator

@ryardley ryardley commented Jun 17, 2020

This may fix this issue: blitz-js/legacy-framework#931

What are the changes and their implications?

I added a test looking for errors where streams corked themselves because of throughput and I found that happened within pumpify. Generally for the stream to be memory efficient we should have it manage to handle a low highwatermark scenario and pump is designed to handle this apparently although it appears as though this is not the case with pumpify for whatever reason so I have increased the highwatermark to a huge number.

We shouldn't necessarily be doing this as it might mean we use far more memory than we should or get into situations where we run out of memory but I have a good feeling it might solve this issue for now and we should investigate whether or not it is possible to lower the value to use a limited amount of memory instead of a huge amount. (Which should be possible if every stream handling thing we use including external libs are behaving themselves)

Checklist

  • Tests added for changes
  • PR submitted to blitzjs.com for any user facing changes

@flybayer flybayer changed the title Increase highwatermark Maybe fix the stuck at _manifest issue: Increase stream highwatermark Jun 17, 2020
@flybayer flybayer merged commit 15c7b76 into canary Jun 17, 2020
@flybayer flybayer deleted the 632-fix-manifest-highwatermark branch June 17, 2020 04:49
@dillondotzip dillondotzip changed the title Maybe fix the stuck at _manifest issue: Increase stream highwatermark [legacy-framework] Maybe fix the stuck at _manifest issue: Increase stream highwatermark Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants