Skip to content

Commit a252163

Browse files
authored
feat(native): Add http2 support for HTTP client (#26439)
``` == RELEASE NOTES == General Changes * Add http2 support for HTTP client ```
1 parent e97b004 commit a252163

File tree

11 files changed

+56
-14
lines changed

11 files changed

+56
-14
lines changed

presto-docs/src/main/sphinx/presto_cpp/properties.rst

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,17 @@ Exchange Properties
561561

562562
Maximum wait time for exchange request in seconds.
563563

564+
HTTP Client Properties
565+
----------------------
566+
567+
``http-client.http2-enabled``
568+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
569+
570+
* **Type:** ``boolean``
571+
* **Default value:** ``false``
572+
573+
Specifies whether HTTP/2 should be enabled for HTTP client.
574+
564575
Memory Checker Properties
565576
-------------------------
566577

presto-native-execution/presto_cpp/main/PrestoExchangeSource.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -278,9 +278,11 @@ void PrestoExchangeSource::processDataResponse(
278278
return;
279279
}
280280
auto* headers = response->headers();
281-
VELOX_CHECK(
282-
!headers->getIsChunked(),
283-
"Chunked http transferring encoding is not supported.");
281+
if (!SystemConfig::instance()->httpClientHttp2Enabled()) {
282+
VELOX_CHECK(
283+
!headers->getIsChunked(),
284+
"Chunked http transferring encoding is not supported.");
285+
}
284286
const uint64_t contentLength =
285287
atol(headers->getHeaders()
286288
.getSingleOrEmpty(proxygen::HTTP_HEADER_CONTENT_LENGTH)

presto-native-execution/presto_cpp/main/PrestoServer.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,10 @@ void PrestoServer::run() {
259259
"Https Client Certificates are not configured correctly");
260260
}
261261

262-
sslContext_ =
263-
util::createSSLContext(optionalClientCertPath.value(), ciphers);
262+
sslContext_ = util::createSSLContext(
263+
optionalClientCertPath.value(),
264+
ciphers,
265+
systemConfig->httpClientHttp2Enabled());
264266
}
265267

