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

Added proxy_http_version 1.1 to nginx block conf to enable gzipping. #344

Closed
wants to merge 1 commit into from

Conversation

tiloio
Copy link

@tiloio tiloio commented Jan 10, 2019

Nginx gzip only runs if the http version is 1.1.
Nginx proxy default makes the http version to 1.0.

You can change the gzip_http_version 1.0 in your app, but I think it would be better to make it default to 1.1 with proxy_http_version 1.1.

@@ -58,6 +58,7 @@ if (!s.forceSsl || s.hasSsl) {
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
proxy_http_version 1.1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

As far as I understand, this is activating gzip for internal communication between two services on the same machine. It has nothing to do with gzipping between the client and the server. If my assumption is correct, this is not a good change, gzipping comes with CPU usage. The bandwidth that is being saved due to gzipping is the local bandwidth (within the server) which is quite cheap. So we are consuming CPU time for zipping and unzipping data on the same machine.

@tiloio
Copy link
Author

tiloio commented Jan 11, 2019

The captain ngnix acts like a proxy and a proxy does not process the message. You can test it out, I get a gzipped file at the client.

@githubsaturn
Copy link
Collaborator

@tiloio

I just re-tested it to be sure. And I am getting consistent result. Just to be clear, this is what I do:

  • Create a new app
  • Go to apps settings and edit the app nginx config to this:
<%
if (s.forceSsl) {
%>
    server {

        listen       80;

        server_name  <%-s.publicDomain%>;

        # Used by Lets Encrypt
        location /.well-known/ {
            root <%-s.staticWebRoot%>;
        }

        location / {
            return 302 https://$http_host$request_uri$is_args$query_string;
        }
    }
<%
}
%>


<%
if (!s.forceSsl || s.hasSsl) {
%>

    server {

    <%
    if (!s.forceSsl) {
    %>
        listen       80;
    <%
    }
    if (s.hasSsl) {
    %>
        listen              443 ssl;
        ssl_certificate     <%-s.crtPath%>;
        ssl_certificate_key <%-s.keyPath%>;
    <%
    }
    %>

        client_max_body_size 500m;

        server_name  <%-s.publicDomain%>;

        # 127.0.0.11 is DNS set up by Docker, see:
        # https://docs.docker.com/engine/userguide/networking/configure-dns/
        # https://github.com/moby/moby/issues/20026
        resolver 127.0.0.11 valid=10s;
        set $upstream http://<%-s.localDomain%>;

        location / {
            proxy_pass $upstream;
            proxy_set_header Host $host;
            proxy_set_header X-Real-IP $remote_addr;
            proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
            proxy_set_header X-Forwarded-Proto $scheme;
            proxy_http_version 1.1;
        }

        location /.well-known/ {
            root <%-s.staticWebRoot%>;
        }
    }

<%
}
%>

The only added line is proxy_http_version 1.1; inside the location. The response from client is this:

HTTP/1.1 200 OK
Server: nginx/1.15.7
Date: Fri, 11 Jan 2019 06:39:26 GMT
Content-Type: text/html; charset=utf-8
Content-Length: 297
Connection: keep-alive
X-Powered-By: Express
ETag: W/"129-+tEececw1ZecRqzNt0J8UuEP9Lc"

Now to turn of gzip, all I need to do is to edit the config file to this:

<%
if (s.forceSsl) {
%>
    server {

        listen       80;

        server_name  <%-s.publicDomain%>;

        # Used by Lets Encrypt
        location /.well-known/ {
            root <%-s.staticWebRoot%>;
        }

        location / {
            return 302 https://$http_host$request_uri$is_args$query_string;
        }
    }
<%
}
%>


<%
if (!s.forceSsl || s.hasSsl) {
%>

    server {

    <%
    if (!s.forceSsl) {
    %>
        listen       80;
    <%
    }
    if (s.hasSsl) {
    %>
        listen              443 ssl;
        ssl_certificate     <%-s.crtPath%>;
        ssl_certificate_key <%-s.keyPath%>;
    <%
    }
    %>

        client_max_body_size 500m;

        server_name  <%-s.publicDomain%>;

        # 127.0.0.11 is DNS set up by Docker, see:
        # https://docs.docker.com/engine/userguide/networking/configure-dns/
        # https://github.com/moby/moby/issues/20026
        resolver 127.0.0.11 valid=10s;
        set $upstream http://<%-s.localDomain%>;
        gzip  on;

        location / {
            proxy_pass $upstream;
            proxy_set_header Host $host;
            proxy_set_header X-Real-IP $remote_addr;
            proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
            proxy_set_header X-Forwarded-Proto $scheme;
        }

        location /.well-known/ {
            root <%-s.staticWebRoot%>;
        }
    }

<%
}
%>

Now the only changed line is gzip on;. In this case, I get a gzipped response. This is the header for the second configuration:

HTTP/1.1 200 OK
Server: nginx/1.15.7
Date: Fri, 11 Jan 2019 06:41:00 GMT
Content-Type: text/html; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive
X-Powered-By: Express
ETag: W/"127-bLJBOB//bnr7t6MElmMa31pR43o"
Content-Encoding: gzip

As you can see both responses are using HTTP 1.1, the presence of proxy_http_version 1.1; doesn't make any difference for the client. For more details, have a look at this:
https://ma.ttias.be/enable-keepalive-connections-in-nginx-upstream-proxy-configurations/

proxy_http_version is used for upstream connections not client to nginx connections.

I doubted my test results and repeated the test with Chrome, Curl and a third party website: https://checkgzipcompression.com/

All of them had the same result. I am very curious what technique did you use to test your configuration?

@tiloio
Copy link
Author

tiloio commented Jan 11, 2019

Oh, okay now I now what you mean.

I think I didn't showed that I want to use an ngnix as an app, so an ngnix in an ngnix. The outer ngnix should only load balance and SSL things.

The inner ngnix should gzip etc. (because it's time and CPU consuming).

Ngnix would only gzip on http1.1 and not on http1.0, so the outer ngnix should forward the http request with http1.1 and not with http1.0.

I think a lot of implementations now rely on http1.1 and not http1.0 so this change should not only improve an ngnix in an ngnix.

@githubsaturn
Copy link
Collaborator

githubsaturn commented Jan 11, 2019

To wrap up:

  • CaptainDuckDuck by default has only one layer of nginx, the nginx config that I edited is not an internal nginx, it's the same outer nginx, but the server block is specific to the app.
  • HTTP 1.1 is the new default for all connections that nginx handles with the client. You don't need to change anything to make nginx use HTTP 1.1 for connections to the outside world, i.e. connections that you care about.

Have a look at this: https://captainduckduck.com/img/captain-architecture.png

The dark blue and the green line that connect end user to your server are using http 1.1 without any special instructions.

I am closing this PR as it doesn't actually add gzip to the http block. You're more that welcome to open another PR and add a proper set of gzip instructions to this server block. Have a look at this post for example: https://www.keycdn.com/support/enable-gzip-compression

@tiloio
Copy link
Author

tiloio commented Jan 11, 2019

Yes, you are right.
The title of the pull request is wrong.

But shouldn't be the lightblue inner communication http1.1 too?

The nginx proxy conf says:

Syntax: proxy_http_version 1.0 | 1.1;
Default: proxy_http_version 1.0;

http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_http_version

So the default value is http 1.0 and not 1.1.

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

Successfully merging this pull request may close these issues.

None yet

2 participants