bthread_worker_usage这个统计值为何有时候会超时bthread

[zc08291161]

Describe the bug (描述bug)
在brpc的统计页面上，很容易就发现bthread_worker_usage的值
超过bthread_worker_count,理论上根据代码，bthread_worker_usage
的值不会超过TaskGroup的个数，因为代码
double TaskControl::get_cumulated_worker_time() {
int64_t cputime_ns = 0;
BAIDU_SCOPED_LOCK(_modify_group_mutex);
const size_t ngroup = _ngroup.load(butil::memory_order_relaxed);
for (size_t i = 0; i < ngroup; ++i) {
if (_groups[i]) {
cputime_ns += _groups[i]->_cumulated_cputime_ns;
}
}
return cputime_ns / 1000000000.0;
}

这个函数是一秒钟统计一次，如果定时器完全准的话，一定不会超过ngroup的个数
因此这个值是否强依赖定时器，因而不一定完全准确，是否应该将
cputime_ns / 1000000000.0;
更改为 cputime_ns / （real_elapse_time）;其中real_elapse_time是这个函数两次
调用的时间差。

To Reproduce (复现方法)

Expected behavior (期望行为)

Versions (各种版本)
OS:
Compiler:
brpc: 1.0.0
protobuf:

Additional context/screenshots (更多上下文/截图)

[yanglimingcn]

听上去这么修改更合理些，你能否简单改一下验证一下呢？

[zc08291161]

ok，I will try

[yanglimingcn]

ok，I will try

这个有PR了吗？

[lintianzhi]

不仅仅是时间的误差
重计算的线程在计算的时候不会增加cputime_ns，但是结束的时候跨秒会把前面用掉的时间都加到当前秒的cputime_ns上，会导致超过很多

[gitccl]

I agree that accurately computing bthread_worker_usage within the bthread framework is quite challenging, especially since some tasks may run for extended periods without yielding. Rather than striving for exact precision, we could use a bounded estimation to ensure that bthread_worker_usage not exceeds bthread_worker_count.

Here’s a possible solution:

• For each TaskGroup, add:

  • _cur_cumulated_worker_time_ns: stores the cumulative worker time as observed by bvar.
  • _last_cumulated_cputime_ns: stores the value of TaskGroup::_cumulated_cputime_ns at the time of the last observation.

• For TaskControl, add:

  • _last_observation_time: records the timestamp of the last observation.

Updated get_cumulated_worker_time() Implementation：

double TaskControl::get_cumulated_worker_time() {
    int64_t cputime_ns = 0;
    BAIDU_SCOPED_LOCK(_modify_group_mutex);
    int64_t now = butil::cpuwide_time_ns();
    for_each_task_group([&](TaskGroup* g) {
        if (g) {
            int64_t cur_cumulated_cputime_ns = g->_cumulated_cputime_ns;
            g->_cur_cumulated_worker_time_ns += std::min(now - _last_observation_time,
                                                      cur_cumulated_cputime_ns - g->_last_cumulated_cputime_ns);
            g->_last_cumulated_cputime_ns = cur_cumulated_cputime_ns;
            cputime_ns += g->_cur_cumulated_worker_time_ns;
        }
    });
    _last_observation_time = now;
    return cputime_ns / 1000000000.0;
}

This approach ensures that the reported bthread_worker_usage stays within a reasonable and bounded range by constraining its growth based on both elapsed wall clock time and actual CPU time consumed. It should effectively prevent the metric from exceeding bthread_worker_count, even in cases of long-running, non-yielding tasks.

[chenBright]

是否应该将cputime_ns / 1000000000.0;更改为 cputime_ns / （real_elapse_time）;其中real_elapse_time是这个函数两次调用的时间差。

I think this formula is correct and cannot be modified in this way. The unit of cputime_ns in the formula is ns, and the unit of 1000000000.0 is ns/core. The result is core. The time interval is implemented by PerSecond.

重计算的线程在计算的时候不会增加cputime_ns，但是结束的时候跨秒会把前面用掉的时间都加到当前秒的cputime_ns上，会导致超过很多

I think this is the root cause of the bug.

[chenBright]

I agree that accurately computing bthread_worker_usage within the bthread framework is quite challenging, especially since some tasks may run for extended periods without yielding. Rather than striving for exact precision, we could use a bounded estimation to ensure that bthread_worker_usage not exceeds bthread_worker_count.

Here’s a possible solution:

• For each TaskGroup, add:

  • _cur_cumulated_worker_time_ns: stores the cumulative worker time as observed by bvar.
  • _last_cumulated_cputime_ns: stores the value of TaskGroup::_cumulated_cputime_ns at the time of the last observation.

• For TaskControl, add:

  • _last_observation_time: records the timestamp of the last observation.

Updated get_cumulated_worker_time() Implementation：

double TaskControl::get_cumulated_worker_time() {
int64_t cputime_ns = 0;
BAIDU_SCOPED_LOCK(_modify_group_mutex);
int64_t now = butil::cpuwide_time_ns();
for_each_task_group([&](TaskGroup* g) {
if (g) {
int64_t cur_cumulated_cputime_ns = g->_cumulated_cputime_ns;
g->_cur_cumulated_worker_time_ns += std::min(now - _last_observation_time,
cur_cumulated_cputime_ns - g->_last_cumulated_cputime_ns);
g->_last_cumulated_cputime_ns = cur_cumulated_cputime_ns;
cputime_ns += g->_cur_cumulated_worker_time_ns;
}
});
_last_observation_time = now;
return cputime_ns / 1000000000.0;
}
This approach ensures that the reported bthread_worker_usage stays within a reasonable and bounded range by constraining its growth based on both elapsed wall clock time and actual CPU time consumed. It should effectively prevent the metric from exceeding bthread_worker_count, even in cases of long-running, non-yielding tasks.

Good idea!
But I think this bug has not been fixed. Assume the following scenario: bthread_worker_usage starts sampling at 0 seconds, a task is also started at 0 seconds, and runs on the worker for 1.5 seconds. The worker is idle after running the task.
Then in the (1, 2] sampling period, the CPU time of the worker is calculated to be 1 second, but in fact the CPU time of the worker in this period is only 0.5 seconds. In this way, bthread_worker_usage is still an incorrect value.

I think the essence of the problem is that each sampling misses the CPU time of the tasks that are still running. Maybe we need to solve this problem fundamentally.