wenzhenghu commented on code in PR #64649:
URL: https://github.com/apache/doris/pull/64649#discussion_r3503035841
##########
be/src/runtime/workload_management/workload_sched_policy.cpp:
##########
@@ -80,6 +80,12 @@ bool
WorkloadSchedPolicy::is_match(WorkloadAction::RuntimeContext* action_runtim
val =
std::to_string(action_runtime_ctx->resource_ctx->io_context()->scan_bytes());
break;
}
+ // Evaluate the remote read breaker against the existing IO context
remote scan counter.
+ case WorkloadMetricType::SCAN_BYTES_FROM_REMOTE_STORAGE: {
+ val = std::to_string(action_runtime_ctx->resource_ctx->io_context()
+ ->scan_bytes_from_remote_storage());
Review Comment:
Thanks for catching this. Your concern is valid.
The new workload policy condition itself only reuses the existing
`io_context()->scan_bytes_from_remote_storage()` counter, but after checking
the current BE implementation, I agree that this counter does not always
represent true remote-backed reads correctly.
In `FileScanner::update_realtime_counters()`, when both
`bytes_read_from_local` and `bytes_read_from_remote` are zero, the code falls
back to attributing all `read_bytes` to `scan_bytes_from_remote_storage()`. For
`FILE_LOCAL` paths such as `local()` TVF, the reader goes through
`global_local_filesystem()`, and `LocalFileReader` does not populate those
file-cache counters. As a result, purely local file reads can be observed as
remote scan bytes by this metric.
So the root issue here is the correctness of the existing
`scan_bytes_from_remote_storage` metric, rather than the policy matching logic
itself. I agree this should be fixed before relying on the metric as "remote
read bytes".
I will address the metric correctness separately in a dedicated follow-up
PR, so that `scan_bytes_from_remote_storage` only reflects actual remote-backed
reads. After that, this workload policy condition can safely rely on it.
--
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]
