cherry pick 60989:[opt](query cache) Support multiple tablets in cache key building#61527
cherry pick 60989:[opt](query cache) Support multiple tablets in cache key building#61527yiguolei merged 2 commits intoapache:branch-4.1from
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
There was a problem hiding this comment.
Pull request overview
This PR cherry-picks changes to extend BE query-cache key construction to support fragment instances that scan multiple tablets, and updates BE unit tests accordingly.
Changes:
- Update
QueryCache::build_cache_key()to accept multiple scan ranges and incorporate multiple tablet IDs into the cache key. - Remove the “only one scan range” restriction in
CacheSourceLocalState::init()and record multiple tablet IDs in the runtime profile. - Adjust/extend BE unit tests to set
digestand validate multi-tablet cache-key behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| be/test/pipeline/operator/query_cache_operator_test.cpp | Updates operator tests to use Status::ok() and set digest for cache params. |
| be/test/pipeline/exec/query_cache_test.cpp | Expands cache-key tests to cover multiple tablets and additional error paths. |
| be/src/pipeline/query_cache/query_cache.h | Implements multi-tablet cache-key building logic (sorting tablet IDs, validating version/range assumptions). |
| be/src/pipeline/exec/cache_source_operator.cpp | Removes single-scan-range restriction and records multiple tablet IDs in the profile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| if (current_version != first_version) { | ||
| return Status::InternalError( | ||
| "All tablets in one instance must have the same version, plan error"); | ||
| } | ||
| if (find_tablet->second != first_tablet_range) { | ||
| return Status::InternalError( | ||
| "All tablets in one instance must have the same tablet_to_range, plan " | ||
| "error"); | ||
| } |
| std::string digest; | ||
| try { | ||
| digest = cache_param.digest; | ||
| } catch (const std::exception&) { | ||
| return Status::InternalError("digest is invalid, plan error"); | ||
| } |
| std::vector<int64_t> tablet_ids; | ||
| tablet_ids.reserve(scan_ranges.size()); | ||
| for (const auto& scan_range : scan_ranges) { | ||
| auto tablet_id = scan_range.scan_range.palo_scan_range.tablet_id; | ||
| tablet_ids.push_back(tablet_id); | ||
| } |
| int64_t current_version = -1; | ||
| std::from_chars(scan_range_iter->scan_range.palo_scan_range.version.data(), | ||
| scan_range_iter->scan_range.palo_scan_range.version.data() + | ||
| scan_range_iter->scan_range.palo_scan_range.version.size(), | ||
| current_version); | ||
|
|
| std::vector<int64_t> tablet_ids; | ||
| tablet_ids.reserve(scan_ranges.size()); | ||
| for (const auto& scan_range : scan_ranges) { | ||
| auto tablet_id = scan_range.scan_range.palo_scan_range.tablet_id; | ||
| tablet_ids.push_back(tablet_id); | ||
| } | ||
| std::sort(tablet_ids.begin(), tablet_ids.end()); | ||
|
|
||
| int64_t first_version = -1; | ||
| std::string first_tablet_range; | ||
| for (size_t i = 0; i < tablet_ids.size(); ++i) { | ||
| auto tablet_id = tablet_ids[i]; | ||
|
|
||
| auto find_tablet = cache_param.tablet_to_range.find(tablet_id); | ||
| if (find_tablet == cache_param.tablet_to_range.end()) { | ||
| return Status::InternalError("Not find tablet in partition_to_tablets, plan error"); | ||
| } | ||
|
|
||
| auto scan_range_iter = | ||
| std::find_if(scan_ranges.begin(), scan_ranges.end(), | ||
| [&tablet_id](const TScanRangeParams& range) { | ||
| return range.scan_range.palo_scan_range.tablet_id == tablet_id; | ||
| }); |
|
|
||
| auto find_tablet = cache_param.tablet_to_range.find(tablet_id); | ||
| if (find_tablet == cache_param.tablet_to_range.end()) { | ||
| return Status::InternalError("Not find tablet in partition_to_tablets, plan error"); |
| cache_param.digest = "test_digest"; | ||
| cache_param.output_slot_mapping[0] = 0; | ||
| cache_param.tablet_to_range.insert({42, "test"}); | ||
| cache_param.force_refresh_query_cache = false; |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)