Skip to content

fix(specs): report Docker resource availability accurately, not as a reservation#31

Open
Sentinel-Bluebuilder wants to merge 2 commits into
sentinel-official:mainfrom
Sentinel-Bluebuilder:fix/docker-resource-labels
Open

fix(specs): report Docker resource availability accurately, not as a reservation#31
Sentinel-Bluebuilder wants to merge 2 commits into
sentinel-official:mainfrom
Sentinel-Bluebuilder:fix/docker-resource-labels

Conversation

@Sentinel-Bluebuilder

Copy link
Copy Markdown
Contributor

Summary

The specs:v1 on-chain memo publishes cr/rr next to total host cores/RAM.
Both were labeled "reserved for the dvpn-node container", but nothing is
actually reserved
— the node container is launched with no CPU or memory cap
(no HostConfig.Memory/NanoCpus). The values came from docker info
NCPU/MemTotal, i.e. the Docker engine totals — the real ceiling a
container can draw from, not a reservation.

On Windows this is a concrete inaccuracy: the engine runs inside the WSL2 VM,
whose default RAM allocation is min(50% host, 8 GB). A 16 GB host therefore
published rr = 16 GB while the container could only ever see ~8 GB.

This PR keeps the (correct) engine-derived values and fixes the semantics +
labels
: cr/rr now mean "available to the container (Docker/WSL2 VM)",
with r/c as the host totals — so the two fields carry distinct, meaningful
information instead of looking like redundant duplicates.

No runtime/behavior change. The node still runs uncapped and earns at full
capacity. Setting real HostConfig limits (user-settable reservations) is a
separate, deliberately deferred change.

Changes

  • node-specs.ts — rewrite the memo schema doc with the host-vs-VM
    rationale; relabel captureLocalSpecs (values unchanged); captureRemoteSpecs
    now probes docker info --format "{{.NCPU}}|{{.MemTotal}}" over SSH
    so
    remote nodes report the engine view too. Bail-safe — falls back to /proc
    host totals when Docker isn't reachable over SSH (on a single-purpose VPS the
    engine == host, so that's the right default).
  • shared/types.tsNodeSpecsSnapshot doc updated.
  • Renderer labels → "available": OnChainSpecs (explainer text, the
    meter's "Available / Total", field descriptions), System (Docker resource
    cells), ManageDocker (card title + stats), NodeDetails ("RAM available"),
    DeployLocal (explainer bullet).

Secondary commit

  • test(metrics)better-sqlite3 is a single-ABI native addon built for
    Electron's ABI (npm run rebuild:electron), which the vitest runner (plain
    Node) can't dlopen. The metrics suite previously ran against the no-op
    degraded store and could fail with 0 rows. It now probes the binding once
    and describe.skips with a logged reason when the ABI doesn't match,
    while still running the full logic when it does. Keeps the suite green and
    honest off main.

Verification

npx vitest run43 passed, 3 skipped (the metrics suite, ABI mismatch,
skip reason logged), 0 failed.

…reservation

The specs:v1 on-chain memo exposes cr/rr alongside total host cores/RAM. Both
were labeled "reserved for the dvpn-node container", but the node container is
launched with no CPU or memory cap (no HostConfig Memory/NanoCpus), so nothing
is actually reserved. The values were sourced from `docker info` NCPU/MemTotal
(the Docker engine totals) which is the real ceiling a container can draw from,
not a reservation.

On Windows this produced a concrete inaccuracy: the engine runs inside the WSL2
VM, whose default RAM allocation is min(50% host, 8 GB). A 16 GB host therefore
published rr=16 GB while the container could only ever see ~8 GB.

This change keeps the (correct) engine-derived values and fixes the semantics:
cr/rr now mean "available to the container (Docker/WSL2 VM)", with r/c as the
host totals, so the two fields carry distinct, meaningful info instead of
looking like redundant duplicates. No runtime/behavior change; the node still
runs uncapped.

- node-specs.ts: rewrite memo schema doc; relabel captureLocalSpecs comment;
  captureRemoteSpecs now probes `docker info --format {{.NCPU}}|{{.MemTotal}}`
  over SSH so remote reports the engine view too (bail-safe, falls back to
  /proc host totals when Docker is unreachable).
- shared/types.ts: NodeSpecsSnapshot doc updated.
- Renderer labels updated to "available": OnChainSpecs (explainer, meter,
  field descs), System (Docker resource cells), ManageDocker (card title +
  stats), NodeDetails (RAM available), DeployLocal (explainer bullet).

Does not set HostConfig limits; user-settable reservations is a separate,
deferred change.
better-sqlite3 is a single-ABI native addon. The shipping app builds it for
Electron's ABI via `npm run rebuild:electron`, which the vitest runner (plain
Node, a different NODE_MODULE_VERSION) physically cannot dlopen. When that's
the case the metrics store degrades to a no-op, so the assertions silently
"passed" against a disabled store before and fail (0 rows) when the binary
happens to match neither runtime.

Probe the binding once (open an in-memory DB to force the dlopen) and
describe.skip the suite WITH A logged reason when it can't load, instead of
running against a dead store. The full logic still runs whenever the runner
ABI matches the built binary (`npm rebuild better-sqlite3`, or under Electron).
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