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

impl(bigtable): Table accepts Options #9307

Merged
merged 1 commit into from
Jun 19, 2022
Merged

impl(bigtable): Table accepts Options #9307

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
13 changes: 13 additions & 0 deletions google/cloud/bigtable/table.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ static_assert(std::is_copy_assignable<bigtable::Table>::value,

Status Table::Apply(SingleRowMutation mut) {
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could/should these be outside the if (connection_) conditional?

Copy link
Member Author

@dbolduc dbolduc Jun 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spans could be outside the if conditionals, but there is no point. The code outside of the if conditionals does not care about CurrentOptions(). I'd rather have the spans inside the if blocks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code outside of the if conditionals does not care about CurrentOptions().

OK, although I'll assert that it is difficult to know that, and it may be better to be safe than sorry.

return connection_->Apply(app_profile_id_, table_name_, std::move(mut));
}

Expand Down Expand Up @@ -94,6 +95,7 @@ Status Table::Apply(SingleRowMutation mut) {

future<Status> Table::AsyncApply(SingleRowMutation mut) {
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->AsyncApply(app_profile_id_, table_name_,
std::move(mut));
}
Expand Down Expand Up @@ -138,6 +140,7 @@ future<Status> Table::AsyncApply(SingleRowMutation mut) {

std::vector<FailedMutation> Table::BulkApply(BulkMutation mut) {
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->BulkApply(app_profile_id_, table_name_, std::move(mut));
}

Expand Down Expand Up @@ -171,6 +174,7 @@ std::vector<FailedMutation> Table::BulkApply(BulkMutation mut) {

future<std::vector<FailedMutation>> Table::AsyncBulkApply(BulkMutation mut) {
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->AsyncBulkApply(app_profile_id_, table_name_,
std::move(mut));
}
Expand All @@ -191,6 +195,7 @@ RowReader Table::ReadRows(RowSet row_set, Filter filter) {
RowReader Table::ReadRows(RowSet row_set, std::int64_t rows_limit,
Filter filter) {
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->ReadRows(app_profile_id_, table_name_,
std::move(row_set), rows_limit,
std::move(filter));
Expand All @@ -207,6 +212,7 @@ RowReader Table::ReadRows(RowSet row_set, std::int64_t rows_limit,
StatusOr<std::pair<bool, Row>> Table::ReadRow(std::string row_key,
Filter filter) {
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->ReadRow(app_profile_id_, table_name_,
std::move(row_key), std::move(filter));
}
Expand Down Expand Up @@ -236,6 +242,7 @@ StatusOr<MutationBranch> Table::CheckAndMutateRow(
std::string row_key, Filter filter, std::vector<Mutation> true_mutations,
std::vector<Mutation> false_mutations) {
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->CheckAndMutateRow(
app_profile_id_, table_name_, std::move(row_key), std::move(filter),
std::move(true_mutations), std::move(false_mutations));
Expand Down Expand Up @@ -272,6 +279,7 @@ future<StatusOr<MutationBranch>> Table::AsyncCheckAndMutateRow(
std::string row_key, Filter filter, std::vector<Mutation> true_mutations,
std::vector<Mutation> false_mutations) {
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->AsyncCheckAndMutateRow(
app_profile_id_, table_name_, std::move(row_key), std::move(filter),
std::move(true_mutations), std::move(false_mutations));
Expand Down Expand Up @@ -325,6 +333,7 @@ future<StatusOr<MutationBranch>> Table::AsyncCheckAndMutateRow(
// samples otherwise the result is an inconsistent set of sample row keys.
StatusOr<std::vector<bigtable::RowKeySample>> Table::SampleRows() {
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->SampleRows(app_profile_id_, table_name_);
}

Expand Down Expand Up @@ -370,6 +379,7 @@ StatusOr<std::vector<bigtable::RowKeySample>> Table::SampleRows() {

future<StatusOr<std::vector<bigtable::RowKeySample>>> Table::AsyncSampleRows() {
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->AsyncSampleRows(app_profile_id_, table_name_);
}

Expand All @@ -385,6 +395,7 @@ StatusOr<Row> Table::ReadModifyWriteRowImpl(
::google::bigtable::v2::ReadModifyWriteRowRequest>(
request, app_profile_id_, table_name_);
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->ReadModifyWriteRow(std::move(request));
}

Expand All @@ -406,6 +417,7 @@ future<StatusOr<Row>> Table::AsyncReadModifyWriteRowImpl(
::google::bigtable::v2::ReadModifyWriteRowRequest>(
request, app_profile_id_, table_name_);
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->AsyncReadModifyWriteRow(std::move(request));
}

Expand Down Expand Up @@ -435,6 +447,7 @@ future<StatusOr<Row>> Table::AsyncReadModifyWriteRowImpl(
future<StatusOr<std::pair<bool, Row>>> Table::AsyncReadRow(std::string row_key,
Filter filter) {
if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
return connection_->AsyncReadRow(app_profile_id_, table_name_,
std::move(row_key), std::move(filter));
}
Expand Down
20 changes: 14 additions & 6 deletions google/cloud/bigtable/table.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "google/cloud/bigtable/filters.h"
#include "google/cloud/bigtable/idempotent_mutation_policy.h"
#include "google/cloud/bigtable/internal/data_connection.h"
#include "google/cloud/bigtable/internal/defaults.h"
#include "google/cloud/bigtable/internal/legacy_async_row_reader.h"
#include "google/cloud/bigtable/mutation_branch.h"
#include "google/cloud/bigtable/mutations.h"
Expand Down Expand Up @@ -52,7 +53,8 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
// public.
bigtable::Table MakeTable(std::shared_ptr<DataConnection> conn,
std::string project_id, std::string instance_id,
std::string app_profile_id, std::string table_id);
std::string app_profile_id, std::string table_id,
Options options = {});

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
} // namespace bigtable_internal
Expand Down Expand Up @@ -888,6 +890,7 @@ class Table {
"RowFunctor should return a future<bool>.");

if (connection_) {
google::cloud::internal::OptionsSpan span(options_);
connection_->AsyncReadRows(
app_profile_id_, table_name_, std::move(on_row), std::move(on_finish),
std::move(row_set), rows_limit, std::move(filter));
Expand Down Expand Up @@ -937,18 +940,22 @@ class Table {
private:
friend Table bigtable_internal::MakeTable(
std::shared_ptr<bigtable_internal::DataConnection>, std::string,
std::string, std::string, std::string);
std::string, std::string, std::string, Options);
explicit Table(std::shared_ptr<bigtable_internal::DataConnection> conn,
std::string project_id, std::string instance_id,
std::string app_profile_id, std::string table_id)
std::string app_profile_id, std::string table_id,
Options options = {})
: app_profile_id_(std::move(app_profile_id)),
project_id_(std::move(project_id)),
instance_id_(std::move(instance_id)),
table_name_(TableName(project_id_, instance_id_, table_id)),
table_id_(std::move(table_id)),
metadata_update_policy_(
MetadataUpdatePolicy(table_name_, MetadataParamTypes::TABLE_NAME)),
connection_(std::move(conn)) {}
connection_(std::move(conn)),
options_(google::cloud::internal::MergeOptions(
std::move(options),
internal::DefaultDataOptions(connection_->options()))) {}

/**
* Send request ReadModifyWriteRowRequest to modify the row and get it back
Expand Down Expand Up @@ -1021,6 +1028,7 @@ class Table {
std::shared_ptr<IdempotentMutationPolicy> idempotent_mutation_policy_;
std::shared_ptr<BackgroundThreads> background_threads_;
std::shared_ptr<bigtable_internal::DataConnection> connection_;
Options options_;
};

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand All @@ -1032,10 +1040,10 @@ inline bigtable::Table MakeTable(std::shared_ptr<DataConnection> conn,
std::string project_id,
std::string instance_id,
std::string app_profile_id,
std::string table_id) {
std::string table_id, Options options) {
return bigtable::Table(std::move(conn), std::move(project_id),
std::move(instance_id), std::move(app_profile_id),
std::move(table_id));
std::move(table_id), std::move(options));
}

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand Down
Loading