apache
[improve][broker] Add gzip compression before http service response
@xuesongxs Please provide a correct documentation label for your PR. Instructions see Pulsar Documentation Label Guide.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@xuesongxs Please provide a correct documentation label for your PR. Instructions see Pulsar Documentation Label Guide.
Having gzip support for all http endpoints could be useful too. I wonder if it could be simply enabled for all by default. @codelipenghui wdyt?
Having gzip support for all http endpoints could be useful too. I wonder if it could be simply enabled for all by default. @codelipenghui wdyt?
I remember this PR and penghui said that not all the endpoints has a big response body, make all the endpoints output compressed response may lead to CPU usage increase
for the tests, I remember the HTTP response code is always 302, so they didn't passed, does it fixed? and it's better to make sure the response body could parse to Prometheus Metrics successfully, current test only requires http code = 200.
@lhotari Done. I suggested several changes.
@asafm great work on the review! Thanks
Having gzip support for all http endpoints could be useful too. I wonder if it could be simply enabled for all by default. @codelipenghui wdyt?
I remember this PR and penghui said that not all the endpoints has a big response body, make all the endpoints output compressed response may lead to CPU usage increase
Can we enable conditional compression that only compresses payloads above a certain size?
Can we enable conditional compression that only compresses payloads above a certain size?
Yes, that is supported in GzipHandler.
https://www.eclipse.org/jetty/javadoc/jetty-9/org/eclipse/jetty/server/handler/gzip/GzipHandler.html#setMinGzipSize(int)
Can we enable conditional compression that only compresses payloads above a certain size?
Yes, that is supported in GzipHandler.
https://www.eclipse.org/jetty/javadoc/jetty-9/org/eclipse/jetty/server/handler/gzip/GzipHandler.html#setMinGzipSize(int)
That seems like a good indication to me that we should just enable it for all HTTP endpoints.
Can we enable conditional compression that only compresses payloads above a certain size?
Yes, that is supported in GzipHandler. https://www.eclipse.org/jetty/javadoc/jetty-9/org/eclipse/jetty/server/handler/gzip/GzipHandler.html#setMinGzipSize(int)
That seems like a good indication to me that we should just enable it for all HTTP endpoints.
One interesting question here is if we experience any issues currently with other HTTP endpoints. In Prometheus, the shear response size of 300 MB is worth compressing since the issue with a timeout. I wonder which other endpoints have similar issues?
@xuesongxs - it looks like this PR stalled. It'd be great to move this along and get it merged. Do you plan on continuing work for this PR? Is the issue CI? Thanks!
@xuesongxs - it looks like this PR stalled. It'd be great to move this along and get it merged. Do you plan on continuing work for this PR? Is the issue CI? Thanks!
I will continue to do this PR this week.
@xuesongxs - it looks like something went wrong with your most recent commit. The diff is now very large.
I created a new PR to replace it. https://github.com/apache/pulsar/pull/18102
From: Michael Marshall Date: 2022-10-19 09:42 To: apache/pulsar CC: xuesongxs; Mention Subject: Re: [apache/pulsar] [improve][broker] Add gzip compression support for /metrics endpoint (PR #16888) @xuesongxs - it looks like something went wrong with your most recent commit. The diff is now very large. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>