feat(storage): support pre-warming read ranges in AsyncConnection - #16275
feat(storage): support pre-warming read ranges in AsyncConnection#16275kalragauri wants to merge 3 commits into
Conversation
product-auto-label
Bot
added
the
api: storage
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for pre-warmed ranges in the asynchronous GCS client, allowing specified ranges to be downloaded as soon as a stream opens. It adds the ReadRangesOption configuration, updates AsyncConnectionImpl::Open to populate the initial request with these ranges, and modifies ObjectDescriptorImpl to cache and claim them during reads. The feedback suggests critical improvements to the caching mechanism: using std::multimap instead of std::map to prevent resource leaks when duplicate ranges are configured, using emplace for insertion, and utilizing std::move to avoid unnecessary copies of std::shared_ptr when claiming cached ranges.
| // 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>(); |
There was a problem hiding this comment.
This exact same ID generation and deduplication loop is copied in both connection_impl.cc and object_descriptor_impl.cc. If someone updates the logic in one file and forgets the other, the read_ids won't match and data will be sent to the wrong ranges. Should we move this logic into a single shared helper function?
| if (it != stream_manager_->End()) { | ||
| std::int64_t id = 0; | ||
| std::set<std::pair<std::int64_t, std::int64_t>> seen_ranges; | ||
| for (auto const& r : ranges) { |
| // these ranges. | ||
| it->active_ranges.emplace(id, range); | ||
| // Cache them so subsequent `Read()` calls can claim them. | ||
| prewarmed_ranges_.emplace(range_key, PrewarmedRange{range, id}); |
There was a problem hiding this comment.
What would happen if the user configures pre-warmed ranges but their application never actually calls Read() to claim them?
2 participants
This PR implements the backend plumbing and happy-path caching to support pre-warming read ranges in the GCS async client.
Key changes:
ReadRangesOptioncontaining a list ofReadRangeConfig(offset and length) to configure which ranges to pre-warm.AsyncConnectionImpl::Opento populate the initialBidiReadObjectRequestwith the configuredread_ranges.ObjectDescriptorImplconstructor to initializeReadRangeobjects for pre-warmed ranges and register them on the initial stream so incoming GCS data chunks are routed correctly and not discarded.prewarmed_ranges_cache inObjectDescriptorImpl. When the user callsRead(), the cache is checked. On a hit, the pre-warmedReadRangeis claimed and returned as anObjectDescriptorReader.