Skip to content

Commit 25c4258

Browse files
committed
feat(native): Add http2 support for exchange client
1 parent 9dfe771 commit 25c4258

File tree

11 files changed

+53
-13
lines changed

11 files changed

+53
-13
lines changed

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,15 @@ The configuration properties of AsyncDataCache and SSD cache are described here.
553553
Exchange Properties
554554
-------------------
555555

556+
``exchange.http-client.http2-enabled``
557+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
558+
559+
* **Type:** ``boolean``
560+
* **Default value:** ``false``
561+
562+
Specifies whether HTTP/2 should be enabled for exchange HTTP client.
563+
564+
556565
``exchange.http-client.request-data-sizes-max-wait-sec``
557566
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
558567

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <sstream>
2020

2121
#include "presto_cpp/main/QueryContextManager.h"
22+
#include "presto_cpp/main/common/Configs.h"
2223
#include "presto_cpp/main/common/Counters.h"
2324
#include "velox/common/base/Exceptions.h"
2425
#include "velox/common/testutil/TestValue.h"
@@ -278,9 +279,11 @@ void PrestoExchangeSource::processDataResponse(
278279
return;
279280
}
280281
auto* headers = response->headers();
281-
VELOX_CHECK(
282-
!headers->getIsChunked(),
283-
"Chunked http transferring encoding is not supported.");
282+
if (!SystemConfig::instance()->exchangeHttp2Enabled()) {
283+
VELOX_CHECK(
284+
!headers->getIsChunked(),
285+
"Chunked http transferring encoding is not supported.");
286+
}
284287
const uint64_t contentLength =
285288
atol(headers->getHeaders()
286289
.getSingleOrEmpty(proxygen::HTTP_HEADER_CONTENT_LENGTH)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,8 @@ void PrestoServer::run() {
258258
}
259259

260260
sslContext_ =
261-
util::createSSLContext(optionalClientCertPath.value(), ciphers);
261+
util::createSSLContext(
262+
optionalClientCertPath.value(), ciphers, systemConfig->exchangeHttp2Enabled());
262263
}
263264

264265
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(kExchangeHttp2Enabled, false),
235236
STR_PROP(kExchangeMaxErrorDuration, "3m"),
236237
STR_PROP(kExchangeRequestTimeout, "20s"),
237238
STR_PROP(kExchangeConnectTimeout, "20s"),
@@ -832,6 +833,10 @@ uint64_t SystemConfig::heartbeatFrequencyMs() const {
832833
return optionalProperty<uint64_t>(kHeartbeatFrequencyMs).value();
833834
}
834835

836+
bool SystemConfig::exchangeHttp2Enabled() const {
837+
return optionalProperty<bool>(kExchangeHttp2Enabled).value();
838+
}
839+
835840
std::chrono::duration<double> SystemConfig::exchangeMaxErrorDuration() const {
836841
return velox::config::toDuration(
837842
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
@@ -638,6 +638,10 @@ class SystemConfig : public ConfigBase {
638638
static constexpr std::string_view kHeartbeatFrequencyMs{
639639
"heartbeat-frequency-ms"};
640640

641+
/// Whether HTTP/2 should be enabled for exchange HTTP client.
642+
static constexpr std::string_view kExchangeHttp2Enabled{
643+
"exchange.http-client.http2-enabled"};
644+
641645
static constexpr std::string_view kExchangeMaxErrorDuration{
642646
"exchange.max-error-duration"};
643647

@@ -1013,6 +1017,8 @@ class SystemConfig : public ConfigBase {
10131017

10141018
uint64_t heartbeatFrequencyMs() const;
10151019

1020+
bool exchangeHttp2Enabled() const;
1021+
10161022
std::chrono::duration<double> exchangeMaxErrorDuration() const;
10171023

10181024
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"
@@ -200,13 +201,22 @@ class ResponseHandler : public proxygen::HTTPTransactionHandler {
200201
return promise_.getSemiFuture();
201202
}
202203

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

208214
void onHeadersComplete(
209215
std::unique_ptr<proxygen::HTTPMessage> msg) noexcept override {
216+
if (protocol_.has_value()) {
217+
VLOG(2) << "HttpClient received response of "
218+
<< proxygen::getCodecProtocolString(protocol_.value());
219+
}
210220
response_ = std::make_unique<HttpResponse>(
211221
std::move(msg),
212222
client_->memoryPool(),
@@ -267,6 +277,7 @@ class ResponseHandler : public proxygen::HTTPTransactionHandler {
267277
folly::Promise<std::unique_ptr<HttpResponse>> promise_;
268278
std::shared_ptr<ResponseHandler> self_;
269279
std::shared_ptr<HttpClient> client_;
280+
std::optional<proxygen::CodecProtocol> protocol_;
270281
};
271282

272283
// 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)