Skip to content

fix: stop reporting unknown server values as if they were measurements - #1120

Merged
kesselb merged 1 commit into
masterfrom
fix/unknown-values
Sep 9, 2026
Merged

kesselb merged 1 commit into
masterfrom
fix/unknown-values

Conversation

@karlitschek

Copy link
Copy Markdown
Member

1. Unknown values were rendered as measurements

Every "not available" figure in this app is -1, and several paths did arithmetic on that sentinel or formatted it as though it were a reading:

where what an admin saw
SystemStatistics scaled -1 to kB the OCS API answered mem_total: -1024 to monitoring tools
formatBytes() took Math.abs() PHP's memory_limit = -1 ("unlimited") rendered as "1.0 B"
DatabaseStatistics returned the string 'N/A' for a numeric field "NaN B" on the page
the chart components guarded against an 'N/A' string the backend never sends -1 charted as a real series on a degenerate 0-to-0 axis
the OS backends claimed 1 core when the count was unknown that 1 became the divisor for the load average: an 8-core host read as "800 %"

Unknown values are now -1 throughout and render as "Unknown", or are left out where a percentage cannot be computed. PHP's own "unlimited" sentinels (memory_limit -1, max_execution_time 0) are labelled as such rather than formatted as sizes.

The load average deliberately survives a missing core count — it comes from sys_getloadavg() and is real even when the count is not, so it is still shown, without the percentage it cannot support.

2. …and on a lot of hosts there was nothing to report in the first place

Debian's and Ubuntu's apache2.service ships ProcSubset=pid and ProtectProc=invisible, so the web server sees only its own processes in /proc. I confirmed on such a host that /proc/uptime, /proc/stat, /proc/1, uptime(1), who -b and /run/utmp are all closed to it, with open_basedir and disable_functions empty. On those installs this app has never been able to report CPU, memory or uptime.

Commands still run there, so the Linux backend now falls back:

  • threads → nproc, which uses sched_getaffinity rather than /proc
  • memory → the per-NUMA-node meminfo files under /sys, which ProcSubset does not cover, tried before free(1): free reads /proc/meminfo itself and is bound to fail wherever that file is hidden, so trying /sys first settles which situation we are in without paying for a fork
  • uptime → a process that has only just started carries the system uptime in its own /proc/<pid>/stat, and a process may always read its own entry

aarch64 and s390x are handled too: they list their processors but carry no model name line, so the count is now taken separately from the name.

3. The two-second poll cost more than it needed to

Measured on a live instance: LiveData::getData() takes 1.31 ms inside an HTTP request that takes 467 ms (median of 24). About 99.7% of a poll is the Nextcloud request cycle, so the lever is fewer and cheaper requests, not faster collection.

  • useLiveData stops polling while the tab is hidden and polls immediately on return, so a page left open in the background no longer fetches numbers nobody can see
  • getTime() no longer forks date(1) for something PHP can answer (that fork was 1.28 ms of the 1.31 ms); the system zone comes from /etc/localtime, since Nextcloud itself runs on UTC
  • LiveData no longer calls getCPU() — which parses /proc/cpuinfo, or forks nproc — purely to reach sys_getloadavg(), which uses none of it
  • the sampled uptime is cached as a boot time and re-measured once a minute rather than once per poll

Net effect: hosts with a readable /proc now fork nothing per poll, where every host used to fork date. The new fallbacks never make the fork count worse than it was.

Verification

Unit tests: 31 in LinuxTest (13 new), plus FreeBSD/Dummy/NetInterface green. New fixtures for aarch64 cpuinfo, free -k and /sys node meminfo. Two of the tests assert executeCommand is never called, so the removed forks stay removed. php-cs-fixer clean over all 49 files; psalm "No errors found"; eslint clean.

Measured end to end against a live NC 35 instance whose Apache cannot read /proc:

  • uptime derived from the cached boot time: 326639 s vs /proc/uptime truth 326639.51 s
  • five rapid polls produced exactly one boot-time sample, confirming the cache
  • CPU reported as 2 threads (the true count; it previously claimed 1) with the load average and a real percentage
  • memory reported as 1.81 GB of 3.30 GB, which had never worked on that host
  • visibility gating: 5 /update requests in 12 s while visible, 0 in 12 s while backgrounded, and one immediately on return rather than after the interval
  • no NaN, no -1024, no 1.0 B, no (-1 threads) anywhere on the page

Notes for review

This is three related changes in one commit — honest unknowns, the /proc fallbacks, and the polling cost — because the later two grew directly out of the first and share the -1 contract. Happy to split them into separate PRs if you would rather review them apart.

sampleUptime() is new on IOperatingSystem; FreeBSD and Dummy return -1. Swap stays unknown on hosts that fall back to /sys, which publishes no swap totals — better than inventing a number.

@nextcloud-command nextcloud-command added the AI assisted This PR contains AI-assisted commits label Sep 9, 2026
@karlitschek

Copy link
Copy Markdown
Member Author

Comment thread src/components/MemoryChartSection.vue Outdated
const memTotalText = computed(() => formatMegabytes(props.memTotal as number))
const memUsed = computed(() => formatMegabytes((props.memTotal as number) - (props.memFree as number)))
const swapUsed = computed(() => formatMegabytes((props.swapTotal as number) - (props.swapFree as number)))
const memTotalText = computed(() => formatMegabytesprops.memTotal)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const memTotalText = computed(() => formatMegabytesprops.memTotal)
const memTotalText = computed(() => formatMegabytes(props.memTotal)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const memTotalText = computed(() => formatMegabytesprops.memTotal)
const memTotalText = computed(() => formatMegabytes(props.memTotal))

Every "not available" figure in this app is -1, and several paths did
arithmetic on that sentinel or formatted it as though it were a reading:

- SystemStatistics scaled it to kB, so the OCS API answered
  mem_total: -1024 to external monitoring tools
- formatBytes() took the absolute value, so PHP's memory_limit = -1
  ("unlimited") rendered as "1.0 B"
- DatabaseStatistics returned the string 'N/A' for a field typed and
  formatted as a number, which reached the page as "NaN B"
- MemoryChartSection and ResourceOverviewSection guarded against an 'N/A'
  string the backend never sends, so -1 charted as a real series on a
  degenerate 0-to-0 axis
- the OS backends claimed a single core when the count was unknown, and
  that 1 became the divisor for the load average: an eight-core host under
  load read as "800 %"

Unknown values are now -1 throughout and render as "Unknown", or are left
out where a percentage cannot be computed. PHP's own "unlimited" sentinels
(memory_limit -1, max_execution_time 0) are labelled as such rather than
formatted as sizes. The load average survives a missing core count: it
comes from sys_getloadavg() and is real even when the count is not.

Read CPU, memory and uptime where /proc is hidden

Debian's and Ubuntu's apache2 unit ships ProcSubset=pid and
ProtectProc=invisible, so the web server sees only its own processes in
/proc. On those hosts -- a large share of installs -- this app has never
been able to report CPU, memory or uptime at all, and now reports honestly
that it cannot, which is worse to look at than it needs to be.

Commands still run there, so the Linux backend now falls back:

- threads: nproc, which uses sched_getaffinity rather than /proc
- memory: the per-NUMA-node meminfo files under /sys, which ProcSubset
  does not cover, before free(1) -- free reads /proc/meminfo itself and is
  bound to fail wherever that file is hidden, so trying /sys first settles
  which situation we are in without paying for a fork
- uptime: a process that has only just started carries the system uptime
  in its own /proc/<pid>/stat, and a process may always read its own entry

aarch64 and s390x are handled too: they list their processors but carry no
"model name" line, so the count is now taken separately from the name.

Cut the cost of the two-second poll

A poll spends about 1.3 ms collecting data inside a request that costs
hundreds of milliseconds, so the fix is fewer and cheaper requests, not
faster collection:

- useLiveData stops polling while the tab is hidden and polls immediately
  on return, so a page left open in the background no longer fetches
  numbers nobody can see
- getTime() no longer forks date(1) for something PHP can answer; the
  system zone is read from /etc/localtime because Nextcloud runs on UTC
- LiveData no longer calls getCPU() -- which parses /proc/cpuinfo, or forks
  nproc -- only to reach sys_getloadavg(), which uses none of it
- the sampled uptime is cached as a boot time and re-measured once a
  minute rather than once per poll

Together these leave the poll forking less than it did before: hosts with a
readable /proc now fork nothing at all, where every host used to fork date.

Signed-off-by: Frank Karlitschek <frank.karlitschek@nextcloud.com>
Assisted-by: Claude Code:claude-opus-5
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb
kesselb enabled auto-merge September 9, 2026 17:23
@kesselb
kesselb merged commit 390086f into master Sep 9, 2026
44 checks passed
@kesselb
kesselb deleted the fix/unknown-values branch September 9, 2026 17:29
@kesselb

kesselb commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

/backport to stable35 please

@backportbot

backportbot Bot commented Sep 9, 2026

Copy link
Copy Markdown

The backport to stable35 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable35
git pull origin stable35

# Create the new backport branch
git checkout -b backport/1120/stable35

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 987fd83e

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/1120/stable35

Error: Failed to check for changes with origin/stable35: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@kesselb

kesselb commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

/backport to stable35 please

@backportbot

backportbot Bot commented Sep 9, 2026

Copy link
Copy Markdown

The backport to stable35 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable35
git pull origin stable35

# Create the new backport branch
git checkout -b backport/1120/stable35

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 987fd83e

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/1120/stable35

Error: Failed to check for changes with origin/stable35: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

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

Labels

AI assisted This PR contains AI-assisted commits backport-request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants