More explicitly drain the leftover bytes in buffer for the POST request in HTTPHander#81561
Conversation
clickhouse-gh
bot
added
the
pr-bugfix
| { | ||
| auto hex_string = hexString(remaining.data(), remaining.size()); | ||
| #if defined(DEBUG_OR_SANITIZER_BUILD) | ||
| throw Exception(ErrorCodes::LOGICAL_ERROR, |
There was a problem hiding this comment.
Won't the test throw this logical error then?
There was a problem hiding this comment.
In the test, the remaining bytes are the chunked encoding footer, not logical payload bytes. The HTTPChunkedReadBuffer will eat those bytes from its underlying ReadBuffer, but won't output any more bytes to higher-level ReadBuffer-s such as in_post_maybe_compressed.
| throw Exception(ErrorCodes::LOGICAL_ERROR, | ||
| "There is still some data in the buffer: {}", hex_string); | ||
| #else | ||
| LOG_WARNING(log, "There is still some data in the buffer: {}", hex_string); |
There was a problem hiding this comment.
This is printing huge strings for me with the test code in #81461.
This is just a fraction:
2025.06.10 10:10:48.586380 [ 1054220 ] {9a5f038d-8aa4-40fa-ae23-e60cfba0f6dc} <Warning> DynamicQueryHandler: There is still some data in the buffer: 3765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497a513765497
| if (!has_external_data && request.isStreamBounded() && response.getKeepAlive()) | ||
| { | ||
| String remaining; | ||
| readStringUntilEOF(remaining, *in_post_maybe_compressed); |
There was a problem hiding this comment.
I think it is wrong to read data here, we do not know how much data there. We do not know why it is here.
It could be that we spend servers and clients resources just to drop the data.
This is my version:
#81595
4 participants
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fixed rare HTTP errors (
Invalid HTTP version string: /?query=I) after INSERT using ArrowStream format, HTTP chunked encoding, and connection keep-alive.Like #81461 , but with a simpler test and more reliable drain condition.