266268
if (systemConfig->internalCommunicationJwtEnabled()) {

presto-native-execution/presto_cpp/main/common/Configs.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ SystemConfig::SystemConfig() {
232232
NUM_PROP(kLogNumZombieTasks, 20),
233233
NUM_PROP(kAnnouncementMaxFrequencyMs, 30'000), // 30s
234234
NUM_PROP(kHeartbeatFrequencyMs, 0),
235+
BOOL_PROP(kHttpClientHttp2Enabled, false),
235236
STR_PROP(kExchangeMaxErrorDuration, "3m"),
236237
STR_PROP(kExchangeRequestTimeout, "20s"),
237238
STR_PROP(kExchangeConnectTimeout, "20s"),
@@ -850,6 +851,10 @@ uint64_t SystemConfig::heartbeatFrequencyMs() const {
850851
return optionalProperty<uint64_t>(kHeartbeatFrequencyMs).value();
851852
}
852853

854+
bool SystemConfig::httpClientHttp2Enabled() const {
855+
return optionalProperty<bool>(kHttpClientHttp2Enabled).value();
856+
}
857+
853858
std::chrono::duration<double> SystemConfig::exchangeMaxErrorDuration() const {
854859
return velox::config::toDuration(
855860
optionalProperty(kExchangeMaxErrorDuration).value());

presto-native-execution/presto_cpp/main/common/Configs.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,10 @@ class SystemConfig : public ConfigBase {
648648
static constexpr std::string_view kHeartbeatFrequencyMs{
649649
"heartbeat-frequency-ms"};
650650

651+
/// Whether HTTP/2 is enabled for HTTP client connections.
652+
static constexpr std::string_view kHttpClientHttp2Enabled{
653+
"http-client.http2-enabled"};
654+
651655
static constexpr std::string_view kExchangeMaxErrorDuration{
652656
"exchange.max-error-duration"};
653657

@@ -1029,6 +1033,8 @@ class SystemConfig : public ConfigBase {
10291033

10301034
uint64_t heartbeatFrequencyMs() const;
10311035

1036+
bool httpClientHttp2Enabled() const;
1037+
10321038
std::chrono::duration<double> exchangeMaxErrorDuration() const;
10331039

10341040
std::chrono::duration<double> exchangeRequestTimeoutMs() const;

presto-native-execution/presto_cpp/main/common/Utils.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,18 @@ DateTime toISOTimestamp(uint64_t timeMilli) {
3131

3232
std::shared_ptr<folly::SSLContext> createSSLContext(
3333
const std::string& clientCertAndKeyPath,
34-
const std::string& ciphers) {
34+
const std::string& ciphers,
35+
bool http2Enabled) {
3536
try {
3637
auto sslContext = std::make_shared<folly::SSLContext>();
3738
sslContext->loadCertKeyPairFromFiles(
3839
clientCertAndKeyPath.c_str(), clientCertAndKeyPath.c_str());
3940
sslContext->setCiphersOrThrow(ciphers);
40-
sslContext->setAdvertisedNextProtocols({"http/1.1"});
41+
if (http2Enabled) {
42+
sslContext->setAdvertisedNextProtocols({"h2", "http/1.1"});
43+
} else {
44+
sslContext->setAdvertisedNextProtocols({"http/1.1"});
45+
}
4146
return sslContext;
4247
} catch (const std::exception& ex) {
4348
LOG(FATAL) << fmt::format(

presto-native-execution/presto_cpp/main/common/Utils.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ DateTime toISOTimestamp(uint64_t timeMilli);
3030

3131
std::shared_ptr<folly::SSLContext> createSSLContext(
3232
const std::string& clientCertAndKeyPath,
33-
const std::string& ciphers);
33+
const std::string& ciphers,
34+
bool http2Enabled);
3435

3536
/// Returns current process-wide CPU time in nanoseconds.
3637
long getProcessCpuTimeNs();

presto-native-execution/presto_cpp/main/http/HttpClient.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#endif // PRESTO_ENABLE_JWT
1919
#include <folly/io/async/EventBaseManager.h>
2020
#include <folly/synchronization/Latch.h>
21+
#include <proxygen/lib/http/codec/CodecProtocol.h>
2122
#include <velox/common/base/Exceptions.h>
2223
#include "presto_cpp/main/common/Configs.h"
2324
#include "presto_cpp/main/common/Counters.h"
@@ -201,13 +202,22 @@ class ResponseHandler : public proxygen::HTTPTransactionHandler {
201202
return promise_.getSemiFuture();
202203
}
203204

204-
void setTransaction(proxygen::HTTPTransaction* /* txn */) noexcept override {}
205+
void setTransaction(proxygen::HTTPTransaction* txn) noexcept override {
206+
if (txn) {
207+
protocol_ = txn->getTransport().getCodec().getProtocol();
208+
}
209+
}
210+
205211
void detachTransaction() noexcept override {
206212
self_.reset();
207213
}
208214

209215
void onHeadersComplete(
210216
std::unique_ptr<proxygen::HTTPMessage> msg) noexcept override {
217+
if (protocol_.has_value()) {
218+
VLOG(2) << "HttpClient received response of "
219+
<< proxygen::getCodecProtocolString(protocol_.value());
220+
}
211221
response_ = std::make_unique<HttpResponse>(
212222
std::move(msg),
213223
client_->memoryPool(),
@@ -268,6 +278,7 @@ class ResponseHandler : public proxygen::HTTPTransactionHandler {
268278
folly::Promise<std::unique_ptr<HttpResponse>> promise_;
269279
std::shared_ptr<ResponseHandler> self_;
270280
std::shared_ptr<HttpClient> client_;
281+
std::optional<proxygen::CodecProtocol> protocol_;
271282
};
272283

273284
// Responsible for making an HTTP request. The request will be made in 2

presto-native-execution/presto_cpp/main/http/HttpClient.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,7 @@ class HttpClient : public std::enable_shared_from_this<HttpClient> {
214214

215215
class RequestBuilder {
216216
public:
217-
RequestBuilder() {
218-
headers_.setHTTPVersion(1, 1);
219-
}
217+
RequestBuilder() {}
220218

221219
RequestBuilder& method(proxygen::HTTPMethod method) {
222220
headers_.setMethod(method);

presto-native-execution/presto_cpp/main/tests/AnnouncerTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ class AnnouncerTestSuite : public ::testing::TestWithParam<bool> {
104104

105105
std::string keyPath = getCertsPath("client_ca.pem");
106106
std::string ciphers = "ECDHE-ECDSA-AES256-GCM-SHA384,AES256-GCM-SHA384";
107-
sslContext_ = util::createSSLContext(keyPath, ciphers);
107+
sslContext_ = util::createSSLContext(keyPath, ciphers, false);
108108
}
109109

110110
protected:

0 commit comments

Comments
 (0)