Skip to content

Commit

Permalink
enable connection reuse in libcurl
Browse files Browse the repository at this point in the history
Originally, in the `HttpClient::perform()` method, a new `CurlHandle` was created
upon every invocation, leading to subsequent calls to `curl_easy_init()` and
`curl_easy_cleanup()`, as the variable went in and out of scope while it executed
on the `asio::thread_pool`. This meant that there was no opportunity to reuse
connections from the underlying connection pool, because it was always cleaned up.

By marking the `CurlHandle` with the `thread_local` storage duration, it ensures
that the same one remains in use for each thread, thus providing the means to
reuse the established connection pool.

Fixes Netflix-Skunkworks#51.
  • Loading branch information
copperlight committed Oct 20, 2023
1 parent 79a2713 commit f51cdb2
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 4 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
run: |
tools/generate_version.sh > spectator/version.h
cd $BUILD_DIR
cmake .. -DCMAKE_BUILD_TYPE=Release
cmake .. -DCMAKE_BUILD_TYPE=Debug
cmake --build .
echo "==== ldd ===="
ldd bin/spectatord_main || true
Expand Down
8 changes: 7 additions & 1 deletion build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,13 @@ cmake --build . || exit 1

if [[ "$1" != "skiptest" ]]; then
echo -e "${BLUE}==== test ====${NC}"
GTEST_COLOR=1 ASAN_OPTIONS=detect_container_overflow=0 ctest --verbose

if [[ "$(uname -s)" == "Darwin" ]]; then
export ASAN_OPTIONS="detect_container_overflow=0"
echo "== export ASAN_OPTIONS=$ASAN_OPTIONS"
fi

GTEST_COLOR=1 ctest --verbose
fi

popd || exit 1
9 changes: 7 additions & 2 deletions spectator/http_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,12 @@ class CurlHandle {

auto handle() const noexcept -> CURL* { return handle_; }

auto perform() -> CURLcode { return curl_easy_perform(handle()); }
auto perform() -> CURLcode {
if (response_.size() > 0) {
response_.clear();
}
return curl_easy_perform(handle());
}

auto set_opt(CURLoption option, const void* param) -> CURLcode {
return curl_easy_setopt(handle(), option, param);
Expand Down Expand Up @@ -199,7 +204,7 @@ auto HttpClient::perform(const char* method, const std::string& url,
int attempt_number) const -> HttpResponse {
LogEntry entry{registry_, method, url};

CurlHandle curl;
thread_local CurlHandle curl;
auto total_timeout = config_.connect_timeout + config_.read_timeout;
curl.set_timeout(total_timeout);
curl.set_connect_timeout(config_.connect_timeout);
Expand Down

0 comments on commit f51cdb2

Please sign in to comment.