fabiolb / fabio

Consul Load-Balancing made simple
https://fabiolb.net
MIT License
7.27k stars 616 forks source link

Unbuffered chunked streaming #324

Open bharendt opened 7 years ago

bharendt commented 7 years ago

Unbuffered chunked streaming

Fabio uses the go ReverseProxy which buffers the outgoing response to the client.

It uses a ResponseWriter with a buffered writer which buffers all outgoing data with a size of 4kB.

As workaround an Accept: text/event-stream request header can be set to enable periodically flushing of that buffer. This needs to be set already in the incoming request, because the flush interval must be set already in the ReverseProxy constructor.

Setting the accept header is not suitable for us, because we need to get the chunked resource directly from the browser. It streams live data.

In addition buffering up to 4kB before forwarding the response increases the latency for each request.

That's why we modified the default ReverseProxy implementation and added a Flush() after the write to the response writer which solves both problems for us.

If you think it makes sense to adopt this change we are happy to open a pull request.

magiconair commented 7 years ago

That sounds interesting and there was https://github.com/fabiolb/fabio/issues/129#issuecomment-315313167 which seems related. It was always kind of clear that forking the reverse proxy would eventually happen but I'm glad that for the most part I didn't have to.

So my suggestion would be this:

tommyvicananza commented 5 years ago

Hello, here's what we did. Thanks @bharendt approach but we wanted to try to avoid unnecessary flushes so we decided to add a Target option which adds flush to desired route.

E.g: route add nomad nomad.tommyvicananza.io http://192.168.50.4:4646 opts "flush=-1s"

aaronhurt commented 5 years ago

@tommyvicananza I believe this is related https://github.com/fabiolb/fabio/pull/655 and may solve your issue. It does not implement a configurable flush but does re-add the Flush method that was lost in a previous change.

oterrien commented 3 years ago

Hi

We have the same issue (fabio:1.5.15-go1.15.5).

Our use-case is to display, by streaming (SSE), all statuses of a given object. Because the size of the buffer is never reached, the content is never displayed.

Updating proxy.flushinterval in fabio.properties has no impact.

Is it possible to configure the size of buffer to be taken into account while streaming?

sbrl commented 3 years ago

This is a serious problem when putting Fabio in front of Hashicorp Nomad: https://github.com/hashicorp/nomad/issues/5404

If I access my nomad instance directly, then viewing the logs of any running job works fine. But if I try to view them when connecting via Fabio, it doesn't work. Apparently flush=-1s is supposed to help here, but it doesn't have any effect at all.

What's the status on this issue please?

james-masson commented 3 years ago

I hit the same problem.

@sbrl - flush=-1s is a product of the patch @tommyvicananza made in his custom build - it's not in mainline Fabio as far as I can see.

per-route config for this would be ideal, would you be willing to submit this as a PR @tommyvicananza ?

The workaround for now is to set proxy.globalflushinterval="-1s"

In the environment I'm working on, globally disabling buffering should be fine, but for many this will be very inefficient.

Nomad does not trigger the SSE handling, as it uses content-type "application/json"

sbrl commented 3 years ago

Thanks, @james-masson! I've implemented that as a temporary fix. It seems to make it to work in some scenarios where it didn't before, but not others (e.g. when there aren't many logs) - so there's still work that needs doing here.

Nomad does not trigger the SSE handling, as it uses content-type "application/json"

Does that mean that even implementing this fix won't completely solve the problem? If so, that explains the behaviour I'm seeing?

Himura2la commented 3 years ago

I tried to set FABIO_PROXY_GLOBALFLUSHINTERVAL = "-1s", and it doesn't help with Nomad logs issue...