Proxy: compresssion support #1246
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
wprzytula
force-pushed
the
proxy-compresssion-support
branch
from
f8ebaac to
a745540
Compare
It was already such for scylla crate, so this is not a big change.
Those will be used by the proxy.
This will also be used by the proxy.
Again, proxy will take use of this.
Now they can be called in the static context, too.
It's convenient to be able to compare the whole frame at once.
It's enough for its job.
It was surely `pub` by mistake. `ResponseFrame::write` was `pub(crate)`.
Before, they printed the shard number next to the driver, not the node, which was confusing.
Some tests just set random CQL flags and assert that they are simply passed through the proxy. It used to work, but as now we make the proxy clever enough to interpret at least one of them, namely the compression flag, we need to stop abusing it blindly. The compression flag is the bit 0x1, so we simply must not use any odd numbers as flags.
After introducing changes connected to compression, the test `proxy_processes_requests_concurrently` started to be flaky on my machine. As its timeout was arbitrarily tight, I increased it. As a side note, the particular timeout is actually redundant. We could fully rely on the ntest::timeout.
For now, its only variant is FrameHeaderParseError, whose usage in proxy is replaced by ReadFrameError. In next commits, another variant will be added for (de)compression errors.
`CompressionReader` and `CompressionWriter` work in tandem to propagate
the compression that the driver negociated with the node (in STARTUP
frame) across the proxy connection workers.
- `CompressionWriter` will be used by the `request_processor` to set the
compression globally for the whole bunch of proxy workers for that
connection once a STARTUP message is intercepted.
- `CompressionReader` will be used by all workers that perform ser/de
(`{sender_to,receiver_from}_{driver,cluster}`) to (de)compress frames
as they come. Proxy will always store the frames and feed them into
feedback channels in the decompressed form, so that it is for example
possible to use `Condition`s based on uncompressed body contents.
This commit introduces the bare minimum of those abstractions and
passes them to all relevant workers. For now, no compression is ever
set.
`request_processor` worker now intercepts STARTUP messages and sets compression through its `CompressionWriter` according to the (optional) `COMPRESSION` key in a STARTUP frame. The set compression is still not used anywhere.
CompressionReader is now passed down to functions that read frames. In the next commits, it will be actually used to decompress frames if applicable.
This commit finally makes proxy decompress frames, if they have the compression flag set AND the proxy already intercepted a STARTUP frame.
CompressionReader is now passed down to functions that write frames. In the next commits, it will be actually used to compress frames if applicable.
This commit finally makes proxy compress frames, if they have the compression flag set AND the proxy already intercepted a STARTUP frame. This effectively makes the proxy re-compress frames that it decompressed when it intercepted them.
A unit test is written that tests compression mechanisms in the proxy. Outline of the test: 1. "driver" sends an, uncompressed, e.g., QUERY frame, feedback returns its uncompressed body, and "node" receives the uncompressed frame. 2. "node" responds with an uncompressed RESULT frame, feedback returns its uncompressed body, and "driver" receives the uncompressed frame. 3. "driver" sends an uncompressed STARTUP frame, feedback returns its uncompressed body, and "node" receives the uncompressed frame. 4. "driver" sends a compressed, e.g., QUERY frame, feedback returns its uncompressed body, and "node" receives the compressed frame. 5. "node" responds with a compressed RESULT frame, feedback returns its uncompressed body, and "driver" receives the compressed frame.
wprzytula
force-pushed
the
proxy-compresssion-support
branch
from
12443ad to
ee4f8e1
Compare
|
Is that useful for @fruch 's compression tests? |
In my tests I wasn't reading the content, just using the total sizes of frames I can help as extra validation that it's compressed, and which algorithm was used |
Yes: once proxy detects COMPRESSION key in STARTUP frame, it starts to interpret further frames having compression flag set as compressed, and decompresses them automatically using the negotiated compression. So it proxy fails to decompress a frame with an error, then we now that the frame was not compressed even though it should, or it was ill-compressed (perhaps with a wrong compression method). |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
During hackathon, there emerged a need to test compressed communication through proxy. However, proxy did not yet support compression.
What's done
scylla-cqlcompression utilitiesExposed some
scylla-cqlcompression utilitiesThese are now used by the proxy.
New impls
impl FromStr for Compressionimpl DeserializableRequest for StartupThe impls are used by the proxy to read the negotiated compression from STARTUP frame.
Minor proxy fixes / features
impl (Partial)Eq for {Request,Response}FrameIt's convenient to be able to compare the whole frame at once.
Fixed some debug prints
Before, they printed the shard number next to the driver, not the node, which was confusing.
Stopped abusing
COMPRESSEDframe flag in tests randomlySome tests just set random CQL flags and assert that they are simply passed through the proxy. It used to work, but as now we make the proxy clever enough to interpret at least one of them, namely the compression flag, we needed to stop abusing it blindly.
Combat timing-based flakiness
The test
proxy_processes_requests_concurrentlyshowed to be flaky on my machine. As its timeout there was arbitrarily tight, I increased it.Compression abstractions
CompressionReaderandCompressionWriterwork in tandem to propagate the compression that the driver negociated with the node (in STARTUP frame) across the proxy connection workers.CompressionWriteris used by therequest_processorto set the compression globally for the whole bunch of proxy workers for that connection once a STARTUP message is intercepted.CompressionReaderis used by all workers that perform ser/de ({sender_to,receiver_from}_{driver,cluster}) to (de)compress frames as they come. Proxy always stores the frames and feeds them into feedback channels in the decompressed form, so that it is for example possible to useConditions based on uncompressed body contents.Unit test for compression mechanisms in the proxy
Outline of the test:
Pre-review checklist
[ ] I have adjusted the documentation in./docs/source/.[ ] I added appropriateFixes:annotations to PR description.