feat(storage): Cherry pick initial read ranges in AsyncClient support by leedsalim · Pull Request #16537 · googleapis/google-cloud-cpp

leedsalim · 2026-10-05T16:38:52Z

cherry-picked #16341 and its dependencies #16275, #16309 and #16323

Test Summary
All affected unit test and sample targets were built and executed via Bazel:
--//google/cloud/storage:enable_grpc_metrics=true (default) and
--//google/cloud/storage:enable_grpc_metrics=false.

Compilation fixes applied

In google/cloud/storage/internal/async/connection_tracing_test.cc, the test added by feat(storage): add telemetry for pre-warmed ranges in ObjectDescriptorImpl #16323 (ConnectionTracing.OpenSuccessWithInitialReadRanges) referenced OTelAttribute and SpanHasAttributes, whose using ::google::cloud::testing_util::... declarations had been added on main by an earlier unrelated commit not present in this feature branch.
Added using ::google::cloud::testing_util::OTelAttribute; and using ::google::cloud::testing_util::SpanHasAttributes; to google/cloud/storage/internal/async/connection_tracing_test.cc, amended the feat(storage): add telemetry for pre-warmed ranges in ObjectDescriptorImpl #16323 cherry-pick commit and re-applied feat(storage): expose initial read ranges in AsyncClient #16341.

…ogleapis#16275)

…bjectDescriptorImpl (googleapis#16309)

…rImpl (googleapis#16323)

…16341)

google-cla · 2026-10-05T16:39:04Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

gemini-code-assist

Code Review

This pull request introduces support for pre-warmed initial read ranges when opening an object in the asynchronous Google Cloud Storage client. It adds InitialReadRanges to AsyncClient::Open, propagates these ranges down to the connection implementation, and manages their caching, deduplication, and pacing (with a default 5 MiB buffer limit) in ObjectDescriptorImpl. Tracing and unit tests are also updated to support this feature. The review feedback focuses on adhering to the repository's style guide regarding type deduction (auto), specifically recommending the use of explicit types instead of auto when domain objects, enums, or protobuf messages are obscured, and using auto const& to avoid unnecessary copies of containers or std::shared_ptr instances. Additionally, it suggests moving a std::shared_ptr in ObjectDescriptorImpl to prevent atomic reference count overhead.

gemini-code-assist · 2026-10-05T16:42:38Z

+  // If pre-warmed ranges are configured, populate the initial request
+  // with these ranges to start downloading them as soon as the stream opens.
+  if (current->has<ReadRangesOption>()) {
+    auto const& ranges = current->get<ReadRangesOption>();


When a function returns a reference to a container or a large object, use auto const& instead of auto to avoid unnecessary and inefficient copies.

auto const& ranges = current->get<ReadRangesOption>();

References

When a function returns a reference to a container or a large object (such as std::multimap), use 'auto const&' instead of 'auto' to avoid unnecessary and inefficient copies.

gemini-code-assist · 2026-10-05T16:42:38Z

+  // with these ranges to start downloading them as soon as the stream opens.
+  if (current->has<ReadRangesOption>()) {
+    auto const& ranges = current->get<ReadRangesOption>();
+    for (auto const& r : DeduplicateRanges(ranges)) {


Avoid using auto when it hides domain objects. Explicitly declare the loop variable type.

for (DedupedReadRange const& r : DeduplicateRanges(ranges)) {

References

Reject auto when it hides domain objects or function return types. ^(link)

gemini-code-assist · 2026-10-05T16:42:38Z

+  if (current->has<ReadRangesOption>()) {
+    auto const& ranges = current->get<ReadRangesOption>();
+    for (auto const& r : DeduplicateRanges(ranges)) {
+      auto* proto_range = initial_request.add_read_ranges();


Avoid using auto when it hides protobuf messages or fields. Use the explicit protobuf type instead.

google::storage::v2::ReadRange* proto_range = initial_request.add_read_ranges();

References

Reject auto when it hides protobuf messages/fields. ^(link)

gemini-code-assist · 2026-10-05T16:42:38Z

      OpenParams p) override {
    auto span = internal::MakeSpan("storage::AsyncConnection::Open");
+    if (p.options.has<ReadRangesOption>()) {
+      auto const& ranges = p.options.get<ReadRangesOption>();


When a function returns a reference to a container or a large object, use auto const& instead of auto to avoid unnecessary and inefficient copies.

auto const& ranges = p.options.get<ReadRangesOption>();

References

When a function returns a reference to a container or a large object (such as std::multimap), use 'auto const&' instead of 'auto' to avoid unnecessary and inefficient copies.

gemini-code-assist · 2026-10-05T16:42:38Z

+  // If pre-warmed ranges are specified, initialize their `ReadRange` objects,
+  // register them as active on the initial stream, and cache them.
+  if (options_.has<ReadRangesOption>()) {
+    auto const& ranges = options_.get<ReadRangesOption>();


When a function returns a reference to a container or a large object, use auto const& instead of auto to avoid unnecessary and inefficient copies.

auto const& ranges = options_.get<ReadRangesOption>();

References

When a function returns a reference to a container or a large object (such as std::multimap), use 'auto const&' instead of 'auto' to avoid unnecessary and inefficient copies.

gemini-code-assist · 2026-10-05T16:42:39Z

+        it->active_ranges.emplace(dr.read_id, range);
+        // Cache them so subsequent `Read()` calls can claim them.
+        auto [cache_it, inserted] = prewarmed_ranges_.emplace(
+            range_key, PrewarmedRange{range, dr.read_id});


Since range is not used after this point in the loop, we can move it into PrewarmedRange to avoid an unnecessary copy (atomic reference count increment/decrement) of the std::shared_ptr.

range_key, PrewarmedRange{std::move(range), dr.read_id});

References

Scrutinize copies of non-fundamental C++ types. Is it necessary to copy the data? Can we move the data instead? ^(link)

gemini-code-assist · 2026-10-05T16:42:39Z

+  auto cache_status = has_initial_read_ranges_
+                          ? InitialReadRangesCacheStatus::kMiss
+                          : InitialReadRangesCacheStatus::kNone;


Avoid using auto when it hides domain objects or enums. Use explicit types instead.

InitialReadRangesCacheStatus cache_status = has_initial_read_ranges_ ? InitialReadRangesCacheStatus::kMiss : InitialReadRangesCacheStatus::kNone;

References

Reject auto when it hides domain objects or function return types. ^(link)

gemini-code-assist · 2026-10-05T16:42:39Z

+  if (cache_it != prewarmed_ranges_.end()) {
+    cache_status = InitialReadRangesCacheStatus::kHit;
+    // Cache hit. Claim the pre-warmed range and return it to the user.
+    auto prewarmed = std::move(cache_it->second);


Avoid using auto when it hides domain objects. Use explicit types instead.

PrewarmedRange prewarmed = std::move(cache_it->second);

References

Reject auto when it hides domain objects or function return types. ^(link)

gemini-code-assist · 2026-10-05T16:42:39Z

+    return false;
+  };
+
  for (auto& range_data : *response->mutable_object_data_ranges()) {


Avoid using auto when it hides protobuf messages or fields. Use the explicit protobuf type instead.

for (google::storage::v2::ObjectRangeData& range_data : *response->mutable_object_data_ranges()) {

References

Reject auto when it hides protobuf messages/fields. ^(link)

gemini-code-assist · 2026-10-05T16:42:39Z

    auto const l = copy.find(id);
    if (l == copy.end()) continue;

+    auto range = l->second;


Copying a std::shared_ptr increments the reference count, which is an atomic operation and has overhead. Since range is not modified or moved, use auto const& to avoid the copy.

auto const& range = l->second;

References

Scrutinize copies of non-fundamental C++ types. Is it necessary to copy the data? Can we move the data instead? ^(link)

When a function returns a reference to a container or a large object (such as std::multimap), use 'auto const&' instead of 'auto' to avoid unnecessary and inefficient copies.

leedsalim · 2026-10-12T18:30:32Z

PTAL @v-pratap , @kalragauri - I don't have permissions to add reviewers yet.

I'll register my enterprise account to be able to do this in the future.

kalragauri

optional: this cherry-pick stack omits the follow-up #16357 from main, which added unit and integration tests.

Consider cherry-picking #16357 as well and adding a test verifying that pre-warmed ranges emit range-read latency telemetry on this branch.

kalragauri · 2026-10-22T06:38:12Z

+
+}  // namespace
+
 ObjectDescriptorImpl::ObjectDescriptorImpl(


Pre-warmed ReadRange instances never pass through Flush(), so t4_ remains default-initialized. Because ReaderConnectionTelemetry::RecordRead() on feat/storage-experimental-metrics exits early when t4 is unset, range-read latency metrics and trace events are silently dropped for all pre-warmed ranges.

We probably need to add a call to SetT4(std::chrono::steady_clock::now()) (under the OpenTelemetry preprocessor guard) when initializing each pre-warmed range.

google-cloud-cpp/google/cloud/storage/internal/async/reader_connection_telemetry.cc

Lines 93 to 96 in ad2eaf2

if (t4 == std::chrono::steady_clock::time_point{} || t4 > t5 || t5 > t6 ||

t6 > t7) {

return;

}

kalragauri · 2026-10-22T06:42:56Z

 MakeTracingObjectDescriptorReader(std::shared_ptr<ReadRange> impl,
-                                  std::string bucket_name);
+                                  std::string bucket_name,
+                                  absl::string_view cache_status = "");


On main (#16323), cache_status does not have a default argument. All callers already pass cache_status explicitly. Consider removing the = "" default value to match main.

google-cloud-cpp/google/cloud/storage/internal/async/object_descriptor_reader_tracing.h

Line 29 in 6831cf3

std::shared_ptr<ReadRange> impl, absl::string_view cache_status,

kalragauri added 4 commits October 2, 2026 18:33

feat(storage): support pre-warming read ranges in AsyncConnection (go…

f47af17

…ogleapis#16275)

feat(storage): implement pacing & eviction for pre-warmed ranges in O…

d9c09be

…bjectDescriptorImpl (googleapis#16309)

feat(storage): add telemetry for pre-warmed ranges in ObjectDescripto…

75869a7

…rImpl (googleapis#16323)

feat(storage): expose initial read ranges in AsyncClient (googleapis#…

6b3b430

…16341)

leedsalim requested review from a team as code owners October 5, 2026 16:38

leedsalim deployed to false October 5, 2026 16:38 — with GitHub Actions Active

gemini-code-assist Bot reviewed Oct 5, 2026

View reviewed changes

leedsalim changed the title ~~Cherry pick initial read ranges in AsyncClient support~~ feat(storage): Cherry pick initial read ranges in AsyncClient support Oct 12, 2026

kalragauri self-requested a review October 22, 2026 03:46

kalragauri approved these changes Oct 22, 2026

View reviewed changes

	if (t4 == std::chrono::steady_clock::time_point{} \|\| t4 > t5 \|\| t5 > t6 \|\|
	t6 > t7) {
	return;
	}

Conversation

leedsalim commented Oct 5, 2026

Uh oh!

google-cla Bot commented Oct 5, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

leedsalim commented Oct 12, 2026

Uh oh!

kalragauri left a comment

Choose a reason for hiding this comment

Uh oh!

kalragauri Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

kalragauri Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants