This change aims to address #2065 except for tracking the main_thread utilization.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.77%. Comparing base (0b1f99d) to head (b6822b1).
⚠️ Report is 20 commits behind head on unstable.

@@             Coverage Diff              @@
##           unstable    #2463      +/-   ##
============================================
+ Coverage     74.33%   74.77%   +0.44%     
============================================
  Files           129      129              
  Lines         71009    71137     +128     
============================================
+ Hits          52784    53193     +409     
+ Misses        18225    17944     -281     

@deepakrn Good, I hope we can also find a way to track main thread usefulness.

Can you link the PR to the issue where this was discussed?

Do we have any concerns against this approach? The only concern for me is that we would print times for all io-threads, should we cap number of printed thread times to 16 for example?

Thanks for the question @dvkashapov.

See #2065 (comment) for relevant discussion. Since setups with thread count > 16 are rare, we thought we could start without that optimization.

@deepakrn Finally, we have made a decision about these INFO fields. Sorry for the delay. What we decided is written in the issue #2065:

• Use field names used_cpu_active_io_thread_1, _2, etc.
• Don't introduce the thread uptime field. (Users can just compare the active CPU time between two different INFO calls.)
• Make sure that we don't reset the number to zero if the number of IO threads decrease and then increase again when the configured number of I/O threads is changed during runtime. (We should add a test case for this scenario.)

Do you want to update this PR to match the above?

Now, we also have the implementation for the main thread CPU utilization #2931 by Daniil, so I'm excited to get both of these PRs released in 9.1.

Hi @zuiderkwast , Thanks for the update and the idea makes sense to me. I'll work on making these changes.

@zuiderkwast - couple of follow ups.

Don't introduce the thread uptime field. (Users can just compare the active CPU time between two different INFO calls.)

If we are getting rid of thread uptime field, there wouldn't be an easy way for the user to know if the thread was running but was busy looping waiting for work? Wouldn't that be a gap in observability? The user could still track total time elapsed and look at delta of used_active_cpu_time but that wouldn't tell for how long the IO thread was enabled in that duration?

Make sure that we don't reset the number to zero if the number of IO threads decrease and then increase again when the configured number of I/O threads is changed during runtime. (We should add a test case for this scenario.)

What shouldn't be reset to zero? Are you referring to the active cpu time? If yes, isn't that already the case? Also, I am assuming that INFO should display the active CPU time of all IO threads with value > 0?

The user could still track total time elapsed and look at delta of used_active_cpu_time but that wouldn't tell for how long the IO thread was enabled in that duration?

From relevant issue:

Even without it, the caller can compare the the delta between two points in time and use this delta to compare the I/O thread's active time delta between two INFO calls. To calculate the exact time between two calls to INFO, the delta of the INFO field for the server's uptime can be used.

What shouldn't be reset to zero? Are you referring to the active cpu time? If yes, isn't that already the case?

Looks like you're right about not resetting cpu time currently. The case we want to make sure works is not resetting metric when io thread becomes active->not active->active again.

If we are getting rid of thread uptime field, there wouldn't be an easy way for the user to know if the thread was running but was busy looping waiting for work? Wouldn't that be a gap in observability? The user could still track total time elapsed and look at delta of used_active_cpu_time but that wouldn't tell for how long the IO thread was enabled in that duration?

I don't think it's important to know whether the thread was disabled or just waiting during some time.

To get a percentage between two points in time, you can for example take the delta of the active CPU time for an IO thread and divide by the delta of the server_time_usec INFO field.

What shouldn't be reset to zero? Are you referring to the active cpu time? If yes, isn't that already the case? Also, I am assuming that INFO should display the active CPU time of all IO threads with value > 0?

I believe that's already the case yes, but I'm not completely sure. I haven't verified it. If it's not too much work to write a test case for this, it would be nice to have.

INFO should display the active CPU time of the IO threads that are currently configured in the config io-threads config, even if some of them report zero. If we have had more I/O threads configured in the past, these threads will not run and will not change their CPU time. We don't need to report them in INFO every time. A server can run for years.

Update.

I have been discussing back and forth with various people weather these metrics should measure the CPU-time during active work or just monotonic time of active work.

Currently, the preference is to use the monotonic clock, not the CPU-time clock. The reason is that a low CPU usage doesn't necessarily mean that the thread has spare capacity to handle more work.

If we instead measure monotonic time and see that 90% of the time, the thread is doing work, it means that the remaining 10%, the thread is waiting for more work. This is better information, because the purpose of these metrics is to be able to tell how much more work the I/O threads can handle.

For a longer expanation, see this #2931 (comment).

@deepakrn do you still think CPU-time is better? If there are any more aspects to consider, I'm interested so we can be sure we make the right decision.

If we decide to use the monotonic clock, we shouldn't include "cpu" in the field names, because it can be misleading. We can rename them to used_active_time_io_thread_1.

@zuiderkwast - Clarifying my thought process below:

If we do monotonic clock, we won't have visibility into whether the thread was context switched or was actually doing work in the Valkey process.

If we do CPU clock, it tells us the amount of time that was spent running the process excluding blocking syscalls, preemptions etc.

CPU clock time < monotonic clock time < total time

Understanding the metric with an example:

If (CPU_clock_time_delta/total_time) for a io_thread is 80%, it means 80% of the time was spent doing real work in Valkey and 20% comprises - context_swtich, blocking syscall, busy polling.

if (monotic_time_delta/total_time) for a io_thread is 80%, it means 80% comprises of (context_swith + blocking syscall + active work) and 20% comprises primarily of (busy polling + some more context switch potentially).

So, (100 - monotonic_time_delta/total_time) could be closer to the time spent busy polling (and hence potential free capacity) than (100 - cpu_time_delta/total_time).

If my understanding is correct, it seems like monotonic_clock would better tell us about free capacity than cpu_time. WDYT?

Yes, exactly, good explanation. We came to the same conclusion.

• Active time = time trying to work = (cpu-time + context switch + blocking syscalls)
• Not active time = time waiting for work

So let's change to getMonotonicUs and the name used_active_time_io_thread_%d. When you have time.