Conversation
…tion A divide-by-zero in the process CPU percentage calculation could produce +Inf/NaN when two CPU snapshots reported identical total system jiffies (very frequent stats sampling, or coarse platform CPU accounting). SgwFloatStat then emitted raw "+Inf"/"NaN" bytes, which the JSON encoder rejected, collapsing the entire SgwStats blob to "null" in sg_stats.log. - Guard the CPU percentage division: return 0 when no total system time has elapsed. The arithmetic is extracted into cpuStatsSnapshot.cpuPercentageSince so the degenerate case is unit-testable without reading live system state. - Harden SgwFloatStat rendering (shared formatValue used by MarshalJSON and String) to emit "0" for non-finite values, so no single stat can corrupt the whole serialized blob or the expvar output. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents non-finite process CPU utilization stats (+Inf/NaN) from breaking Sync Gateway stats JSON/expvar serialization by guarding the CPU percentage calculation and hardening float stat rendering.
Changes:
- Extracted CPU utilization arithmetic into
cpuStatsSnapshot.cpuPercentageSinceand guarded divide-by-zero/degenerate total-time deltas. - Hardened
base.SgwFloatStatJSON/expvar rendering to emit0for non-finite float values. - Added regression tests covering both the CPU percentage degenerate case and non-finite stat marshaling.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rest/stats_context.go | Refactors CPU % calculation into a testable helper and guards zero/negative total-time deltas. |
| rest/stats_context_test.go | Adds unit tests for the CPU % divide-by-zero regression and normal-case math. |
| base/stats.go | Adds SgwFloatStat.formatValue() to sanitize NaN/Inf during JSON/expvar rendering. |
| base/stats_test.go | Adds regression test ensuring non-finite float stats don’t break stats JSON/expvar output. |
Comment on lines
+190
to
+197
| // The output must be valid, parseable JSON. | ||
| var roundTripped map[string]float64 | ||
| require.NoError(t, JSONUnmarshal(marshalled, &roundTripped)) | ||
|
|
||
| // String() satisfies expvar.Var, which likewise requires a valid JSON value. | ||
| var viaString float64 | ||
| require.NoError(t, JSONUnmarshal([]byte(stat.String()), &viaString)) | ||
| }) |
6 tasks
torcolvin
reviewed
Jul 6, 2026
torcolvin
left a comment
torcolvin
left a comment
Collaborator
There was a problem hiding this comment.
Nice catch, I think this is mostly fine.
Comment on lines
+119
to
+120
| // Store the current values as the previous values for the next time this function is called | ||
| statsContext.cpuStatsSnapshot = currentSnapshot |
Collaborator
There was a problem hiding this comment.
Should we drop cpuprecentagesince is 0? I think it is probably fine to keep it since it would probably be the same
torcolvin
previously approved these changes
Jul 6, 2026
- Assert non-finite float stats fall back to 0, not just valid JSON - Trim cpuPercentageSince doc comment to describe the zero-delta case Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
torcolvin
approved these changes
Jul 7, 2026
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.
CBG-3658
A divide-by-zero in the process CPU percentage calculation could produce +Inf/NaN when two CPU snapshots reported identical total system jiffies (very frequent stats sampling, or coarse platform CPU accounting). SgwFloatStat then emitted raw "+Inf"/"NaN" bytes, which the JSON encoder rejected, collapsing the entire SgwStats blob to "null" in sg_stats.log.
Integration Tests