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

feat(clickhouse-driver): allow to enable compression #9341

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Graphmaxer
Copy link

@Graphmaxer Graphmaxer commented Mar 13, 2025

Check List

  • Tests have been run in packages where changes made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Issue Reference this PR resolves

Description of Changes Made

Allow to enable compression via env variable for clickhouse driver

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
Graphmaxer Graphmaxer
@Graphmaxer Graphmaxer requested review from a team as code owners March 13, 2025 09:30
@github-actions github-actions bot added the pr:community Contribution from Cube.js community members. label Mar 13, 2025

Verified

This commit was signed with the committer’s verified signature.
Graphmaxer Graphmaxer
@Graphmaxer
Copy link
Author

Thanks for the comments, I also updated the clickhouseReadOnly option to the new env-var way :)

@igorlukanin
Copy link
Member

Before we merge this—

@Graphmaxer Did you confirm that enabling the compression actually improves the performance for large result sets?

@igorlukanin
Copy link
Member

Also, if this is a non-breaking change (except for the nuance with read-only connections), should we consider enabling the compression by default in the next minor version of Cube?

Verified

This commit was signed with the committer’s verified signature.
Graphmaxer Graphmaxer
@Graphmaxer
Copy link
Author

I did some testing documented in my issue #9340, I got positive impact in my use case. According to the Clickhouse documentation, this can have negative impacts depending on the queries.

The client does not enable request or response compression by default. However, when selecting or inserting large datasets, you could consider enabling it via ClickHouseClientConfigOptions.compression (either for just request or response, or both).
Compression has significant performance penalty. Enabling it for request or response will negatively impact the speed of selects or inserts, respectively, but will reduce the amount of network traffic transferred by the application.

https://clickhouse.com/docs/integrations/javascript#tips-for-performance-optimizations

@maximethebault
Copy link

Also facing the same issue when using PowerBI... Queries time out, but when replaying them in Clickhouse directly, they are almost instantaneous.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:community Contribution from Cube.js community members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants