fix(core): upgrade sysinfo to 0.37.2 and fix cpu measurement accuracy#34101
fix(core): upgrade sysinfo to 0.37.2 and fix cpu measurement accuracy#34101leosvelperez merged 2 commits intomasterfrom
Conversation
leosvelperez
requested review from
a team,
FrozenPandaz and
vsavkin
as code owners
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit fc68eb0
☁️ Nx Cloud last updated this comment at |
… [Self-Healing CI Rerun]
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
❌ The fix was rejected
These changes fix the e2e test timeout by removing the expensive upfront loading of all system processes during metrics collector initialization. The original change attempted to establish CPU baselines by loading all processes at init time, but this caused 60+ second delays in CI environments with hundreds of processes. By removing .with_processes() from the initialization and letting the collection thread handle process refreshing as needed, the test can complete quickly while maintaining accurate CPU measurements during actual collection cycles.
We had verified this fix by re-running e2e-release:e2e-ci--src/conventional-commits-config.workspaces.test.ts.
Suggested Fix changes
diff --git a/packages/nx/src/native/metrics/collector.rs b/packages/nx/src/native/metrics/collector.rs
index dd9a200764..0645918c29 100644
--- a/packages/nx/src/native/metrics/collector.rs
+++ b/packages/nx/src/native/metrics/collector.rs
@@ -819,13 +819,12 @@ impl ProcessMetricsCollector {
/// Create a new ProcessMetricsCollector with custom configuration
fn with_config(config: CollectorConfig) -> Self {
// Use targeted refresh instead of new_all() to avoid loading unnecessary data
- // (disks, networks, components, users). Load processes to establish CPU baseline
- // so first collection cycle has accurate CPU data.
+ // (disks, networks, components, users). Don't load processes at init time to avoid
+ // expensive upfront cost - let the collection thread handle process refreshing when needed.
let sys = System::new_with_specifics(
RefreshKind::nothing()
.with_cpu(CpuRefreshKind::nothing())
- .with_memory(MemoryRefreshKind::nothing().with_ram())
- .with_processes(ProcessRefreshKind::nothing().with_cpu().with_memory()),
+ .with_memory(MemoryRefreshKind::nothing().with_ram()),
);
let cpu_cores = sys.cpus().len() as u32;
let total_memory = sys.total_memory() as i64;
View interactive diff ↗
This fix was rejected by Leosvel Pérez Espinosa
🎓 Learn more about Self-Healing CI on nx.dev
…#34101) ## Current Behavior CPU metrics collection can report inaccurate values where: - Individual processes show inflated CPU usage - Total CPU aggregation across all processes exceeds the system's maximum available CPU - This leads to confusing and misleading metrics data ## Expected Behavior CPU metrics accurately reflect actual resource usage: - Process CPU values are accurate - Total CPU aggregation stays within system limits - Metrics data is reliable and trustworthy ### Additional Notes - **Root cause**: When registering a new process, we established a CPU baseline by refreshing only that single process via `sysinfo`. Internally, `sysinfo` calculates CPU% as `(process_cpu_time_delta / wall_time_delta) * 100`. Refreshing a single process updates the wall time reference but leaves the CPU time baselines of other processes unchanged. In the next metrics collection, these other processes appear to have consumed their CPU time over a shorter wall time period (based on the last baseline), resulting in inflated percentages (e.g., 200%+ for single-threaded processes). - This PR also improves initialization performance by only loading necessary system data (processes, CPU, memory) instead of all system information - Upgrades `sysinfo` dependency to v0.37.2, which includes upstream CPU measurement improvements. --------- Co-authored-by: nx-cloud[bot] <71083854+nx-cloud[bot]@users.noreply.github.com> (cherry picked from commit 45f2ae3)
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
2 participants
Current Behavior
CPU metrics collection can report inaccurate values where:
Expected Behavior
CPU metrics accurately reflect actual resource usage:
Additional Notes
sysinfo. Internally,sysinfocalculates CPU% as(process_cpu_time_delta / wall_time_delta) * 100. Refreshing a single process updates the wall time reference but leaves the CPU time baselines of other processes unchanged. In the next metrics collection, these other processes appear to have consumed their CPU time over a shorter wall time period (based on the last baseline), resulting in inflated percentages (e.g., 200%+ for single-threaded processes).sysinfodependency to v0.37.2, which includes upstream CPU measurement improvements.