overriden res.write() has altered flushing semantics

[usernameisalreadytaken2014]

When writing via the regular res.write(), under normal circumstances, data always reaches the browser.

This is also the case when the connection is never-ending, and data is pushed to write() as it becomes available. For example when data comes in small portions interspersed with pauses.

The Nagle algorithm makes sure that data reaches the browser in this case. The default setting on Linux is to wait 40 milliseconds, and if no further writes have happened, flush the data to the network.

When the compression module is included via app.use(), this no longer happens. There is a buffer in the zlib library which holds on to the data forever, and it never reaches the browser.

This feature request is to implement the following:

1. When a write() happens, set or reset a 40 millisecond timer.
2. If the timer runs out, call zlib to flush() the data to the browser.

That way, we retain the semantics of the original res.write().

Implementation ideas below.

I imagine that one way to implement this would be a "nagle" writable stream running on top of the gzip stream.

When the "nagle" stream's pipe() method is called, it looks if the passed stream is "instanceof Gzip".

If so, set an internal property to reference the underlying stream, so that we can later call flush() on it when a timer runs out.

[dougwilson]

Unfortunately we won't be making such a change. You're completely welcome to publish a version to npm that implements this, if that is your desire :) The way this module works is also the same as if you had nginx between your client and the server doing compression; it would buffer it up to compress it as well.

[aegyed91]

@dougwilson Hey there, sorry for posting a non relevant question here.

Is that a correct assumption if nginx gzipping the static files i don't need to use the compression module myself? Like i dont have to use serve-static either.

Also if you got 3 minutes to run over this gist i would really appreciate it. 👍

[usernameisalreadytaken2014]

@dougwilson

Unfortunately we won't be making such a change. You're completely welcome to publish a version

Hi Doug

Can you substantiate why you believe that this is not worth fixing?

(I know from personal experience that it sucks when someone comes along and points out that you're doing something incorrectly according to an overall design, user expectations or what not. That said, I'd much appreciate a logical discussion of the issue at hand.)

It seems to me that the current implementation breaks applications unexpectedly when the compression module is enabled because of this issue.

[dougwilson]

Hi @usernameisalreadytaken2014, the reason we're not going to change it is because you are perfectly capable of implementing this is a user-land module.

It seems to me that the current implementation breaks applications unexpectedly when the compression module is enabled because of this issue.

This is not true and can happen with anything buffering. Just because the buffering in memory does not change this fact. If you have a reverse proxy front your Node.js server doing the buffering, your application would have the same issue. If you are trying to stream out, you need to disable the compression, for example setting the Cache-Control: no-transform header.

But in the end, this was really a question, and asking for us to implement these changes, which we don't see the need. Perhaps your case will be more compelling if you can provide a pull request, implementing the changes in this module, complete with full documentation and tests with full coverage. That would make a much more compelling case to add this feature, and can also allow us to discuss the implications of the given implementation.

[usernameisalreadytaken2014]

This is not true

I beg to differ. The reason that I opened this issue is that my application broke the moment I enabled the compression module.

I expect anyone else using both SSE and compression to run into the same issue.

[dougwilson]

The reason that I opened this issue is that my application broke the moment I enabled the compression module.

Then I'd argue your application was written incorrectly. Regardless, please provide the aforementioned pull request. Also, would you be interested in simply taking over this module? This would be the best way to make progress on your issue and provide the compress expertise the community needs from you.

[dougwilson]

I expect anyone else using both SSE and compression to run into the same issue.

SSE will have this same issue on intermediate servers. Why are you not setting the Cache-Control: no-transform header?

[dougwilson]

Your entire argument of fixing SSE behavior with a magical 40ms write timeout is even worse than how it is today, not only because it will hurt the compression for all other applications, like slowing reading data from disk, but also will cause people to scratch their heads for why their event takes 40ms longer to reach the client with compression than without.

Regardless, if you are passionate about pursuing this change, please provide a pull request with the necessary implementation, complete with full documentation and tests with full coverage.

[usernameisalreadytaken2014]

Why are you not setting the Cache-Control: no-transform header?

Would that cause the compression module to behave any different?

If using this header gets rids of the stalls that adding the compression module causes, then this is a nice workaround.

(Will compression still work then? The first couple thousands of events are sent in bulk and are highly compressible, the latter arrive in drips and are reasonably compressible.)

[dougwilson]

Why are you not setting the Cache-Control: no-transform header?

Would that cause the compression module to behave any different?

Yes, as is documented on the README.

If using this header gets rids of the stalls that adding the compression module causes, then this is a nice workaround.

Yes, it absolutely would.

Will compression still work then?

No, this header is acknowledged by the module and no longer transforms the request (compressing is a transformation of the request, as defined by the HTTP RFCs). The biggest problem with not including that header is that any intermediate proxy between your server and the client can buffer the request in the same way this module is doing, and kill your SSE functionality. There are many servers out there people use like Amazon Silk's speed boost, Opera and Google have similar public services. Not including that header is rolling the dice with your users being able to receive the events in a timely fashion.

[usernameisalreadytaken2014]

it will hurt the compression for all other applications, like slowing reading data from disk

Yeah, that makes sense. The algorithm is basically Nagle and should only be used on network streams.

will cause people to scratch their heads for why their event takes 40ms longer to reach the client with compression than without.

The default Nagle timeout is 40ms, so the tail segment in a non-compressed stream (stream = no response.end() to flush) that doesn't fill the segment size fully should have the exact same behaviour. As far as I can tell, no difference?

[dougwilson]

will cause people to scratch their heads for why their event takes 40ms longer to reach the client with compression than without.

The default Nagle timeout is 40ms, so the tail segment in a non-compressed stream (stream = no response.end() to flush) that doesn't fill the segment size fully should have the exact same behaviour. As far as I can tell, no difference?

I don't think you understand. For example, the system reads a chunk of 4kb from disk, and gzip compresses it down to 600b. The next 4kb buffer does not get read off the disk until 50ms later, but since that was longer than 40ms, this module will have sent that tiny, incomplete gzip window to the client, when it could have compressed more of that second 4kb buffer into the same gzip window, resulting in very sub-optimal compression for slow file systems. In fact, this can likely cause the response size to go up due to the overhead of the gzip window framing and dictionaries if the system reads are consistently below this 40ms flush window.

While we were talking here, I wondered how can you do SSE responses in other languages like Java and use compression. They all require the user to indicate when you want to actually flush our your event, aka call res.flus() in this module, or the code generating the SSE writes is directly handling the compression and not leaving it up to a middleware-like system.

If you don't want to go and make that pull request, can you at least point me to some other compression/SSE systems like this in other languages and stuff that implement the solution you are describing?