Skip to content

Cache VM stats - #859

Open
ben-grande wants to merge 1 commit into
QubesOS:mainfrom
ben-grande:cache-stats
Open

ben-grande wants to merge 1 commit into
QubesOS:mainfrom
ben-grande:cache-stats

Conversation

@ben-grande

@ben-grande ben-grande commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

TTL cache to avoid multiple clients or requests from the same client of blocking CPU cycles from qubesd.

@ben-grande

Copy link
Copy Markdown
Contributor Author

Marek said: #827 (comment)

The change to use self.stats_cache_time instead of the explicit
value from the parameter is not great. If nobody is updating
(requesting) stats for some time, next one will return average of
a long time pretending it's the "current" value.

Let's assume the previous moment the stats were collected was a long
time ago.

  • Empty previous, stats never called:

    • Without this PR: 0%
    • With PR: 0%
    • Resolution: Unaffected
  • Empty previous, but stats generated at least once:

    • Without this PR: 0%
    • With PR: 0%, calculate_cpu_usage_raw() is not called as
      domid is not in previous, as there is no previous
    • Resolution: Unaffected
  • Non-empty previous:

    • Without PR: avg considering the previous time
    • With PR: avg considering the previous time
    • Resolution: It already happened if the client is not using the
      API loop and calling get_vm_stats() directly
  • Empty previous, but stats generated at least once:
    ...
    • With PR: 0%, calculate_cpu_usage_raw() is not called as
      domid is not in previous, as there is no previous

But let's assume that I did an average of a long time by assigning previous to the last cache if previous=None:

if not previous and self.stats_cache_data:
    previous = self.stats_cache_data

I don't think it would be wrong to have an average of a long time compared to reporting 0%.

@ben-grande
ben-grande force-pushed the cache-stats branch 2 times, most recently from 92d83f7 to 45f3fdb Compare August 12, 2026 08:58
Clients that subscribe will each query extraneously query the statistics
(spending time) and get different results for time based data such as
CPU usage (elapsed time).

This patch aims to fix those problems, but current the sleep between
queries is only considered on the API.
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 59.57447% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.49%. Comparing base (e231e51) to head (3aa3201).

Files with missing lines Patch % Lines
qubes/app.py 58.06% 13 Missing ⚠️
qubes/api/admin.py 62.50% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #859      +/-   ##
==========================================
- Coverage   70.51%   70.49%   -0.02%     
==========================================
  Files          61       61              
  Lines       14371    14375       +4     
==========================================
  Hits        10133    10133              
- Misses       4238     4242       +4     
Flag Coverage Δ
unittests 70.49% <59.57%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ben-grande
ben-grande marked this pull request as ready for review August 12, 2026 09:24
@ben-grande

Copy link
Copy Markdown
Contributor Author

If you don't like the _get_vm_stats checking for the cache. I could make it default to ignore so the behavior doesn't change (even though it is a protected version), and then, if calling via admin.vm.Stats, always use the cache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant