[fix](scan) Respect byte budget when merging scan blocks#63296
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
There was a problem hiding this comment.
Reviewed the full PR diff and existing review context. No existing inline comments were present, and there was no additional user focus.
Critical checkpoints: correctness has a blocking issue because disabled adaptive batch sizing now still enforces a 512MB merge cap; compatibility/regression risk is therefore not acceptable until fixed. I did not identify concurrency, memory-accounting, security, or API-surface issues beyond this behavior regression. Tests/build were not run by me in this review environment; the PR description reports object compilation only.
|
|
||
| const auto free_block_data_bytes = free_block->bytes(); | ||
| const bool within_byte_budget = preferred_block_size_bytes == 0 || | ||
| last_block->bytes() + free_block_data_bytes <= |
There was a problem hiding this comment.
This treats preferred_block_size_bytes == 0 as the disabled case, but RuntimeState::preferred_block_size_bytes() never returns 0 when adaptive batch sizing is disabled; it returns kMax (512MB) and the accessor comment says callers that need the active/disabled state must check config::enable_adaptive_batch_size explicitly. As a result, with enable_adaptive_batch_size=false, this PR still stops merging once the combined data bytes exceed 512MB, whereas the old behavior was row-budget-only. That is a behavior regression for disabled adaptive mode and can produce extra cached blocks / earlier scan-task yielding for wide rows. Please gate the byte-budget check on config::enable_adaptive_batch_size (or otherwise pass a disabled byte budget) instead of relying on 0 here.
Summary
Fix scanner scheduler block merging so the adaptive batch size byte budget is respected when multiple scanned blocks are stitched into a cached block.
Root Cause
The scheduler merge path only checked the row count against
batch_size(). When adaptive batch size produced multiple blocks that were individually acceptable, the scheduler could still merge them into a much larger block because it ignoredpreferred_block_size_bytes().Changes
preferred_block_size_bytes()for the scan task.allocated_bytes()for memory accounting while usingbytes()for the adaptive data-size budget.Validation
git diff --check -- be/src/exec/scan/scanner_scheduler.cppninja -C be/ut_build_ASAN src/exec/CMakeFiles/Exec.dir/scan/scanner_scheduler.cpp.oNote:
./run-be-ut.sh --run --filter=ScannerContextTest.*was started earlier but stopped after it triggered a broad ASAN UT build; the changed object had already compiled successfully.