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

GZIP Support #345

Open
bbassingthwaite opened this issue Feb 2, 2018 · 21 comments
Open

GZIP Support #345

bbassingthwaite opened this issue Feb 2, 2018 · 21 comments
Assignees

Comments

@bbassingthwaite
Copy link

I can't find any docs around enabling this or support for this? Am I missing a hidden feature or does endpoints just not do this? It would be a huge improvement if transcoded JSON responses were compressed. Thanks!

@qiwzhang
Copy link
Contributor

qiwzhang commented Feb 2, 2018

This is mainly for gRPC transcoding to gzip compress the JSON response. is that right?

Hi @PiotrSikora @lizan Does Nginx support GZIP compression in Response? If no, how easy to add it?

@PiotrSikora
Copy link
Contributor

@qiwzhang it does (gzip on;).

@lizan
Copy link
Contributor

lizan commented Feb 2, 2018

If you are using custom nginx.conf adding gzip on; should do compression.
http://nginx.org/en/docs/http/ngx_http_gzip_module.html

@bbassingthwaite
Copy link
Author

I've tried many variations of this and have not been able to get this to work.

One attempt:

gzip             on;
gzip_types       application/json;

I expect this to work, am I missing something?

@qiwzhang
Copy link
Contributor

qiwzhang commented Feb 2, 2018

We never try it. Hi @mangchiandjjoe Could you try it?

@mangchiandjjoe
Copy link
Contributor

mangchiandjjoe commented Feb 3, 2018

The default value of gzip_types is "text/html"
Nginx only compress for the response content type is text/html.
With this configuration, it works for me.

gzip            on;
gzip_types      text/plain application/xml application/json;

$ curl -v -H "Accept-Encoding: gzip" "http://127.0.0.1:8090/shelves"

*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8090 (#0)
> GET /shelves HTTP/1.1
> Host: 127.0.0.1:8090
> User-Agent: curl/7.55.0
> Accept: */*
> Accept-Encoding: gzip
> 
< HTTP/1.1 200 OK
< Server: nginx/1.13.4
< Date: Sat, 03 Feb 2018 00:59:35 GMT
< Content-Type: application/json; charset=utf-8
< Transfer-Encoding: chunked
< Connection: keep-alive
< X-Powered-By: Express
< ETag: W/"5b-j+/3rV/oFOnatLQ4z3XKYtlDXRc"
< Content-Encoding: gzip
< 
Warning: Binary output can mess up your terminal. Use "--output -" to tell 
Warning: curl to output it to your terminal anyway, or consider "--output 
Warning: <FILE>" to save to a file.
* Failed writing body (0 != 77)
* Failed writing data
* stopped the pause stream!
* Closing connection 0

@mangchiandjjoe
Copy link
Contributor

Could you please check your backend is returning "Content-Type" header?

@bbassingthwaite
Copy link
Author

The backed is grpc so I'm not setting any content to type.

@mangchiandjjoe
Copy link
Contributor

nginx requires content-type header to enable gzip compression.
the original response can be already compressed by the backend or content type doesnot requires compression(jpg, png,..)

https://github.com/nginx/nginx/blob/master/src/http/modules/ngx_http_gzip_filter_module.c#L257

    if (!conf->enable
        || (r->headers_out.status != NGX_HTTP_OK
            && r->headers_out.status != NGX_HTTP_FORBIDDEN
            && r->headers_out.status != NGX_HTTP_NOT_FOUND)
        || (r->headers_out.content_encoding
            && r->headers_out.content_encoding->value.len)
        || (r->headers_out.content_length_n != -1
            && r->headers_out.content_length_n < conf->min_length)
        || ngx_http_test_content_type(r, &conf->types) == NULL // disabled by this line 
        || r->header_only)
    {
        return ngx_http_next_header_filter(r);
    }

@bbassingthwaite
Copy link
Author

What does this mean for gRPC? From my end, i've added the gzip headers but the gRPC transcoding to JSON is never gzipped by nginx.

@lizan
Copy link
Contributor

lizan commented Feb 5, 2018

The content type for transcoding is set to application/json. I don’t think we set content-length but use chunked encoding, I think setting length for transcoding might work.

It will be only for unary calls, does that solves your problem?

@bbassingthwaite
Copy link
Author

That does solve my problem.

@ghost
Copy link

ghost commented Feb 17, 2018

This would greatly help me as well, any thoughts on when this might be available?

@lizan Is there any chance gzip could work for streaming calls as well or is there a technical reason that can't be done?

@ashi009
Copy link

ashi009 commented Oct 25, 2018

@lizan Is it possible to add gzip support in start_esp now? I just learned that GCP LB won't handle gzip compression. So ESP as the application frontend should do this.

@qiwzhang
Copy link
Contributor

At current state, just adding "gzip on" only works for HTTP backend.

@ashi009
Copy link

ashi009 commented Oct 26, 2018

A small gRPC response gets bloated for over 20x in size due to JSON and the lacking of compression.

Could you evaluate the efforts needed for adding such support? We can help that out if this won't stand in the way of the esp dev roadmap.

@craigmulligan
Copy link

craigmulligan commented Mar 20, 2019

Would you be open to PR adding a startup gzip option that adds gzip to the nginx config? I'd prefer to use the esp options rather than manage my own nginx config.

@qiwzhang
Copy link
Contributor

Yes, we will appreciate your contribution. Please go ahead.

@craigmulligan
Copy link

Cool, as a quick fix I just made the origin server add to do the gzipping. Will circle back to this when we have time. If anyone else wants to pick it up in the meantime 👍

@qiwzhang qiwzhang self-assigned this Apr 26, 2019
@burk
Copy link

burk commented Aug 23, 2019

The content type for transcoding is set to application/json. I don’t think we set content-length but use chunked encoding, I think setting length for transcoding might work.

It will be only for unary calls, does that solves your problem?

@lizan What does "setting length for transcoding" mean specifically? Is that a nginx option?

@qiwzhang
Copy link
Contributor

No, it is in the ESP proto to json response transcoding code. It currently doesn't set it. It is not easy, data flow wise, it writes response headers first, then do body translation, so at the time of writing headers, content-length is not known.

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

8 participants