[fix](materialization) Make merge_multi_response detect insufficient backend rows and return error instead of DCHECK#60970
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
a430486 to
1559eef
Compare
|
run buildall |
There was a problem hiding this comment.
Pull request overview
This PR improves runtime robustness by converting a debug-only DCHECK into a real error in MaterializationSharedState::merge_multi_response, and also expands query-cache related logic to support multiple scan ranges/tablets while adjusting FE shuffle parallelism behavior when query cache is enabled.
Changes:
- BE: Replace a
DCHECKinmerge_multi_response()with an explicit bounds check andInternalErrorto avoid out-of-bounds reads in release builds. - BE: Rework query-cache key building to handle multiple scan ranges/tablets, and update the cache source operator + tests accordingly.
- FE: Add logic to cap
UnassignedShuffleJobinstance count when query cache is enabled, with new unit tests.
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/distribute/worker/job/UnassignedShuffleJob.java |
Changes parallelism computation to apply a query-cache-based instance cap. |
fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/distribute/worker/job/UnassignedShuffleJobTest.java |
Adds unit tests covering the new degree-of-parallelism and instance-limiting behavior. |
be/src/pipeline/query_cache/query_cache.h |
Extends QueryCache::build_cache_key to support multiple tablets and adds additional validations. |
be/src/pipeline/exec/cache_source_operator.cpp |
Removes the single-scan-range restriction and updates profiling output for multiple tablets. |
be/test/pipeline/exec/query_cache_test.cpp |
Updates/adds tests for multi-tablet cache key behavior and error cases. |
be/src/util/bitmap_value.h |
Switches BITMAP fast-union implementation to use Roaring64Map::fastunion. |
be/src/pipeline/exec/materialization_opertor.cpp |
Converts a DCHECK into a production error when backend blocks have insufficient rows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TPC-H: Total hot run time: 28908 ms |
TPC-DS: Total hot run time: 184578 ms |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
…backend rows and return error instead of DCHECK
1559eef to
8d0836b
Compare
|
run buildall |
TPC-H: Total hot run time: 28769 ms |
TPC-DS: Total hot run time: 183614 ms |
BE 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)