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

Support for nginx X-Accel headers #90

Closed
abemusic opened this issue Mar 10, 2020 · 5 comments
Closed

Support for nginx X-Accel headers #90

abemusic opened this issue Mar 10, 2020 · 5 comments

Comments

@abemusic
Copy link

Is your feature request related to a problem? Please describe.

It would be extremely beneficial for those of us using nginx in front of ld-relay to have the relay set the X-Accel-Buffering header to inform nginx that the application server is wanting to stream responses.

Describe the solution you'd like

For ld-relay routes that do streaming, it should set the header X-Accel-Buffering: no

Describe alternatives you've considered

Currently, we are disabling proxy buffering entirely for ld-relay which is not the most appropriate solution. We are considering mapping each ld-relay endpoint that supports streaming in nginx and configure those routes for streaming. Because we are simply proxying from nginx to ld-relay, it would be very nice to have the application server inform us of the streaming intent.

Additional context

https://www.nginx.com/resources/wiki/start/topics/examples/x-accel/#x-accel-buffering

@eli-darkly
Copy link
Contributor

I don't suppose it's possible to configure nginx to recognize that it's a streaming response based on the content type? LaunchDarkly streaming data will always have the content type text/event-stream.

@eli-darkly
Copy link
Contributor

After some digging through the docs, it seems like the answer to my question is "no"... alas. So it makes sense for Relay to add a header as you suggested. We should be able to release a new version fairly soon.

LaunchDarklyCI pushed a commit that referenced this issue Mar 18, 2020
break out config structs into their own file and simplify how defaults work
@eli-darkly
Copy link
Contributor

The 5.11.0 release adds this header. Please let us know when you've had a chance to try it out.

@abemusic
Copy link
Author

This is great! Thank you for doing that. Apologies for not getting back to you sooner, things have been a bit hectic as you can probably imagine.

I'll make some time first thing tomorrow to give the new version a spin. Thanks again!!

@abemusic
Copy link
Author

Seems to be working great! Thank you all for the super fast turnaround! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants