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

http.createServer highwatermark config #32731

Closed
0807Jpatel opened this issue Apr 9, 2020 · 9 comments
Closed

http.createServer highwatermark config #32731

0807Jpatel opened this issue Apr 9, 2020 · 9 comments
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem. stream Issues and PRs related to the stream subsystem.

Comments

@0807Jpatel
Copy link

Is your feature request related to a problem? Please describe.
http.get allows you to set highwatermark values as of node 13.x. Both IncomingMessage and OutgoingMessage supports setting hwm values in config, those settings are also applied to socket. why can't we do the same for http.createServer
#30135

Describe the solution you'd like
set highwatermark and enable objectMode for response (writable).

Describe alternatives you've considered
Please describe alternative solutions or features you have considered.

@bnoordhuis bnoordhuis added feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem. labels Apr 9, 2020
@bnoordhuis
Copy link
Member

set highwatermark and enable objectMode for response

What would that do? It's not at all clear to me what use that has. Please elaborate.

@0807Jpatel
Copy link
Author

0807Jpatel commented Apr 9, 2020

We are trying to generate backpressure over the network. we would like our highwatermark to be 5kb, but this is negated since response's highwatermark is always set to 16kb. we would like to be able to decrease it. so that it can generate back pressure faster.

var server = http.createServer(function (request, response) {
  myReadable.pipe(response); //response's highwater marks is set to 16kb by default.
});

@bnoordhuis
Copy link
Member

Do I understand correctly you want to be able to override writableHighWaterMark like this?

var server = http.createServer(function (request, response) {
  response.writableHighWaterMark = 5 * 1024;
  myReadable.pipe(response);
});

@0807Jpatel
Copy link
Author

0807Jpatel commented Apr 10, 2020

Yes that's exactly what we would like to do. But I don't think it's possible to overwrite writablehighwatermark after stream is created. So the simplest solution would be to implement at http.createServer level just like http.get

@bnoordhuis bnoordhuis added the stream Issues and PRs related to the stream subsystem. label Apr 10, 2020
@bnoordhuis
Copy link
Member

It's read-only right now but that could probably be changed if there's a good enough use case. I've updated the labels.

Having requests inherit it from the http.Server is both clunky and inflexible, IMO.

@0807Jpatel
Copy link
Author

0807Jpatel commented Apr 11, 2020

@bnoordhuis how do we go about this? As far as I can tell both objectMode and highwatermark values for our stream are useless as long as we can't edit those values for response stream.

@bnoordhuis
Copy link
Member

You can open a pull request with your suggested change. If you're not completely sure yet what direction to take, it's okay to open it in an incomplete state as a draft, to collect feedback.

request

You mentioned response in your original comment. Which one is it?

@0807Jpatel
Copy link
Author

0807Jpatel commented Apr 13, 2020

response. yea that was a typo, and I will try to get a pull request, soon.

@ronag
Copy link
Member

ronag commented May 15, 2020

There are other way to create back pressure. You can pipe it through a transform stream that applies it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants