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
Support compression for Keeper protocol #49507
Comments
@alesapin What't the performance of keeper comparing with apache zookeeper nowadays? |
Seems only one single thread processes the requests, so there is no chance of a big performance improvement, am I right? Here is the code: |
@helifu Receiving requests and sending responses is done in different threads. What's more each connection has its own thread for receiving requests. Decompressing would be done in a multithreaded way so we expect some improvements for bigger requests. |
Thanks @antonio2368 ~ |
[Zoo]Keeper Client and [Zoo]Keeper server communicate using custom TCP protocol:
ClickHouse/src/Common/ZooKeeper/ZooKeeperImpl.cpp
Lines 57 to 265 in 27f9384
ZooKeeper has quite strict (but configurable) limitations on request/response size but for Keeper we don't have them. For large multi-requests and big responses (for example
getChildren
response) we can have big advantages from compression, because both requests and responses should compress well.We already support compression in client-server TCP protocol: https://github.com/ClickHouse/clickhouse-private/blob/c496ff8dc14f1a5f115c257e96c9b1d16e628e32/src/Server/TCPHandler.cpp#L1745-L1748. And in client-server HTTP protocol: https://github.com/ClickHouse/clickhouse-private/blob/c496ff8dc14f1a5f115c257e96c9b1d16e628e32/src/Server/HTTPHandler.cpp#L575-L581.
The same technique can be applied -- wrap our TCP socket with compressed buffer on both sides:
ClickHouse/src/Server/KeeperTCPHandler.cpp
Lines 309 to 310 in 27f9384
ClickHouse/src/Common/ZooKeeper/ZooKeeperImpl.cpp
Lines 412 to 413 in 27f9384
To determine do we need to use compression or not, user can specify special tag in client's configuration: https://github.com/ClickHouse/ClickHouse/blob/master/tests/config/config.d/zookeeper.xml#LL1C1-L10C1, like
<compressed_protocol>true</compressed_protocol>
. I think the best way for Keeper Client to tell Server about compression is protocol version constantClickHouse/src/Common/ZooKeeper/ZooKeeperConstants.h
Line 47 in 27f9384
ZOOKEEPER_PROTOCOL_VERSION_WITH_COMPRESSION
(1) but received an exception it should just retry without compression.cc @antonio2368, @alexey-milovidov
The text was updated successfully, but these errors were encountered: