Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: move NUMA platform scan out of testing global #23289

Merged
merged 1 commit into from
Jun 11, 2024
Merged

Conversation

tgross
Copy link
Member

@tgross tgross commented Jun 10, 2024

The testing.go test helpers file for the driver manager initializes the NUMA scan as a package-global variable. This causes it to be pulled in even in production builds, so even running commands like nomad version will cause the NUMA scan to happen. Move the scan into the test helper setup.

I discovered this while debugging #23284 but this suggests there may be other cases where our testing.go test helper files are dragging wasteful startup behaviors into production builds.


Before:

$ objdump -t $(which nomad) | grep topology
00000000011096e0 l     F .text  00000000000003d0              github.com/hashicorp/nomad/nomad/structs.topologyFromLegacyGeneric
0000000001109e60 l     F .text  00000000000000a5              github.com/hashicorp/nomad/nomad/structs.topologyFromLegacy
0000000001109f20 l     F .text  0000000000000314              github.com/hashicorp/nomad/nomad/structs.topologyFromLegacyLinux
000000000110a240 l     F .text  0000000000000066              github.com/hashicorp/nomad/nomad/structs.topologyFromLegacyLinux.func1
0000000004ba9560 l     O .bss   0000000000000008              github.com/hashicorp/nomad/client/pluginmanager/drivermanager.topology

After:

$ objdump -t $(which nomad) | grep topology
00000000011096e0 l     F .text  00000000000003d0              github.com/hashicorp/nomad/nomad/structs.topologyFromLegacyGeneric
0000000001109e60 l     F .text  00000000000000a5              github.com/hashicorp/nomad/nomad/structs.topologyFromLegacy
0000000001109f20 l     F .text  0000000000000314              github.com/hashicorp/nomad/nomad/structs.topologyFromLegacyLinux
000000000110a240 l     F .text  0000000000000066              github.com/hashicorp/nomad/nomad/structs.topologyFromLegacyLinux.func1

The `testing.go` test helpers file for the driver manager initializes the NUMA
scan as a package-global variable. This causes it to be pulled in even in
production builds, so even running commands like `nomad version` will cause the
NUMA scan to happen. Move the scan into the test helper setup.
@tgross tgross added this to the 1.8.1 milestone Jun 10, 2024
@tgross tgross added backport/ent/1.8.x+ent Changes are backported to 1.8.x+ent backport/1.8.x backport to 1.8.x release line backport/ent/1.7.x+ent Changes are backported to 1.7.x+ent type/bug labels Jun 10, 2024
@tgross tgross marked this pull request as ready for review June 10, 2024 20:53
Copy link
Contributor

@pkazmierczak pkazmierczak left a comment

Choose a reason for hiding this comment

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

LGTM!

@tgross tgross merged commit 61608e4 into main Jun 11, 2024
31 checks passed
@tgross tgross deleted the testing-no-scan-numa branch June 11, 2024 12:52
Copy link

github-actions bot commented Jan 4, 2025

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/ent/1.7.x+ent Changes are backported to 1.7.x+ent backport/ent/1.8.x+ent Changes are backported to 1.8.x+ent backport/1.8.x backport to 1.8.x release line type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants