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.

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(storage): promote gRPC plugin to GA #14712

Merged
merged 23 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 17 additions & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,23 @@ bool_flag(
visibility = ["//:__subpackages__"],
)

cc_library(
name = "storage_grpc",
hdrs = ["//google/cloud/storage:public_grpc_hdrs"],
deps = [
"//google/cloud/storage:google_cloud_cpp_storage_grpc",
],
)

cc_library(
name = "storage_grpc_mocks",
testonly = True,
hdrs = ["//google/cloud/storage:grpc_mocks_hdrs"],
deps = [
"//google/cloud/storage:google_cloud_cpp_storage_grpc_mocks",
],
)

cc_library(
name = "experimental-storage_grpc",
hdrs = ["//google/cloud/storage:public_grpc_hdrs"],
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ breaking changes in the upcoming 3.x release. This release is scheduled for
using this API, and therefore we do not consider this a breaking change.
([#14726](https://github.com/googleapis/google-cloud-cpp/pull/14726))

### [Google Cloud Storage](/google/cloud/storage/README.md)

- The gRPC plugin is now GA. The [Using the gRPC plugin][storage-grpc]
guide describes this feature in more detail. When using GCS from Google
Compute Engine (GCE) this plugin can enable higher total throughput.

## v2.29.0 - 2024-09

### New Libraries
Expand Down Expand Up @@ -1810,3 +1816,4 @@ case it elicits some feedback that requires changes.
[product-launch-stages]: https://cloud.google.com/products/#product-launch-stages
[resource-manager-tags]: https://cloud.google.com/resource-manager/docs/tags/tags-overview
[speech-model-adaptation]: https://cloud.google.com/speech-to-text/docs/adaptation-model
[storage-grpc]: https://cloud.google.com/cpp/docs/reference/storage/latest/storage-grpc
5 changes: 1 addition & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,9 @@ int main(int argc, char* argv[]) {
}
std::string const bucket_name = argv[1];

// Create aliases to make the code easier to read.
namespace gcs = ::google::cloud::storage;

// Create a client to communicate with Google Cloud Storage. This client
// uses the default configuration for authentication and project id.
auto client = gcs::Client();
auto client = google::cloud::storage::Client();

auto writer = client.WriteObject(bucket_name, "quickstart.txt");
writer << "Hello World!";
Expand Down
Binary file not shown.
3 changes: 3 additions & 0 deletions ci/cloudbuild/builds/checkers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,9 @@ time {

mapfile -t libraries < <(features::libraries)
for library in "${libraries[@]}" opentelemetry; do
if [[ "${library}" == "storage_grpc" ]]; then
continue
fi
ci/generate-markdown/update-library-readme.sh "${library}"
done
}
Expand Down
2 changes: 1 addition & 1 deletion ci/cloudbuild/builds/cmake-install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ cmake -G Ninja \
-S "${PROJECT_ROOT}/ci/verify_quickstart" \
-B "${PROJECT_ROOT}/cmake-out/quickstart" \
"-DCMAKE_PREFIX_PATH=${INSTALL_PREFIX}" \
"-DFEATURES=experimental-storage_grpc"
"-DFEATURES=storage_grpc"
cmake --build "${PROJECT_ROOT}/cmake-out/quickstart"

io::log_h2 "Delete installed artifacts and run compiled quickstarts"
Expand Down
6 changes: 3 additions & 3 deletions ci/cloudbuild/builds/cmake-split-install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ io::log_h2 "Building and installing popular libraries"
mapfile -t core_cmake_args < <(cmake::common_args cmake-out/popular-libraries)
io::run cmake "${core_cmake_args[@]}" "${install_args[@]}" \
-DCMAKE_PREFIX_PATH="${INSTALL_PREFIX}" \
-DGOOGLE_CLOUD_CPP_ENABLE="bigtable,pubsub,spanner,storage,iam,policytroubleshooter" \
-DGOOGLE_CLOUD_CPP_ENABLE="bigtable,pubsub,spanner,storage,storage_grpc,iam,policytroubleshooter" \
-DGOOGLE_CLOUD_CPP_USE_INSTALLED_COMMON=ON
io::run cmake --build cmake-out/popular-libraries
io::run cmake --install cmake-out/popular-libraries --prefix "${INSTALL_PREFIX}"
Expand All @@ -71,13 +71,13 @@ mapfile -t feature_cmake_args < <(cmake::common_args cmake-out/features)
io::run cmake "${feature_cmake_args[@]}" "${install_args[@]}" \
-DCMAKE_PREFIX_PATH="${INSTALL_PREFIX}" \
-DGOOGLE_CLOUD_CPP_USE_INSTALLED_COMMON=ON \
-DGOOGLE_CLOUD_CPP_ENABLE="__ga_libraries__,-bigtable,-pubsub,-storage,-spanner,-iam,-policytroubleshooter,-compute"
-DGOOGLE_CLOUD_CPP_ENABLE="__ga_libraries__,-bigtable,-pubsub,-storage,-storage_grpc,-spanner,-iam,-policytroubleshooter,-compute"
io::run cmake --build cmake-out/features
io::run cmake --install cmake-out/features --prefix "${INSTALL_PREFIX}"

# Tests the installed artifacts by building all the quickstarts.
# shellcheck disable=SC2046
mapfile -t feature_list < <(cmake -P cmake/print-ga-features.cmake 2>&1)
mapfile -t feature_list < <(cmake -P cmake/print-ga-features.cmake 2>&1 | grep -v storage_grpc)
FEATURES=$(printf ";%s" "${feature_list[@]}")
FEATURES="${FEATURES:1}"
io::run cmake -G Ninja \
Expand Down
3 changes: 1 addition & 2 deletions ci/cloudbuild/builds/lib/features.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function features::always_build() {
# library instrumentation, and the GCP exporters.
opentelemetry
# Enable storage_grpc in most builds.
experimental-storage_grpc
storage_grpc
)
printf "%s\n" "${list[@]}" | sort -u
}
Expand All @@ -62,7 +62,6 @@ function features::libraries() {
function features::_internal_extra() {
local list=(
experimental-bigquery_rest
experimental-storage_grpc
opentelemetry
)
printf "%s\n" "${list[@]}"
Expand Down
4 changes: 2 additions & 2 deletions ci/cloudbuild/builds/lib/quickstart.sh
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,12 @@ function quickstart::run_gcs_grpc_quickstart() {
io::log_h2 "Running quickstart for GCS+gRPC"

io::log "[ CMake ]"
local cmake_bin_dir="${PROJECT_ROOT}/cmake-out/quickstart/cmake-storage"
local cmake_bin_dir="${PROJECT_ROOT}/cmake-out/quickstart/cmake-storage_grpc"
"${cmake_bin_dir}/quickstart_grpc" "${run_args[@]}"

echo
io::log "[ Make ]"
local makefile_bin_dir="${PROJECT_ROOT}/cmake-out/quickstart/makefile-storage"
local makefile_bin_dir="${PROJECT_ROOT}/cmake-out/quickstart/makefile-storage_grpc"
LD_LIBRARY_PATH="${prefix}/lib64:${prefix}/lib:${LD_LIBRARY_PATH:-}" \
"${makefile_bin_dir}/quickstart_grpc" "${run_args[@]}"
}
Expand Down
2 changes: 1 addition & 1 deletion ci/etc/cloudcxxrc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
ALWAYS_RESET_CMAKE_CACHE=NO

# By default compile most features.
ENABLED_FEATURES=__ga_libraries__,__experimental_libraries__,opentelemetry,experimental-storage_grpc
ENABLED_FEATURES=__ga_libraries__,__experimental_libraries__,opentelemetry

# By default compile all the code.
BAZEL_TARGETS=("...")
Expand Down
2 changes: 1 addition & 1 deletion ci/generate-markdown/generate-readme.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ file="README.md"
) | sponge "${file}"

(
mapfile -t features < <(cmake -P cmake/print-ga-features.cmake 2>&1 | LC_ALL=C sort)
mapfile -t features < <(cmake -P cmake/print-ga-features.cmake 2>&1 | LC_ALL=C sort | grep -v storage_grpc)
sed '/<!-- inject-GA-features-start -->/q' "${file}"
for feature in "${features[@]}"; do
if [[ "${feature}" == "oauth2" ]]; then
Expand Down
2 changes: 1 addition & 1 deletion ci/lib/shard.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ readonly PUBSUB_SHARD=(
readonly STORAGE_SHARD=(
storage
storagecontrol
experimental-storage_grpc
storage_grpc
# oauth2 and universe_domain are included because their deps are already built
# by storage.
oauth2
Expand Down
3 changes: 2 additions & 1 deletion ci/verify_current_targets/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ CURRENT_TARGETS = [
":pubsub",
":spanner",
":storage",
":experimental-storage_grpc",
":storage_grpc",
":storage_grpc_mocks",
]

[cc_test(
Expand Down
6 changes: 4 additions & 2 deletions ci/verify_current_targets/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,14 @@ set(ga_libraries
pubsub_mocks
spanner
spanner_mocks
storage)
storage
storage_grpc
storage_grpc_mocks)

# Define experimental libraries. The only difference is the name of the CMake
# targets.
set(experimental_libraries # cmake-format: sort
bigquery_rest storage_grpc storage_grpc_mocks)
bigquery_rest)

# CMake can use pkg-config to find dependencies. We recommend using CMake
# targets, but we want to verify our pkg-config files remain usable and
Expand Down
6 changes: 3 additions & 3 deletions ci/verify_quickstart/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function (add_cmake_quickstart_target feature library target)
PREFIX "${PROJECT_BINARY_DIR}/cmake-${library}-prefix"
SOURCE_DIR
"${PROJECT_SOURCE_DIR}/../../google/cloud/${library}/quickstart"
BINARY_DIR "${PROJECT_BINARY_DIR}/cmake-${library}"
BINARY_DIR "${PROJECT_BINARY_DIR}/cmake-${feature}"
Copy link
Member

Choose a reason for hiding this comment

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

this is a good change, thanks. We might be able to consolidate the commands that runs the GCS+gRPC quickstart in cmake-install.sh. But that is something we can look at in a future PR.

CMAKE_ARGS ${COMMON_CMAKE_ARGS}
BUILD_COMMAND "${CMAKE_COMMAND}" "--build" "<BINARY_DIR>" "--target"
"${target}"
Expand All @@ -79,7 +79,7 @@ function (add_make_quickstart_target feature library target)
PREFIX "${PROJECT_BINARY_DIR}/makefile-${library}-prefix"
SOURCE_DIR
"${PROJECT_SOURCE_DIR}/../../google/cloud/${library}/quickstart"
BINARY_DIR "${PROJECT_BINARY_DIR}/makefile-${library}"
BINARY_DIR "${PROJECT_BINARY_DIR}/makefile-${feature}"
CONFIGURE_COMMAND ""
BUILD_COMMAND
"env"
Expand All @@ -100,7 +100,7 @@ function (add_make_quickstart_target feature library target)
endfunction ()

foreach (feature IN LISTS FEATURES)
if ("${feature}" STREQUAL "experimental-storage_grpc")
if ("${feature}" STREQUAL "storage_grpc")
add_cmake_quickstart_target("${feature}" storage quickstart_grpc)
add_make_quickstart_target("${feature}" storage quickstart_grpc)
continue()
Expand Down
2 changes: 1 addition & 1 deletion cmake/GoogleCloudCppFeatures.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ set(GOOGLE_CLOUD_CPP_EXPERIMENTAL_LIBRARIES
)

set(GOOGLE_CLOUD_CPP_TRANSITION_LIBRARIES # cmake-format: sort
)
"storage_grpc")
ddelgrosso1 marked this conversation as resolved.
Show resolved Hide resolved

set(GOOGLE_CLOUD_CPP_GA_LIBRARIES
# cmake-format: sort
Expand Down
2 changes: 0 additions & 2 deletions doc/compile-time-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ customers will need to use these. If you have specific questions please start a
[GitHub Discussion]. With that said:

- `generator` enables an internal-only tool to generate new libraries.
- `experimental-storage_grpc` enables the GCS+gRPC plugin. Contact your account
team if you want to use this feature or are interested in the GA timeline.
- `experimental-http-transcoding` enables support for HTTP/1.1 transport (as
opposed to gRPC over HTTP/2) in some libraries.
- `opentelemetry` enables support for [OpenTelemetry].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ time env -C google-cloud-cpp ci/cloudbuild/build.sh -t checkers-pr

```sh
(
echo ENABLED_FEATURES=storage,experimental-storage_grpc,opentelemetry
echo ENABLED_FEATURES=storage,storage_grpc,opentelemetry
echo RUN_CLANG_TIDY_FIX=NO
) >$HOME/.cloudcxxrc
time env -C google-cloud-cpp ci/cloudbuild/build.sh --distro fedora-latest-cmake --build development
Expand Down
2 changes: 1 addition & 1 deletion google/cloud/grpc_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ struct GrpcCompressionAlgorithmOption {
* - `pubsub::MakePublisherConnection()`
* - `pubsub::MakeSubscriberConnection()`
* - `spanner::MakeConnection()`
* - `storage_experimental::DefaultGrpcClient()`
* - `storage::MakeGrpcClient()`
*
* @ingroup options
*/
Expand Down
4 changes: 2 additions & 2 deletions google/cloud/storage/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ endif ()

add_executable(storage_quickstart_grpc "quickstart/quickstart_grpc.cc")
target_link_libraries(storage_quickstart_grpc
PRIVATE google-cloud-cpp::experimental-storage_grpc)
PRIVATE google-cloud-cpp::storage_grpc)
google_cloud_cpp_add_common_options(storage_quickstart_grpc)
add_test(
NAME storage_quickstart_grpc
Expand All @@ -79,7 +79,7 @@ set_tests_properties(storage_quickstart_grpc

add_executable(storage_quickstart_async "quickstart/quickstart_async.cc")
target_link_libraries(storage_quickstart_async
PRIVATE google-cloud-cpp::experimental-storage_grpc)
PRIVATE google-cloud-cpp::storage_grpc)
google_cloud_cpp_add_common_options(storage_quickstart_async)
add_test(
NAME storage_quickstart_async
Expand Down
5 changes: 1 addition & 4 deletions google/cloud/storage/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,9 @@ int main(int argc, char* argv[]) {
}
std::string const bucket_name = argv[1];

// Create aliases to make the code easier to read.
namespace gcs = ::google::cloud::storage;

// Create a client to communicate with Google Cloud Storage. This client
// uses the default configuration for authentication and project id.
auto client = gcs::Client();
auto client = google::cloud::storage::Client();

auto writer = client.WriteObject(bucket_name, "quickstart.txt");
writer << "Hello World!";
Expand Down
6 changes: 6 additions & 0 deletions google/cloud/storage/async/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@

namespace google {
namespace cloud {
/**
* Contains experimental features for the GCS C++ Client Library.
*
* @warning The types, functions, aliases, and objects in this namespace are
* subject to change without notice.
*/
namespace storage_experimental {
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN

Expand Down
2 changes: 1 addition & 1 deletion google/cloud/storage/benchmarks/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ cc_binary(
tags = ["manual"],
deps = [
":storage_benchmarks",
"//:experimental-storage_grpc",
"//:storage_grpc",
"//google/cloud/testing_util:google_cloud_cpp_testing_private",
],
)
7 changes: 3 additions & 4 deletions google/cloud/storage/benchmarks/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ target_link_libraries(
storage_benchmarks
PUBLIC storage_client_testing
google_cloud_cpp_testing
google-cloud-cpp::experimental-storage_grpc
google-cloud-cpp::storage_grpc
google-cloud-cpp::storage
google-cloud-cpp::grpc_utils
google-cloud-cpp::storage_protos
Expand Down Expand Up @@ -136,6 +136,5 @@ target_compile_features(async_throughput_benchmark PRIVATE cxx_std_20)
# `google/cloud/*`. We cannot apply those recommendations because the headers
# are also used with C++14.
set_target_properties(async_throughput_benchmark PROPERTIES CXX_CLANG_TIDY "")
target_link_libraries(
async_throughput_benchmark
PRIVATE storage_benchmarks google-cloud-cpp::experimental-storage_grpc)
target_link_libraries(async_throughput_benchmark
PRIVATE storage_benchmarks google-cloud-cpp::storage_grpc)
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,7 @@ class Iteration {
gcs::Client MakeClient(AggregateDownloadThroughputOptions const& options) {
auto opts = options.client_options;
#if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
namespace gcs_ex = ::google::cloud::storage_experimental;
if (options.api == "GRPC") {
return gcs_ex::DefaultGrpcClient(std::move(opts));
}
if (options.api == "GRPC") return gcs::MakeGrpcClient();
#endif // GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
return gcs::Client(std::move(opts));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,8 @@ gcs::Client MakeClient(AggregateUploadThroughputOptions const& options) {
// on almost all `.write()` requests.
.set<gcs::UploadBufferSizeOption>(256 * gcs_bm::kKiB);
#if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
namespace gcs_ex = ::google::cloud::storage_experimental;
if (options.api == "GRPC") {
return gcs_ex::DefaultGrpcClient(std::move(opts));
return gcs::MakeGrpcClient(std::move(opts));
}
#endif // GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
return gcs::Client(std::move(opts));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ auto MakeAsyncClients(Configuration const& cfg,

auto MakeClient(ClientConfig const& cc, int background_threads) {
if (cc.transport == "GRPC") {
return google::cloud::storage_experimental::DefaultGrpcClient(
return gcs::MakeGrpcClient(
ddelgrosso1 marked this conversation as resolved.
Show resolved Hide resolved
g::Options{}
.set<g::GrpcBackgroundThreadPoolSizeOption>(background_threads)
.set<g::EndpointOption>(MapPath(cc.path)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,16 +267,15 @@ gcs_bm::ClientProvider BaseProvider(ThroughputOptions const& options) {
return [=](ExperimentTransport t) {
auto opts = options.client_options;
#if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
namespace gcs_ex = ::google::cloud::storage_experimental;
if (t == ExperimentTransport::kDirectPath) {
opts = google::cloud::internal::MergeOptions(options.direct_path_options,
std::move(opts));
return gcs_ex::DefaultGrpcClient(std::move(opts));
return gcs::MakeGrpcClient(std::move(opts));
}
if (t == ExperimentTransport::kGrpc) {
opts = google::cloud::internal::MergeOptions(options.grpc_options,
std::move(opts));
return gcs_ex::DefaultGrpcClient(std::move(opts));
return gcs::MakeGrpcClient(std::move(opts));
}
#else
(void)t; // disable unused parameter warning
Expand Down
Loading
Loading