Skip to content

Commit 1e342de

Browse files
committed
using completetion_queue to run background threads
1 parent d63973e commit 1e342de

3 files changed

Lines changed: 32 additions & 35 deletions

File tree

google/cloud/storage/internal/tracing_connection.cc

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@
1717
#include "google/cloud/storage/options.h"
1818
#include "google/cloud/storage/parallel_upload.h"
1919
#include "google/cloud/internal/opentelemetry.h"
20+
#include "google/cloud/internal/rest_pure_background_threads_impl.h"
2021
#include "google/cloud/options.h"
22+
#if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
23+
#include "google/cloud/grpc_options.h"
24+
#endif
2125
#include <algorithm>
2226
#include <memory>
2327
#include <string>
@@ -29,15 +33,27 @@ namespace cloud {
2933
namespace storage_internal {
3034
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
3135

36+
namespace {
37+
std::size_t DefaultThreadPoolSize(Options const& options) {
38+
#if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
39+
auto pool_size = options.get<GrpcBackgroundThreadPoolSizeOption>();
40+
if (pool_size == 0) return 1U;
41+
return pool_size;
42+
#else
43+
(void)options;
44+
return 1U;
45+
#endif
46+
}
47+
} // namespace
48+
3249
TracingConnection::TracingConnection(std::shared_ptr<StorageConnection> impl)
33-
: impl_(std::move(impl)) {}
50+
: impl_(std::move(impl)),
51+
background_threads_(
52+
std::make_unique<
53+
rest_internal::AutomaticallyCreatedRestPureBackgroundThreads>(
54+
DefaultThreadPoolSize(impl_->options()))) {}
3455

35-
TracingConnection::~TracingConnection() {
36-
std::lock_guard<std::mutex> lk(mu_);
37-
for (auto& f : bg_tasks_) {
38-
if (f.valid()) f.wait();
39-
}
40-
}
56+
TracingConnection::~TracingConnection() = default;
4157

4258
BucketMetadataCache& TracingConnection::cache() {
4359
static BucketMetadataCache instance(10000);
@@ -48,16 +64,6 @@ void TracingConnection::ResetCacheForTesting() { cache().Clear(); }
4864

4965
Options TracingConnection::options() const { return impl_->options(); }
5066

51-
void TracingConnection::CleanupCompletedTasks() {
52-
std::unique_lock<std::mutex> lk(mu_);
53-
bg_tasks_.erase(std::remove_if(bg_tasks_.begin(), bg_tasks_.end(),
54-
[](std::future<void> const& f) {
55-
return f.wait_for(std::chrono::seconds(0)) ==
56-
std::future_status::ready;
57-
}),
58-
bg_tasks_.end());
59-
}
60-
6167
void TracingConnection::EnrichSpan(opentelemetry::trace::Span& span,
6268
BucketCacheEntry const& entry) {
6369
span.SetAttribute("gcp.resource.destination.id", entry.id);
@@ -66,15 +72,12 @@ void TracingConnection::EnrichSpan(opentelemetry::trace::Span& span,
6672

6773
void TracingConnection::MaybeTriggerBackgroundFetch(
6874
std::string const& bucket_name) {
69-
CleanupCompletedTasks();
70-
7175
if (!cache().StartFetch(bucket_name)) {
7276
return;
7377
}
7478

7579
auto current_options = google::cloud::internal::SaveCurrentOptions();
76-
auto f = std::async(std::launch::async, [this, bucket_name,
77-
current_options]() {
80+
background_threads_->cq().RunAsync([this, bucket_name, current_options]() {
7881
google::cloud::internal::OptionsSpan span(current_options);
7982
storage::internal::GetBucketMetadataRequest request(bucket_name);
8083
auto result = impl_->GetBucketMetadata(request);
@@ -87,11 +90,6 @@ void TracingConnection::MaybeTriggerBackgroundFetch(
8790

8891
cache().EndFetch(bucket_name);
8992
});
90-
91-
{
92-
std::lock_guard<std::mutex> lk(mu_);
93-
bg_tasks_.push_back(std::move(f));
94-
}
9593
}
9694

9795
void TracingConnection::EnrichSpan(opentelemetry::trace::Span& span,
@@ -108,8 +106,9 @@ void TracingConnection::EnrichSpan(opentelemetry::trace::Span& span,
108106
}
109107
}
110108

111-
void TracingConnection::EnrichSpan(opentelemetry::trace::Span& span,
112-
storage::BucketMetadata const& metadata) const {
109+
void TracingConnection::EnrichSpan(
110+
opentelemetry::trace::Span& span,
111+
storage::BucketMetadata const& metadata) const {
113112
auto const enabled =
114113
options().get<storage_experimental::OTelSpanEnrichmentOption>();
115114
if (!enabled) return;

google/cloud/storage/internal/tracing_connection.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,9 @@
1919
#include "google/cloud/storage/internal/storage_connection.h"
2020
#include "google/cloud/storage/parallel_upload.h"
2121
#include "google/cloud/storage/version.h"
22-
#include <future>
22+
#include "google/cloud/internal/rest_pure_background_threads_impl.h"
2323
#include <memory>
24-
#include <mutex>
2524
#include <string>
26-
#include <vector>
2725

2826
namespace google {
2927
namespace cloud {
@@ -190,7 +188,6 @@ class TracingConnection : public storage::internal::StorageConnection {
190188
static void EnrichSpan(opentelemetry::trace::Span& span,
191189
BucketCacheEntry const& entry);
192190
void MaybeTriggerBackgroundFetch(std::string const& bucket_name);
193-
void CleanupCompletedTasks();
194191

195192
static void MaybeInvalidate(Status const& status,
196193
std::string const& bucket_name) {
@@ -208,8 +205,8 @@ class TracingConnection : public storage::internal::StorageConnection {
208205
static BucketMetadataCache& cache();
209206

210207
std::shared_ptr<StorageConnection> impl_;
211-
std::mutex mu_;
212-
std::vector<std::future<void>> bg_tasks_;
208+
std::unique_ptr<google::cloud::rest_internal::RestPureBackgroundThreads>
209+
background_threads_;
213210
};
214211

215212
std::shared_ptr<storage::internal::StorageConnection> MakeTracingClient(

google/cloud/storage/internal/tracing_connection_test.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ TEST(TracingClientTest, Options) {
5757
};
5858

5959
auto mock = std::make_shared<MockClient>();
60-
EXPECT_CALL(*mock, options).WillOnce(Return(Options{}.set<TestOption>(42)));
60+
EXPECT_CALL(*mock, options)
61+
.WillRepeatedly(Return(Options{}.set<TestOption>(42)));
6162
auto under_test = TracingConnection(mock);
6263
auto const options = under_test.options();
6364
EXPECT_EQ(42, options.get<TestOption>());

0 commit comments

Comments
 (0)