Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(bigtable): modern Data API policy options #9320

Merged
merged 1 commit into from
Jun 21, 2022
Merged

feat(bigtable): modern Data API policy options #9320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions google/cloud/bigtable/internal/async_bulk_apply.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN

future<std::vector<bigtable::FailedMutation>> AsyncBulkApplier::Create(
CompletionQueue cq, std::shared_ptr<BigtableStub> stub,
std::unique_ptr<DataRetryPolicy> retry_policy,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy,
bigtable::IdempotentMutationPolicy& idempotent_policy,
std::string const& app_profile_id, std::string const& table_name,
Expand All @@ -42,7 +42,7 @@ future<std::vector<bigtable::FailedMutation>> AsyncBulkApplier::Create(

AsyncBulkApplier::AsyncBulkApplier(
CompletionQueue cq, std::shared_ptr<BigtableStub> stub,
std::unique_ptr<DataRetryPolicy> retry_policy,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy,
bigtable::IdempotentMutationPolicy& idempotent_policy,
std::string const& app_profile_id, std::string const& table_name,
Expand Down
6 changes: 3 additions & 3 deletions google/cloud/bigtable/internal/async_bulk_apply.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ class AsyncBulkApplier : public std::enable_shared_from_this<AsyncBulkApplier> {
public:
static future<std::vector<bigtable::FailedMutation>> Create(
CompletionQueue cq, std::shared_ptr<BigtableStub> stub,
std::unique_ptr<DataRetryPolicy> retry_policy,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy,
bigtable::IdempotentMutationPolicy& idempotent_policy,
std::string const& app_profile_id, std::string const& table_name,
bigtable::BulkMutation mut);

private:
AsyncBulkApplier(CompletionQueue cq, std::shared_ptr<BigtableStub> stub,
std::unique_ptr<DataRetryPolicy> retry_policy,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy,
bigtable::IdempotentMutationPolicy& idempotent_policy,
std::string const& app_profile_id,
Expand All @@ -62,7 +62,7 @@ class AsyncBulkApplier : public std::enable_shared_from_this<AsyncBulkApplier> {

CompletionQueue cq_;
std::shared_ptr<BigtableStub> stub_;
std::unique_ptr<DataRetryPolicy> retry_policy_;
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy_;
std::unique_ptr<BackoffPolicy> backoff_policy_;
bigtable::internal::BulkMutatorState state_;
std::atomic<bool> keep_reading_{true};
Expand Down
1 change: 1 addition & 0 deletions google/cloud/bigtable/internal/async_bulk_apply_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace {

namespace v2 = ::google::bigtable::v2;
using ms = std::chrono::milliseconds;
using ::google::cloud::bigtable::DataLimitedErrorCountRetryPolicy;
using ::google::cloud::bigtable::testing::MockAsyncMutateRowsStream;
using ::google::cloud::bigtable::testing::MockBigtableStub;
using ::google::cloud::testing_util::MockBackoffPolicy;
Expand Down
6 changes: 3 additions & 3 deletions google/cloud/bigtable/internal/async_row_reader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class AsyncRowReader : public std::enable_shared_from_this<AsyncRowReader> {
RowFunctor on_row, FinishFunctor on_finish,
bigtable::RowSet row_set, std::int64_t rows_limit,
bigtable::Filter filter,
std::unique_ptr<DataRetryPolicy> retry_policy,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy) {
auto reader = std::shared_ptr<AsyncRowReader>(new AsyncRowReader(
std::move(cq), std::move(stub), std::move(app_profile_id),
Expand All @@ -73,7 +73,7 @@ class AsyncRowReader : public std::enable_shared_from_this<AsyncRowReader> {
RowFunctor on_row, FinishFunctor on_finish,
bigtable::RowSet row_set, std::int64_t rows_limit,
bigtable::Filter filter,
std::unique_ptr<DataRetryPolicy> retry_policy,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy)
: cq_(std::move(cq)),
stub_(std::move(stub)),
Expand Down Expand Up @@ -128,7 +128,7 @@ class AsyncRowReader : public std::enable_shared_from_this<AsyncRowReader> {
bigtable::RowSet row_set_;
std::int64_t rows_limit_;
bigtable::Filter filter_;
std::unique_ptr<DataRetryPolicy> retry_policy_;
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy_;
std::unique_ptr<BackoffPolicy> backoff_policy_;
std::unique_ptr<bigtable::internal::ReadRowsParser> parser_;
/// Number of rows read so far, used to set row_limit in retries.
Expand Down
1 change: 1 addition & 0 deletions google/cloud/bigtable/internal/async_row_reader_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace {

namespace v2 = ::google::bigtable::v2;
using ms = std::chrono::milliseconds;
using ::google::cloud::bigtable::DataLimitedErrorCountRetryPolicy;
using ::google::cloud::bigtable::testing::MockAsyncReadRowsStream;
using ::google::cloud::bigtable::testing::MockBigtableStub;
using ::google::cloud::bigtable_internal::AsyncRowReader;
Expand Down
8 changes: 4 additions & 4 deletions google/cloud/bigtable/internal/async_row_sampler.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
namespace v2 = ::google::bigtable::v2;

future<StatusOr<std::vector<bigtable::RowKeySample>>> AsyncRowSampler::Create(
CompletionQueue cq, std::shared_ptr<bigtable_internal::BigtableStub> stub,
std::unique_ptr<bigtable_internal::DataRetryPolicy> retry_policy,
CompletionQueue cq, std::shared_ptr<BigtableStub> stub,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy,
std::string const& app_profile_id, std::string const& table_name) {
std::shared_ptr<AsyncRowSampler> sampler(new AsyncRowSampler(
Expand All @@ -37,8 +37,8 @@ future<StatusOr<std::vector<bigtable::RowKeySample>>> AsyncRowSampler::Create(
}

AsyncRowSampler::AsyncRowSampler(
CompletionQueue cq, std::shared_ptr<bigtable_internal::BigtableStub> stub,
std::unique_ptr<bigtable_internal::DataRetryPolicy> retry_policy,
CompletionQueue cq, std::shared_ptr<BigtableStub> stub,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy,
std::string const& app_profile_id, std::string const& table_name)
: cq_(std::move(cq)),
Expand Down
6 changes: 3 additions & 3 deletions google/cloud/bigtable/internal/async_row_sampler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ class AsyncRowSampler : public std::enable_shared_from_this<AsyncRowSampler> {
public:
static future<StatusOr<std::vector<bigtable::RowKeySample>>> Create(
CompletionQueue cq, std::shared_ptr<BigtableStub> stub,
std::unique_ptr<DataRetryPolicy> retry_policy,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy,
std::string const& app_profile_id, std::string const& table_name);

private:
AsyncRowSampler(CompletionQueue cq, std::shared_ptr<BigtableStub> stub,
std::unique_ptr<DataRetryPolicy> retry_policy,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy,
std::string const& app_profile_id,
std::string const& table_name);
Expand All @@ -55,7 +55,7 @@ class AsyncRowSampler : public std::enable_shared_from_this<AsyncRowSampler> {

CompletionQueue cq_;
std::shared_ptr<BigtableStub> stub_;
std::unique_ptr<DataRetryPolicy> retry_policy_;
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy_;
std::unique_ptr<BackoffPolicy> backoff_policy_;
std::string app_profile_id_;
std::string table_name_;
Expand Down
1 change: 1 addition & 0 deletions google/cloud/bigtable/internal/async_row_sampler_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace {

namespace v2 = ::google::bigtable::v2;
using ms = std::chrono::milliseconds;
using ::google::cloud::bigtable::DataLimitedErrorCountRetryPolicy;
using ::google::cloud::bigtable::testing::MockAsyncSampleRowKeysStream;
using ::google::cloud::bigtable::testing::MockBigtableStub;
using ::google::cloud::testing_util::MockBackoffPolicy;
Expand Down
5 changes: 3 additions & 2 deletions google/cloud/bigtable/internal/data_connection.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,9 @@ future<StatusOr<std::pair<bool, bigtable::Row>>> DataConnection::AsyncReadRow(
}

std::shared_ptr<DataConnection> MakeDataConnection(Options options) {
internal::CheckExpectedOptions<CommonOptionList, GrpcOptionList,
DataPolicyOptionList>(options, __func__);
google::cloud::internal::CheckExpectedOptions<
CommonOptionList, GrpcOptionList, bigtable::DataPolicyOptionList>(
options, __func__);
options = bigtable::internal::DefaultDataOptions(std::move(options));
auto background = internal::MakeBackgroundThreadsFactory(options)();
auto stub = bigtable_internal::CreateBigtableStub(background->cq(), options);
Expand Down
4 changes: 2 additions & 2 deletions google/cloud/bigtable/internal/data_connection_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ std::vector<bigtable::FailedMutation> DataConnectionImpl::BulkApply(
app_profile_id, table_name, *idempotency_policy(), std::move(mut));
// We wait to allocate the policies until they are needed as a
// micro-optimization.
std::unique_ptr<DataRetryPolicy> retry;
std::unique_ptr<bigtable::DataRetryPolicy> retry;
std::unique_ptr<BackoffPolicy> backoff;
do {
auto status = mutator.MakeOneRequest(*stub_);
Expand Down Expand Up @@ -265,7 +265,7 @@ StatusOr<std::vector<bigtable::RowKeySample>> DataConnectionImpl::SampleRows(

Status status;
std::vector<bigtable::RowKeySample> samples;
std::unique_ptr<DataRetryPolicy> retry;
std::unique_ptr<bigtable::DataRetryPolicy> retry;
std::unique_ptr<BackoffPolicy> backoff;
while (true) {
auto context = absl::make_unique<grpc::ClientContext>();
Expand Down
20 changes: 10 additions & 10 deletions google/cloud/bigtable/internal/data_connection_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,28 +104,28 @@ class DataConnectionImpl : public DataConnection {
std::string row_key, bigtable::Filter filter) override;

private:
std::unique_ptr<DataRetryPolicy> retry_policy() {
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<DataRetryPolicyOption>()) {
return options.get<DataRetryPolicyOption>()->clone();
if (options.has<bigtable::DataRetryPolicyOption>()) {
return options.get<bigtable::DataRetryPolicyOption>()->clone();
}
return options_.get<DataRetryPolicyOption>()->clone();
return options_.get<bigtable::DataRetryPolicyOption>()->clone();
}

std::unique_ptr<BackoffPolicy> backoff_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<DataBackoffPolicyOption>()) {
return options.get<DataBackoffPolicyOption>()->clone();
if (options.has<bigtable::DataBackoffPolicyOption>()) {
return options.get<bigtable::DataBackoffPolicyOption>()->clone();
}
return options_.get<DataBackoffPolicyOption>()->clone();
return options_.get<bigtable::DataBackoffPolicyOption>()->clone();
}

std::unique_ptr<bigtable::IdempotentMutationPolicy> idempotency_policy() {
auto const& options = internal::CurrentOptions();
if (options.has<IdempotentMutationPolicyOption>()) {
return options.get<IdempotentMutationPolicyOption>()->clone();
if (options.has<bigtable::IdempotentMutationPolicyOption>()) {
return options.get<bigtable::IdempotentMutationPolicyOption>()->clone();
}
return options_.get<IdempotentMutationPolicyOption>()->clone();
return options_.get<bigtable::IdempotentMutationPolicyOption>()->clone();
}

std::unique_ptr<BackgroundThreads> background_;
Expand Down
4 changes: 4 additions & 0 deletions google/cloud/bigtable/internal/data_connection_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
namespace {

namespace v2 = ::google::bigtable::v2;
using ::google::cloud::bigtable::DataBackoffPolicyOption;
using ::google::cloud::bigtable::DataLimitedErrorCountRetryPolicy;
using ::google::cloud::bigtable::DataRetryPolicyOption;
using ::google::cloud::bigtable::IdempotentMutationPolicyOption;
using ::google::cloud::bigtable::testing::MockAsyncReadRowsStream;
using ::google::cloud::bigtable::testing::MockBigtableStub;
using ::google::cloud::bigtable::testing::MockDataRetryPolicy;
Expand Down
3 changes: 2 additions & 1 deletion google/cloud/bigtable/internal/default_row_reader.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
DefaultRowReader::DefaultRowReader(
std::shared_ptr<BigtableStub> stub, std::string app_profile_id,
std::string table_name, bigtable::RowSet row_set, std::int64_t rows_limit,
bigtable::Filter filter, std::unique_ptr<DataRetryPolicy> retry_policy,
bigtable::Filter filter,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy)
: stub_(std::move(stub)),
app_profile_id_(std::move(app_profile_id)),
Expand Down
4 changes: 2 additions & 2 deletions google/cloud/bigtable/internal/default_row_reader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class DefaultRowReader : public RowReaderImpl {
std::string app_profile_id, std::string table_name,
bigtable::RowSet row_set, std::int64_t rows_limit,
bigtable::Filter filter,
std::unique_ptr<DataRetryPolicy> retry_policy,
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy,
std::unique_ptr<BackoffPolicy> backoff_policy);

~DefaultRowReader() override;
Expand Down Expand Up @@ -89,7 +89,7 @@ class DefaultRowReader : public RowReaderImpl {
bigtable::RowSet row_set_;
std::int64_t rows_limit_;
bigtable::Filter filter_;
std::unique_ptr<DataRetryPolicy> retry_policy_;
std::unique_ptr<bigtable::DataRetryPolicy> retry_policy_;
std::unique_ptr<BackoffPolicy> backoff_policy_;

std::unique_ptr<grpc::ClientContext> context_;
Expand Down
1 change: 1 addition & 0 deletions google/cloud/bigtable/internal/default_row_reader_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
namespace {

using ::google::bigtable::v2::ReadRowsRequest;
using ::google::cloud::bigtable::DataLimitedErrorCountRetryPolicy;
using ::google::cloud::bigtable::testing::MockBigtableStub;
using ::google::cloud::bigtable::testing::MockDataRetryPolicy;
using ::google::cloud::bigtable::testing::MockReadRowsStream;
Expand Down
14 changes: 7 additions & 7 deletions google/cloud/bigtable/internal/defaults.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,20 +183,20 @@ Options DefaultDataOptions(Options opts) {
if (!opts.has<AuthorityOption>()) {
opts.set<AuthorityOption>("bigtable.googleapis.com");
}
if (!opts.has<bigtable_internal::DataRetryPolicyOption>()) {
opts.set<bigtable_internal::DataRetryPolicyOption>(
bigtable_internal::DataLimitedTimeRetryPolicy(
if (!opts.has<bigtable::DataRetryPolicyOption>()) {
opts.set<bigtable::DataRetryPolicyOption>(
bigtable::DataLimitedTimeRetryPolicy(
kBigtableLimits.maximum_retry_period)
.clone());
}
if (!opts.has<bigtable_internal::DataBackoffPolicyOption>()) {
opts.set<bigtable_internal::DataBackoffPolicyOption>(
if (!opts.has<bigtable::DataBackoffPolicyOption>()) {
opts.set<bigtable::DataBackoffPolicyOption>(
ExponentialBackoffPolicy(kBigtableLimits.initial_delay / 2,
kBigtableLimits.maximum_delay, kBackoffScaling)
.clone());
}
if (!opts.has<bigtable_internal::IdempotentMutationPolicyOption>()) {
opts.set<bigtable_internal::IdempotentMutationPolicyOption>(
if (!opts.has<bigtable::IdempotentMutationPolicyOption>()) {
opts.set<bigtable::IdempotentMutationPolicyOption>(
bigtable::DefaultIdempotentMutationPolicy());
}
opts = DefaultOptions(std::move(opts));
Expand Down
6 changes: 3 additions & 3 deletions google/cloud/bigtable/internal/defaults_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,9 @@ TEST(OptionsTest, DefaultTableAdminOptions) {

TEST(OptionsTest, DefaultDataOptionsPolicies) {
auto options = DefaultDataOptions(Options{});
EXPECT_TRUE(options.has<bigtable_internal::DataRetryPolicyOption>());
EXPECT_TRUE(options.has<bigtable_internal::DataBackoffPolicyOption>());
EXPECT_TRUE(options.has<bigtable_internal::IdempotentMutationPolicyOption>());
EXPECT_TRUE(options.has<bigtable::DataRetryPolicyOption>());
EXPECT_TRUE(options.has<bigtable::DataBackoffPolicyOption>());
EXPECT_TRUE(options.has<bigtable::IdempotentMutationPolicyOption>());
}

TEST(OptionsTest, DataUserProjectOption) {
Expand Down
9 changes: 1 addition & 8 deletions google/cloud/bigtable/options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,6 @@ using ClientOptionList =
InstanceAdminEndpointOption, MinConnectionRefreshOption,
MaxConnectionRefreshOption>;

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
} // namespace bigtable

// TODO(#8860) - Make these options public.
namespace bigtable_internal {
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN

using DataRetryPolicy = ::google::cloud::internal::TraitBasedRetryPolicy<
bigtable::internal::SafeGrpcRetry>;

Expand Down Expand Up @@ -149,7 +142,7 @@ using DataPolicyOptionList =
IdempotentMutationPolicyOption>;

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
} // namespace bigtable_internal
} // namespace bigtable
} // namespace cloud
} // namespace google

Expand Down
4 changes: 2 additions & 2 deletions google/cloud/bigtable/testing/mock_policies.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ namespace cloud {
namespace bigtable {
namespace testing {

class MockDataRetryPolicy : public bigtable_internal::DataRetryPolicy {
class MockDataRetryPolicy : public bigtable::DataRetryPolicy {
public:
MOCK_METHOD(std::unique_ptr<bigtable_internal::DataRetryPolicy>, clone, (),
MOCK_METHOD(std::unique_ptr<bigtable::DataRetryPolicy>, clone, (),
(const, override));
MOCK_METHOD(bool, OnFailure, (Status const&), (override));
MOCK_METHOD(void, OnFailureImpl, (), (override));
Expand Down