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

[Bugfix] set proxy_pass_request_body off when redirect to FE #303

Merged

Conversation

yandongxiao
Copy link
Collaborator

@yandongxiao yandongxiao commented Nov 6, 2023

This is the original snippet in nginx.conf

    location @handle_redirect {
      set $redirect_uri '$upstream_http_location';
      proxy_pass $redirect_uri;
      proxy_set_header Expect $http_expect;
      proxy_pass_request_body on;
      proxy_set_header Host $host;
      proxy_set_header X-Real-IP $remote_addr;
      proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
      error_page 307 = @handle_redirect;
    }

When Nginx forwards requests to Leader FE and then BE, it follows the above Nginx configuration. Because proxy_pass_request_body is set to true, Nginx sends requests to Leader FE. When FE returns 307, Nginx forwards the request to BE. At this point, the data cached in Nginx is lost, causing the error.

2023/11/03 14:35:39 [error] 23#23: *23 sendfile() failed (32: Broken pipe) while sending request to upstream, client: 10.244.0.26, server: , request: "PUT /api/transaction/load HTTP/1.1", upstream: "http://10.96.43.63:8030/api/transaction/load", host: "10.244.0.41:8080"

@yandongxiao yandongxiao added the bugfix fix something that does not work label Nov 6, 2023
@@ -113,6 +114,26 @@ http {
}

location @handle_redirect {
if ($upstream_http_location ~ "%[3]s") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

how about

set $pass_body on;
if ( xxx ) {
    set $pass_body off;
}

proxy_pass_request_body $pass_body;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if can be used in rewrite context. See https://nginx.org/en/docs/http/ngx_http_rewrite_module.html

Copy link
Collaborator

Choose a reason for hiding this comment

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

just need to check if it is a FE redirect and set $pass_body accordingly, no need to do rewrite.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, I tried. if is not allowed to use without rewrite.

@kevincai kevincai merged commit bb130d6 into StarRocks:main Nov 7, 2023
@xlfjcg
Copy link

xlfjcg commented Dec 18, 2023

client should not send body before accept 100, The only exception should be response timeouts

@yandongxiao
Copy link
Collaborator Author

client should not send body before accept 100, The only exception should be response timeouts

I think what you mean is: nginx should not send request body before it accept 307 to BE ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix fix something that does not work v1.8.6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants