fix(utils): advance the CPU tick baseline in systemInfoV2 getCurrentCpuTicks#3824
Open
anxkhn wants to merge 1 commit into
Open
fix(utils): advance the CPU tick baseline in systemInfoV2 getCurrentCpuTicks#3824anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
…puTicks
getCurrentCpuTicks() computes the CPU load as the delta between the current
os.cpus() ticks and a module-level `previousTicks` baseline, but it never wrote
the new sample back to `previousTicks`. The baseline therefore stayed at
{ idle: 0, total: 0 } for the whole process, so every call returned the average
CPU load since boot instead of the load since the previous sample.
Because systemInfoV2 is enabled by default and getCurrentCpuTicks() is the
fallback used in uncontainerized environments, cgroup-less containers and AWS
Lambda, its result feeds cpuCurrentUsage / isCpuOverloaded and drives the
AutoscaledPool. With a frozen boot-average signal the CPU overload flag barely
moves during a run, which defeats CPU-based autoscaling.
Write the sample back after computing the delta (Object.assign(previousTicks,
ticks)), mirroring the V1 path in local_event_manager.createCpuInfo and the
containerized branch in this same file. The now-redundant non-null assertions on
the `previousTicks` const are dropped.
Adds a regression test that takes two samples and asserts the second reflects
the inter-sample delta rather than the cumulative-since-boot ratio, and resets
the shared baseline before each test so the module state no longer leaks between
tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
getCurrentCpuTicks()inpackages/utils/src/internals/systemInfoV2/cpu-info.tscomputes CPU load as the delta between the current
os.cpus()tick counts and amodule-level
previousTicksbaseline, but it never wrote the new sample back topreviousTicks. The baseline therefore stayed at{ idle: 0, total: 0 }for thewhole process, so every call returned the average CPU load since boot instead
of the load since the previous sample.
Why it matters
systemInfoV2is enabled by default, andgetCurrentCpuTicks()is the CPUfallback used in uncontainerized environments, cgroup-less containers and AWS
Lambda (the other branches use cgroup stats). Its result feeds
cpuCurrentUsage/isCpuOverloaded, which drive theAutoscaledPool. With afrozen boot-average signal the CPU value barely moves for the rest of a run, so
CPU-based autoscaling never reacts to real load.
The intended contract is already established twice in the codebase:
LocalEventManager.createCpuInfo(
packages/core/src/events/local_event_manager.ts) does the identical deltamath and then
Object.assign(this.previousTicks, ticks);previousSample = sample.getCurrentCpuTicks()was simply missing the equivalent write-back.The fix
Write the sample back after computing the delta, mirroring both siblings:
The now-redundant non-null assertions (
previousTicks!) on the non-null const aredropped.
Test
Adds a regression test to
test/utils/cpu-infoV2.test.tsthat takes two samplesand asserts the second reflects the inter-sample delta (
0.75) rather than thecumulative-since-boot ratio (
~0.583, which the buggy version returned). AbeforeEachresets the shared module baseline so the singleton state cannot leakbetween tests.
Red -> green: against the unpatched source the new test fails
(
expected 0.5833333333333333 to be close to 0.75); with the fix the file passes18/18.
test/core/autoscaling/(snapshotter + system status + autoscaled pool)also stays green at 39/39.