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

Merged
merged 3 commits into from
Mar 25, 2025

Conversation

Graphmaxer
Copy link
Contributor

@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

@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
@Graphmaxer
Copy link
Contributor 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?

@Graphmaxer
Copy link
Contributor 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.

@Graphmaxer
Copy link
Contributor Author

Up 😢

@ovr ovr self-assigned this Mar 25, 2025
@ovr ovr merged commit 78ac7b2 into cube-js:master Mar 25, 2025
46 checks passed
@ovr
Copy link
Member

ovr commented Mar 25, 2025

Thank you @Graphmaxer for your contribution 👍 🍰

Merged, it will be released in the next release.

@Graphmaxer Graphmaxer deleted the feature/clickhouse-compression-9340 branch March 26, 2025 08:07
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.

4 participants