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

[client-v2] Compression #1717

Closed
chernser opened this issue Jul 1, 2024 · 2 comments · Fixed by #1761
Closed

[client-v2] Compression #1717

chernser opened this issue Jul 1, 2024 · 2 comments · Fixed by #1761

Comments

@chernser
Copy link
Contributor

chernser commented Jul 1, 2024

Compression

Compression may be on transport or application layer. For example, HTTP compressed encoding is transport layer and LZ4 payload compression is what application does.
Currently application compression is what we need to implement. First implementation should support bi-directional LZ4 encoding.

Note 1: There is a satisfying implementation of LZ4 support but it uses own stream classes. Should be adopted for broader use.

Settings

Next settings should be deprecated:

  • com.clickhouse.client.config.ClickHouseClientOption#COMPRESS "Whether the server will compress response it sends to client." - currently means server response compression and it should be named accordingly
  • com.clickhouse.client.config.ClickHouseClientOption#DECOMPRESS "Whether the server will decompress request from client." - currently means client request compression and it should be named accordingly
  • com.clickhouse.client.config.ClickHouseClientOption#COMPRESS_ALGORITHM "Algorithm used for server to compress response." - currently only LZ4 is used and this setting can be dropped.
  • com.clickhouse.client.config.ClickHouseClientOption#DECOMPRESS_ALGORITHM "Algorithm for server to decompress request." - currently only LZ4 is used and this setting can be dropped.
  • com.clickhouse.client.config.ClickHouseClientOption#COMPRESS_LEVEL "Compression level for response, -1 standards for default" - currently means server response compression level and it should be named accordingly. But may be dropped. com.clickhouse.client.config.ClickHouseClientOption#DECOMPRESS_LEVEL "Compression level for request, -1 standards for default" - currently means client request compression level and it should be named accordingly. But may be dropped.

Metrics

Not yet

@mshustov
Copy link
Member

mshustov commented Jul 1, 2024

com.clickhouse.client.config.ClickHouseClientOption#COMPRESS "Whether the server will compress response it sends to client." - currently means server response compression and it should be named accordingly
com.clickhouse.client.config.ClickHouseClientOption#DECOMPRESS "Whether the server will decompress request from client." - currently means client request compression and it should be named accordingly

It's mirroring of ClickHouse settings https://clickhouse.com/docs/en/interfaces/http

com.clickhouse.client.config.ClickHouseClientOption#COMPRESS_ALGORITHM "Algorithm used for server to compress response." - currently only LZ4 is used and this setting can be dropped.
com.clickhouse.client.config.ClickHouseClientOption#DECOMPRESS_ALGORITHM "Algorithm for server to decompress request." - currently only LZ4 is used and this setting can be dropped.

currently only LZ4, maybe we should ignore other value

@chernser
Copy link
Contributor Author

chernser commented Jul 1, 2024

@mshustov we have moved from this naming already because it is super confusing. But I need to mention it here to not forget when it will be the time for a migration guide.

we should not ignore values and in the new client it is not a problem because we will use enum for that, so no invalid values may be passed. However if there is no option today - may be drop these two settings?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants