-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
Use CH Buffer for HTTP out stream, add metrics for interfaces #56064
Conversation
This is an automated comment for commit db97764 with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page Successful checks
|
… Content-Encoding if no data
Some perf tests are failed. Interesting what is the reason. |
I guess because we had some problems with CI yesterday |
…e-server-iface-metrics
throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Unsupported compression method"); | ||
|
||
std::unique_ptr<WriteBuffer> wrapWriteBufferWithCompressionMethod( | ||
WriteBuffer * nested, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks dangerous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is to support not owning version of WriteBufferDecorator
where nested
is decorated but not owned by decorator
LZMADeflatingWriteBuffer( | ||
std::unique_ptr<WriteBuffer> out_, | ||
WriteBufferT && out_, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LZMADeflatingWriteBuffer
is inherited from WriteBufferWithOwnMemoryDecorator
which is a specialization of WriteBufferDecorator
and should now accommodate both owning and non-owning construction
@@ -5,5 +5,5 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) | |||
. "$CURDIR"/../shell_config.sh | |||
|
|||
echo 'SELECT 1 FROM numbers(100)' | | |||
${CLICKHOUSE_CURL_COMMAND} -v "${CLICKHOUSE_URL}&wait_end_of_query=1&send_progress_in_http_headers=0" --data-binary @- 2>&1 | | |||
${CLICKHOUSE_CURL_COMMAND} -vs "${CLICKHOUSE_URL}&wait_end_of_query=1&send_progress_in_http_headers=0" --data-binary @- 2>&1 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you pass -s
(--silent) option to curl, you should always also specify -S
(not ignore errors).
@@ -7,7 +7,7 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) | |||
|
|||
|
|||
CURL_OUTPUT=$(echo 'SELECT 1 + sleepEachRow(0.00002) FROM numbers(100000)' | \ | |||
${CLICKHOUSE_CURL_COMMAND} -v "${CLICKHOUSE_URL}&wait_end_of_query=1&send_progress_in_http_headers=0&max_execution_time=1" --data-binary @- 2>&1) | |||
${CLICKHOUSE_CURL_COMMAND} -vs "${CLICKHOUSE_URL}&wait_end_of_query=1&send_progress_in_http_headers=0&max_execution_time=1" --data-binary @- 2>&1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
@@ -147,8 +147,8 @@ class TCPHandler : public Poco::Net::TCPServerConnection | |||
* because it allows to check the IP ranges of the trusted proxy. | |||
* Proxy-forwarded (original client) IP address is used for quota accounting if quota is keyed by forwarded IP. | |||
*/ | |||
TCPHandler(IServer & server_, TCPServer & tcp_server_, const Poco::Net::StreamSocket & socket_, bool parse_proxy_protocol_, std::string server_display_name_); | |||
TCPHandler(IServer & server_, TCPServer & tcp_server_, const Poco::Net::StreamSocket & socket_, TCPProtocolStackData & stack_data, std::string server_display_name_); | |||
TCPHandler(IServer & server_, TCPServer & tcp_server_, const Poco::Net::StreamSocket & socket_, bool parse_proxy_protocol_, std::string server_display_name_, const ProfileEvents::Event & read_event_ = ProfileEvents::end(), const ProfileEvents::Event & write_event_ = ProfileEvents::end()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Event can be passed by value, and also we can use std::optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const &
accommodates by value and I made defaults rather than optional to make changes less intrusive...
@@ -33,7 +33,9 @@ class PostgreSQLHandler : public Poco::Net::TCPServerConnection | |||
TCPServer & tcp_server_, | |||
bool ssl_enabled_, | |||
Int32 connection_id_, | |||
std::vector<std::shared_ptr<PostgreSQLProtocol::PGAuthentication::AuthenticationMethod>> & auth_methods_); | |||
std::vector<std::shared_ptr<PostgreSQLProtocol::PGAuthentication::AuthenticationMethod>> & auth_methods_, | |||
const ProfileEvents::Event & read_event_ = ProfileEvents::end(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
src/Server/NotFoundHandler.cpp
Outdated
<< " clickhouse-client --query='SELECT 1' > result\n" | ||
<< " clickhouse-client < query > result\n"; | ||
auto out = response.send(); | ||
out->write("There is no handle "); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also a nice thing - IO/Operators.h
(optional)
Wow, that's a huge improvement! This is really amazing! 23.11:
This PR:
|
* [GLUTEN-1632][CH]Daily Update Clickhouse Version (20240103) * [CH] Fix build due to ClickHouse/ClickHouse#56064 --------- Co-authored-by: kyligence-git <gluten@kyligence.io>
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Replace HTTP outgoing buffering based on std ostream with CH Buffer.
Add bytes counting metrics for interfaces.
closes #55576