Skip to content

Stop CpuSet.NodeSetsAreNonEmpty assuming every NUMA node has CPUs - #86

Merged
graphicsMan merged 1 commit into
mainfrom
fix/cpu-set-numa-node-without-cpus
Sep 2, 2026
Merged

graphicsMan merged 1 commit into
mainfrom
fix/cpu-set-numa-node-without-cpus

Conversation

@graphicsMan

@graphicsMan graphicsMan commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Fixes #89.

The test asserts that every NUMA node reports at least one CPU:

for (int32_t i = 0; i < numNodes; ++i) {
  EXPECT_GT(CpuSet::node(i).count(), 0) << "NUMA node " << i << " has no CPUs";
}

That is not true in general. A NUMA node with no CPUs is legal, and at least two
common cases produce one: memory-only nodes (CXL-attached memory, persistent
memory) and nodes whose CPUs have all been taken offline. Both report an empty
cpulist.

On such a host the test fails with NUMA node 1 has no CPUs while CpuSet is
reporting the topology entirely correctly — the failure is in the test's
assumption, not the library.

This sums across nodes and asserts the total is positive, which is the property
actually worth holding: that node enumeration finds CPUs somewhere. The
stronger claim the test appears to be reaching for, that the node sets partition
all(), is already covered by AllSetCoversAllNodeSets.

The test asserted that every NUMA node reports at least one CPU. That is
not true in general: memory-only nodes (CXL-attached memory, persistent
memory) and nodes whose CPUs are all offline both report an empty
cpulist. On such a host the test fails with "NUMA node 1 has no CPUs"
while CpuSet is reporting the topology entirely correctly.

Sum CPUs across nodes and assert the total is positive, which is the
property actually worth holding. The stronger claim the test was
reaching for -- that the node sets partition all() -- is already covered
by AllSetCoversAllNodeSets.
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 2, 2026
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.0%. Comparing base (d74898a) to head (f8a17b8).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main     #86     +/-   ##
=======================================
+ Coverage   92.6%   93.0%   +0.3%     
=======================================
  Files         64      64             
  Lines       4990    4990             
  Branches     678     682      +4     
=======================================
+ Hits        4625    4643     +18     
+ Misses       365     347     -18     

see 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@graphicsMan
graphicsMan merged commit 4ad0abb into main Sep 2, 2026
26 checks passed
@graphicsMan
graphicsMan deleted the fix/cpu-set-numa-node-without-cpus branch September 2, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CpuSet.NodeSetsAreNonEmpty fails on hosts with a CPU-less NUMA node

1 participant