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

[bfetch] set 'X-Accel-Buffering':'no' to streaming response headers #139534

Merged
merged 8 commits into from
Sep 7, 2022

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Aug 26, 2022

Summary

bfetch streams a list of results back to the end user's browser as soon as possible using HTTP streaming, i.e., it uses Transfer-Encoding: chunked. But it is common for reverse proxies, say Nginx, to (instead of passing through the stream) buffer the stream up to 4 or 8 KiB, and if the request completes within those 4/8 KiB, it instead sets the Content-Length: ... header and sends the whole payload in one go. This is also what is happening in ESE.

This pr addes X-Accel-Buffering: no to streaming response that tells NGINX not to buffer the response. The plan is to make ESE proxy also respect that header and skip buffering the response.

Needed for #139746 (but will actually take effect in ese when cloud's fix lands)
Also would fix streaming responses of all bsearch requests and canvas

Release notes

bfetch response headers now include X-Accel-Buffering: no

@Dosant Dosant added the ci:cloud-deploy Create or update a Cloud deployment label Aug 26, 2022
@Dosant Dosant closed this Aug 26, 2022
@Dosant
Copy link
Contributor Author

Dosant commented Aug 26, 2022

#139532

@vadimkibana vadimkibana reopened this Aug 26, 2022
@vadimkibana vadimkibana marked this pull request as ready for review August 26, 2022 13:34
@vadimkibana vadimkibana requested a review from a team as a code owner August 26, 2022 13:34
Copy link
Contributor

@vadimkibana vadimkibana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should merge this anyways, even if it does not help immediately, it is the right header to use here.

@botelastic botelastic bot added the Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) label Aug 26, 2022
@vadimkibana vadimkibana added v8.4.2 release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated: Automatically backport this PR after it's merged v8.5.0 labels Aug 26, 2022
@Dosant
Copy link
Contributor Author

Dosant commented Aug 29, 2022

@elasticmachine merge upstream

@Dosant Dosant marked this pull request as draft August 30, 2022 11:17
@Dosant Dosant changed the title try 'X-Accel-Buffering': 'no' try change bfetch headers Aug 30, 2022
@dominiqueclarke
Copy link
Contributor

@elasticmachine merge upstream

@Dosant Dosant changed the title try change bfetch headers [bfetch] set 'X-Accel-Buffering':no to streaming response headers Sep 5, 2022
@Dosant Dosant changed the title [bfetch] set 'X-Accel-Buffering':no to streaming response headers [bfetch] set 'X-Accel-Buffering':'no' to streaming response headers Sep 5, 2022
@Dosant Dosant added Feature:Search Querying infrastructure in Kibana release_note:fix and removed release_note:skip Skip the PR/issue when compiling release notes labels Sep 5, 2022
@Dosant
Copy link
Contributor Author

Dosant commented Sep 7, 2022

@elasticmachine merge upstream

@Dosant Dosant marked this pull request as ready for review September 7, 2022 09:39
@kibana-ci
Copy link
Collaborator

kibana-ci commented Sep 7, 2022

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.4

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

kibanamachine added a commit that referenced this pull request Sep 7, 2022
…#139534) (#140182)

(cherry picked from commit ea8e034)

Co-authored-by: Anton Dosov <anton.dosov@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated: Automatically backport this PR after it's merged ci:cloud-deploy Create or update a Cloud deployment Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) Feature:Search Querying infrastructure in Kibana release_note:fix v8.4.2 v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants