(v2) Refactor HTTP server. Separate HTTP/1.x implementation. Support HTTP/2 using nghttp2#93386
Open
m0r0zk01 wants to merge 2 commits intoClickHouse:masterfrom
Open
(v2) Refactor HTTP server. Separate HTTP/1.x implementation. Support HTTP/2 using nghttp2#93386m0r0zk01 wants to merge 2 commits intoClickHouse:masterfrom
m0r0zk01 wants to merge 2 commits intoClickHouse:masterfrom
Conversation
1 task
f2c307e to
fed3d37
Compare
Contributor
|
Workflow [PR], commit [a0c1081] Summary: ❌
|
e316438 to
b709d7a
Compare
a7a30a0 to
cf47e29
Compare
92ec712 to
b37efaf
Compare
…TP/1.x implementation. Support HTTP/2 using nghttp2
Resolve merge conflicts between the HTTP/2 PR and master: - CI/infrastructure files (.gitmodules, requirements.txt): take master's version - `contrib/CMakeLists.txt`, `src/configure_config.cmake`, `src/Common/config.h.in`: keep both nghttp2 and new master additions (libcotp, clickstack, simdcomp, wasmedge, wasmtime) - `programs/server/Server.cpp`: merge PR's `http1_params`/`http2_params`/`thread_pool` with master's `server_settings` refactoring and `handlers_config_key` - `src/Server/HTTPHandler.cpp`, `src/Server/HTTPHandler.h`: adapt `CurrentThread::QueryScope` to new `QueryScope` type while keeping PR's `HTTPServerResponseBase` interface - `src/Server/WebUIRequestHandler.cpp`, `src/Server/WebUIRequestHandler.h`: adapt new `JemallocWebUIRequestHandler` and `ClickStackUIRequestHandler` to PR's `HTTPServerResponseBase` interface - `src/Server/IndexRequestHandler.cpp`: keep PR's `response.makeStream()` with master's `#embed` resource naming - `src/Server/HTTP/HTTP1/HTTP1ServerConnection.cpp`, `.h`: keep PR's free-function `sendErrorResponse` pattern - `src/Server/HTTP/HTTP1/HTTP1ServerResponse.cpp`: keep PR's version (master's fixed-length buffer handling requires methods not yet in PR's class hierarchy) - `src/IO/HTTPCommon.cpp`: keep PR's `HTTPServerResponseBase.h` include Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Refactor HTTP server. Separate HTTP/1.x implementation. Support HTTP/2 using nghttp2
Documentation entry for user-facing changes
Technical details
Move changes from #80141.
HTTPRequestHandlernow has a single virtual methodhandleRequestthat takesHTTPServerRequestandHTTPServerResponseBase.HTTPServerRequeststores the request method, path, and header fields. It also provides aReadBufferto read the body from. ThisReadBufferis passed toHTTPServerRequest's constructor, andhandleRequestknows nothing about its implementation.HTTPServerResponseBaseprovides storage for the response status and header fields. It also has an abstractmakeUniqueStreammethod that returns aWriteBufferFromHTTPServerResponseBasefor the response body. Now, this is the only way to send a response to the client. Previously, there were two ways that duplicated each other's logic:HTTPServerResponse::send()andWriteBufferFromHTTPServerResponse. They are now unified.HTTP/2 support is enabled via the <http2_server></http2_server> config section.
If HTTP/2 is enabled,
HTTPServerConnectionFactorycan decide whether the connection should be handled as HTTP/2. If the connection is secure, it checks ALPN. Otherwise, it checks if the client has sent an HTTP/2 preface.Now, there are two connection handlers:
HTTP1ServerConnectionandHTTP2ServerConnection. The first one works as before. It creates anHTTPServerRequestwith theReadBufferfrom the HTTP/1.x stream (fixed length/chunked) and anHTTPServerResponseBasewith theWriteBufferFromHTTPServerResponseBaseimplementation based on the oldWriteBufferFromHTTPServerResponseclass.HTTP2ServerConnectionhandles an HTTP/2 connection. It has a main IO thread that exchanges HTTP/2 frames with the client. For each new stream, it creates a new thread that runshandleRequest. The HTTP/2 stream provides its own implementations ofReadBufferandWriteBufferFromHTTPServerResponseBase.One important thing to mention: the HTTP/1.x server had a feature for sending
X-ClickHouse-Progressheaders to the client over time to report query progress. For HTTP/2, we can't implement the same thing easily (at least because HEADERS (+CONTINUATION) frames must be sent together, without any other frame in between (RFC 9113 section 4.3.)). So,WriteBufferFromHTTPServerResponseBase::onProgressimplementation for HTTP/1.x remains the same as before, while its HTTP/2 implementation is a no-op. I believe the correct way to implement progress reporting for HTTP/2 is via 1xx informational responses.