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

Add compression to request body #207

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hkrutzer
Copy link

We want to send potentially large payloads to store in Clickhouse. It would be nice if we can compress those. What do you think? If you are willing to add this feature I will make it optional. The implementation is from https://github.com/wojtekmach/req/blob/v0.5.6/lib/req/steps.ex#L650-L655

@ruslandoga
Copy link
Contributor

ruslandoga commented Oct 17, 2024

👋 @hkrutzer

Thank you for the PR! I'd like the client to be agnostic of the compression but I definitely want sending compressed requests to be possible and easy. I'll post more tomorrow but for now I'll link a PR where I added support for custom headers for a similar use case: #129

Some things to consider:

  • in HTTP, because we send SQL in the request body, it needs to be either compressed as well (gzip), or be wrapped in an uncompressed frame (zstd), or whatever lz4 is doing :)
  • we need to allow lz4 and zstd codecs since they probably are more efficient than gzip

@hkrutzer
Copy link
Author

hkrutzer commented Oct 17, 2024

That sounds good, what do you think of adding a behaviour for compression? Ch could ship with the implementation for gzip, and allow for other implementations via the behaviour. I think gzip is the only one that can be done

in HTTP, because we send SQL in the request body, it needs to be either compressed as well (gzip), or be wrapped in an uncompressed frame (zstd), or whatever lz4 is doing :)

I am not sure what you mean, isn't the entire body always compressed using the chosen algorithm?

@ruslandoga
Copy link
Contributor

ruslandoga commented Oct 18, 2024

what do you think of adding a behaviour for compression?

I think it can be simpler than that. Here's what I have in mind:

insert = ["INSERT INTO events(time, value) FORMAT RowBinary\n" | rowbinary]

Ch.query(pool, :zlib.compress(insert), _params = [], headers: [{"content-encoding", "gzip"}])
# https://github.com/whatyouhide/nimble_lz4
Ch.query(pool, NimbleLZ4.compress(insert), _params = [], headers: [{"content-encoding", "lz4"}])

DBConnection.run(pool, fn conn ->
  Stream.resource(datastream,  ... do :zlib things ...)
  # here the SQL query would go into the URL's query string, so it doesn't have to be compressed
  |> Stream.into(Ch.stream(conn, "INSERT INTO events(time, value) FORMAT RowBinary", _params = [], headers: [{"content-encoding", "gzip"}]))
  |> Stream.run()
end)

DBConnection.run(pool, fn conn ->
  Stream.map(datastream, &NimbleLZ4.compress_frame/1) # not sure if ClickHouse HTTP supports official LZ4 frames, the native format uses custom framing
  |> Stream.into(Ch.stream(conn, "INSERT INTO events(time, value) FORMAT RowBinary", _params = [], headers: [{"content-encoding", "lz4"}]))
  |> Stream.run()
end)

I am not sure what you mean, isn't the entire body always compressed using the chosen algorithm?

In the current implementation of Ch.query/4 the SQL query is a bit special and it's hard to compress it together with the body. I want to change (WIP: #156) that so that users can send anything really, basically making Ch a "thin layer" above Mint the same way it's a "thin layer" above :gen_tcp / :ssl.

@hkrutzer
Copy link
Author

# not sure if ClickHouse HTTP supports official LZ4 frames, the native format uses custom framing

I tested and it looks like it does not. I think it only supports what content-encoding: lz4 supports, which appears to not include frames.

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.

2 participants