Skip to content

[Improve][broker] Add gzip compression before http service response#16323

Closed
xuesongxs wants to merge 5 commits intoapache:masterfrom
xuesongxs:gzip
Closed

[Improve][broker] Add gzip compression before http service response#16323
xuesongxs wants to merge 5 commits intoapache:masterfrom
xuesongxs:gzip

Conversation

@xuesongxs
Copy link
Contributor

Fixes #16321

Describe the modifications you've done.
Add gzip compression before http service response.
Before compression:
image

After compression:
image

@AnonHxy
Copy link
Contributor

AnonHxy commented Jul 1, 2022

Is there any overhead about broker CPU cost? @xuesongxs

@xuesongxs
Copy link
Contributor Author

Is there any overhead about broker CPU cost? @xuesongxs

CPU utilization is not easy to test, which is also related to the compressed index memory size.

Copy link
Contributor

@liangyuanpeng liangyuanpeng left a comment

Choose a reason for hiding this comment

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

Add configuration would be great and disabled by default.

@xuesongxs
Copy link
Contributor Author

Add configuration would be great and disabled by default.

OK, I'll add a configuration attribute.

@codelipenghui
Copy link
Contributor

looks duplicated with #16197?

@xuesongxs
Copy link
Contributor Author

looks duplicated with #16197?

The purpose is the same, but its implementation method is not based on jetty's gzip handler, and it is not sure whether its repair can be recognized on the promethues client, and the implementation method is somewhat redundant. Please decide which solution to use.

I think that compression should be used in the HTTP service of broker, not only in metrics, but also in rest API.

@tjiuming
Copy link
Contributor

tjiuming commented Jul 4, 2022

@xuesongxs most of the endpoints won't has a big response body, the only known endpoint needs to compress is /metrics. if we enable compress all of the endpoints, it will comes high CPU usage.

@xuesongxs
Copy link
Contributor Author

@xuesongxs most of the endpoints won't has a big response body, the only known endpoint needs to compress is /metrics. if we enable compress all of the endpoints, it will comes high CPU usage.

OK,I'll modify it and only add it on /metrics.

Copy link
Contributor

@tjiuming tjiuming left a comment

Choose a reason for hiding this comment

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

Please add tests to ensure /metrics endpoint response compressed and others not

category = CATEGORY_SERVER,
doc = "enable compression when the HTTP service responds to the client"
)
private boolean enableCompress = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

enableCompressMetricsData should be better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please review

@xuesongxs
Copy link
Contributor Author

@codelipenghui @tjiuming @liangyuanpeng
Please help review the code.

This pull request was closed.
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.

Add gzip compression before http service response

5 participants