Skip to content

Commit 1ce86d0

Browse files
authored
Merge pull request #21 from NodeDB-Lab/fix/idle-cpu-burn
fix: eliminate idle CPU burn + missing system-table warnings + Docker volume UX (closes #20)
2 parents b4cc83f + d96d81e commit 1ce86d0

39 files changed

Lines changed: 1077 additions & 351 deletions

.config/nextest.toml

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
# nextest configuration. Run with: cargo nextest run --all-features
2+
#
3+
# Why nextest over `cargo test`:
4+
# - Each test runs in its own process → no in-process state contention.
5+
# Integration tests that spawn 3-node clusters used to hang under
6+
# `cargo test`'s default within-binary parallelism because multiple
7+
# clusters in the same process exhausted ports / file descriptors.
8+
# - Per-test timeouts make hangs fail fast instead of stalling CI.
9+
# - Better failure output, retry support, and JUnit XML for CI.
10+
11+
[profile.default]
12+
# Hard ceiling per test. Anything above this is a bug, not a slow test.
13+
slow-timeout = { period = "30s", terminate-after = 4 }
14+
15+
# Use every available core for cheap unit tests. Heavy cluster tests
16+
# are kept from starving by `threads-required` overrides below — they
17+
# claim ALL slots so nothing else runs alongside them, regardless of
18+
# whether you're on a 24-core dev box or a 2-core CI runner.
19+
test-threads = "num-cpus"
20+
21+
# Heavy cluster tests: each one brings up 3 servers + per-node Tokio
22+
# runtimes. Two things keep them stable across machine sizes:
23+
#
24+
# 1. `test-group = "cluster"` with `max-threads = 1` ensures at
25+
# most ONE cluster test runs at a time (no two clusters share
26+
# ports / file descriptors / thread pools).
27+
# 2. `threads-required = "num-test-threads"` makes the running
28+
# cluster test claim every available test slot, which evicts
29+
# every other test from the run-queue while it's executing.
30+
# That's what prevents a 24-core dev box from scheduling 23
31+
# unit tests alongside the cluster and starving its Raft
32+
# heartbeats.
33+
#
34+
# The combined effect: cluster tests run strictly serially AND
35+
# strictly alone, and the rest of the suite gets full parallelism
36+
# the moment the cluster test finishes.
37+
[[profile.default.overrides]]
38+
filter = '''
39+
binary(/cluster/)
40+
| binary(/cross_node/)
41+
| binary(/_lease_/)
42+
| binary(descriptor_lease_drain)
43+
| binary(descriptor_lease_forwarding_and_renewal)
44+
| binary(descriptor_lease_planner_integration)
45+
| binary(descriptor_versioning_cross_node)
46+
| binary(prepared_cache_invalidation)
47+
| binary(sql_cluster_cross_node_dml)
48+
'''
49+
test-group = 'cluster'
50+
threads-required = 'num-test-threads'
51+
# Cluster tests bring up real Raft nodes and racy multi-node
52+
# convergence checks. They're flaky enough that one retry catches
53+
# legitimate startup jitter without hiding real regressions — a
54+
# genuinely broken test fails twice in a row.
55+
retries = { backoff = "fixed", count = 2, delay = "1s" }
56+
57+
[test-groups]
58+
cluster = { max-threads = 1 }
59+
60+
[profile.ci]
61+
# CI inherits the default profile (cluster group, threads-required,
62+
# slow-timeout) and adds:
63+
# - more retries: CI runners are ~2× slower per-core than dev
64+
# workstations, so the cluster tests' in-test `wait_for`
65+
# budgets are proportionally tighter. Three retries (four total
66+
# attempts) buys headroom for jitter without papering over real
67+
# regressions — a genuinely broken test fails four times in a row.
68+
# - JUnit XML: picked up by the workflow's artifact upload.
69+
#
70+
# NOTE: we deliberately do NOT bump `slow-timeout` here. The
71+
# slow-timeout only controls when nextest gives up on a stuck
72+
# *process*; it does NOT extend the test's internal `wait_for`
73+
# budgets. Once a `wait_for` panics, the test has already failed —
74+
# making nextest wait longer just wastes CI minutes on cleanup.
75+
retries = { backoff = "fixed", count = 3, delay = "2s" }
76+
fail-fast = false
77+
78+
[profile.ci.junit]
79+
path = "junit.xml"

.github/workflows/test.yml

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,5 +55,21 @@ jobs:
5555
sudo apt-get install -y --no-install-recommends \
5656
cmake clang libclang-dev pkg-config protobuf-compiler perl \
5757
libcurl4-openssl-dev libsasl2-dev
58+
# nextest is required — `.config/nextest.toml` defines the
59+
# `cluster` test-group that serializes 3-node integration tests
60+
# and the `ci` profile that retries flaky cluster tests once and
61+
# writes a JUnit report. Plain `cargo test` ignores all of that
62+
# and will hang/fail on the cluster suite.
63+
- name: Install cargo-nextest
64+
uses: taiki-e/install-action@v2
65+
with:
66+
tool: nextest
5867
- name: Run tests
59-
run: cargo test --all-features --profile ci
68+
run: cargo nextest run --all-features --cargo-profile ci --profile ci
69+
- name: Upload JUnit report
70+
if: always()
71+
uses: actions/upload-artifact@v4
72+
with:
73+
name: junit-report
74+
path: target/nextest/ci/junit.xml
75+
if-no-files-found: ignore

Dockerfile

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@ FROM debian:bookworm-slim AS runtime
3636

3737
# ca-certificates: needed for JWKS fetch, OTLP export, S3 archival
3838
# curl: needed for HEALTHCHECK
39+
# gosu: drop privileges from root after fixing data-dir ownership in entrypoint
3940
RUN apt-get update && apt-get install -y --no-install-recommends \
4041
ca-certificates \
4142
curl \
43+
gosu \
4244
&& rm -rf /var/lib/apt/lists/*
4345

4446
# Non-root user
@@ -51,12 +53,18 @@ RUN mkdir -p /var/lib/nodedb /etc/nodedb \
5153

5254
COPY --from=builder /build/target/release/nodedb /usr/local/bin/nodedb
5355

56+
# Entrypoint: when started as root, fix data-dir ownership and drop to the
57+
# nodedb user. When already started as a non-root user (e.g. `--user 10001`),
58+
# exec directly. This makes `-v <named-volume>:/var/lib/nodedb` work even
59+
# when Docker initialises the volume as root-owned (common on Linux hosts).
60+
COPY docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
61+
RUN chmod +x /usr/local/bin/docker-entrypoint.sh
62+
5463
# Bind to all interfaces (required for Docker port mapping)
5564
# Point data dir at the declared volume
5665
ENV NODEDB_HOST=0.0.0.0 \
5766
NODEDB_DATA_DIR=/var/lib/nodedb
5867

59-
USER nodedb
6068
WORKDIR /var/lib/nodedb
6169

6270
# pgwire | native protocol | HTTP API | WebSocket sync | OTLP gRPC | OTLP HTTP
@@ -67,4 +75,5 @@ VOLUME ["/var/lib/nodedb"]
6775
HEALTHCHECK --interval=10s --timeout=3s --start-period=5s \
6876
CMD curl -f http://localhost:6480/health || exit 1
6977

70-
ENTRYPOINT ["/usr/local/bin/nodedb"]
78+
ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
79+
CMD ["/usr/local/bin/nodedb"]

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ For development or contributing:
148148
git clone https://github.com/NodeDB-Lab/nodedb.git
149149
cd nodedb
150150
cargo build --release
151-
cargo test --all-features
151+
cargo install cargo-nextest --locked # one-time
152+
cargo nextest run --all-features
152153
```
153154

154155
## Status

docker-entrypoint.sh

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#!/bin/sh
2+
# NodeDB container entrypoint.
3+
#
4+
# When invoked as root (the default for `docker run` with no --user), fix
5+
# ownership of NODEDB_DATA_DIR and drop privileges to the unprivileged
6+
# `nodedb` user before exec'ing the server. When invoked as any other UID
7+
# (e.g. `--user 10001` or via Kubernetes runAsUser), exec directly and
8+
# leave the data directory alone.
9+
#
10+
# This makes `-v <named-volume>:/var/lib/nodedb` work even when Docker
11+
# initialises the named volume as root-owned (common on Linux hosts where
12+
# the volume is created out-of-band before the container's first run).
13+
14+
set -e
15+
16+
DATA_DIR="${NODEDB_DATA_DIR:-/var/lib/nodedb}"
17+
18+
if [ "$(id -u)" = "0" ]; then
19+
# Running as root: ensure the data dir exists and is owned by nodedb,
20+
# then drop privileges. mkdir is a no-op for the declared VOLUME but
21+
# protects against custom NODEDB_DATA_DIR overrides.
22+
mkdir -p "$DATA_DIR"
23+
chown -R nodedb:nodedb "$DATA_DIR"
24+
exec gosu nodedb "$@"
25+
fi
26+
27+
# Already non-root: ensure we can actually write to the data dir, otherwise
28+
# fail fast with a clear message instead of the cryptic WAL "Permission
29+
# denied (os error 13)" the user sees on a misconfigured volume mount.
30+
if [ ! -w "$DATA_DIR" ]; then
31+
cat >&2 <<EOF
32+
nodedb: data directory $DATA_DIR is not writable by uid=$(id -u) gid=$(id -g).
33+
34+
This usually means a host volume was mounted with root ownership while
35+
NodeDB is configured to run as a non-root user. Fixes:
36+
37+
1. Let the entrypoint fix it: drop the explicit --user flag so the
38+
container starts as root and chowns the volume on first boot.
39+
2. Pre-create the volume with the right ownership on the host, e.g.
40+
chown -R 10001:10001 /path/to/host/dir
41+
3. Run as root explicitly: docker run --user 0:0 ...
42+
43+
EOF
44+
exit 1
45+
fi
46+
47+
exec "$@"

0 commit comments

Comments
 (0)