github-actions[bot] commented on code in PR #63537:
URL: https://github.com/apache/doris/pull/63537#discussion_r3301711479
##########
be/src/runtime/workload_group/workload_group_metrics.h:
##########
@@ -73,6 +73,7 @@ class WorkloadGroupMetrics {
std::atomic<uint64_t> _cpu_time_nanos {0};
std::atomic<uint64_t> _last_cpu_time_nanos {0};
+ std::atomic<uint64_t> _last_refresh_time_ms {0};
std::atomic<uint64_t> _per_sec_cpu_time_nanos {0}; // used for system table
Review Comment:
Initializing this to 0 makes the first `refresh_metrics()` use
`MonotonicMillis() / 1000`, i.e. host uptime, as the elapsed interval. A newly
created workload group can execute queries before the first daemon refresh;
those CPU/IO deltas are then divided by uptime and `_last_*` counters are
advanced, so the first interval's utilization is effectively lost. Please seed
this timestamp at construction time or make the first refresh initialize the
timestamp without consuming the current deltas.
##########
be/src/runtime/workload_group/workload_group_metrics.cpp:
##########
@@ -83,7 +84,13 @@ void
WorkloadGroupMetrics::update_remote_scan_io_bytes(uint64_t delta_io_bytes)
}
void WorkloadGroupMetrics::refresh_metrics() {
- int interval_second = config::workload_group_metrics_interval_ms / 1000;
+ uint64_t current_time_ms = MonotonicMillis();
+ uint64_t interval_second = (current_time_ms - _last_refresh_time_ms) /
1000;
+ _last_refresh_time_ms = current_time_ms;
Review Comment:
This still does not use the actual elapsed time because the millisecond
delta is truncated to whole seconds before the rate division. For example, a
refresh after 1.9s uses `interval_second == 1` and reports rates about 90%
high, and a delayed default 5.9s cycle divides by 5s instead of 5.9s. It also
means a sub-second configured interval can keep returning without ever
accumulating enough elapsed time because `_last_refresh_time_ms` is advanced
before the guard. Please compute from the millisecond delta directly, for
example `delta * 1000 / elapsed_ms`, and only skip when the elapsed millisecond
delta is 0.
--
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]
