github-actions[bot] commented on code in PR #69911:
URL: https://github.com/apache/doris/pull/69911#discussion_r4435593282


##########
be/src/exec/scan/scanner_scheduler.cpp:
##########
@@ -279,7 +279,7 @@ void 
ScannerScheduler::_scanner_scan(std::shared_ptr<ScannerContext> ctx,
                     // Or it may cause a crash when the block is not normal.
                     _make_sure_virtual_col_is_materialized(scanner, 
free_block.get());
 
-                    // Projection will truncate useless columns, makes block 
size change.

Review Comment:
   [P1] Include consumer projection memory in adaptive scan admission. The 
scheduler now feeds only raw `free_block->allocated_bytes()` into `MemLimiter`, 
while projection/intermediate columns are allocated later on the pipeline 
thread and are tracked only by `_estimate_memory_usage`. A wide or expanding 
projection can therefore make `available_scanner_count()` admit scanners based 
on a small raw estimate and oversubscribe the adaptive scan memory budget. Feed 
a bounded projection component into the same estimate or coordinate admission 
with the pipeline projection budget.



##########
be/src/exec/operator/scan_operator.cpp:
##########
@@ -1346,29 +1347,25 @@ template <typename LocalStateType>
 size_t ScanOperatorX<LocalStateType>::get_reserve_mem_size(RuntimeState* 
state) {
     auto& local_state = get_local_state(state);
     auto ctx = local_state._scanner_ctx.load();
-    if (!local_state._opened || local_state._closed || !ctx) {
-        return config::doris_scanner_row_bytes;
-    }
-
-    if (local_state.low_memory_mode()) {
-        return ctx->low_memory_mode_scan_bytes_per_scanner() * 
ctx->low_memory_mode_scanners();
-    } else {
-        const auto peak_usage = local_state._memory_used_counter->value();
-        const auto block_usage = ctx->block_memory_usage();
-        if (peak_usage > 0) {
-            // It is only a safty check, to avoid some counter not right.
+    size_t scan_reserve = config::doris_scanner_row_bytes;
+    if (local_state._opened && !local_state._closed && ctx) {
+        if (local_state.low_memory_mode()) {
+            scan_reserve =
+                    ctx->low_memory_mode_scan_bytes_per_scanner() * 
ctx->low_memory_mode_scanners();
+        } else {
+            const auto peak_usage = local_state._memory_used_counter->value();
+            const auto block_usage = ctx->block_memory_usage();
+            // Queued blocks are already allocated; reserve only the remaining 
operator memory.
             if (peak_usage > block_usage) {
-                return peak_usage - block_usage;
-            } else {
-                return config::doris_scanner_row_bytes;
+                scan_reserve = peak_usage - block_usage;
             }
-        } else {
-            // If the scan operator is first time to run, then we think it 
will occupy doris_scanner_row_bytes.
-            // It maybe a little smaller than actual usage.
-            return config::doris_scanner_row_bytes;
-            // return local_state._scanner_ctx->max_bytes_in_queue();
         }
     }
+
+    // Projection allocates on the pipeline thread and uses the common 
operator estimate.
+    const size_t projection_reserve =

Review Comment:
   [P1] Combine scan and projection reservation requirements. 
`get_block_impl()` dequeues a raw block into `_origin_block`, and 
`do_projections()` keeps that raw block live while allocating 
intermediate/final projection columns. `scan_reserve` and `projection_reserve` 
therefore represent simultaneous memory, but line 1368 returns `std::max(...)`, 
so a 64 MiB scan residual plus 32 MiB projection reserves only 64 MiB and can 
bypass the workload-group revocation budget. Return an overflow-safe sum (or 
otherwise account for the live raw origin explicitly).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to