Skip to content

Conversation

@hc-github-team-nomad-core
Copy link
Contributor

Backport

This PR is auto-generated from #26712 to be assessed for backporting due to the inclusion of the label backport/1.10.x.

The below text is copied from the body of the original PR.


During a large volume dispatch load test, I discovered that a lot of the total scheduling time is being spent calling structs.ParsePortRanges repeatedly, in order to parse the reserved ports configuration of the node (ex. converting "80,8000-8001" to []int{80, 8000, 8001}). A close examination of the profiles shows that the bulk of the time is being spent hashing the keys for the map of ports we use for de-duplication, and then sorting the resulting slice.

The (*NetworkIndex) SetNode method that calls the offending ParsePortRanges merges all the ports into the UsedPorts map of bitmaps at scheduling time. Which means the consumer of the slice is already de-duplicating and doesn't care about the order. The only other caller of ParsePortRanges is when we validate the configuration file, and that throws away the slice entirely.

By skipping de-duplication and not sorting, we can cut down the runtime of this function by 30x and memory usage by 4x.

See my comment here for why memoizing the result proved to be impractical. This changeset also deletes the unused ParseReservedHostPorts method and moves its tests into the tests for ParsePortRanges (which is really what it was testing anyways).

Ref: https://github.com/hashicorp/nomad/blob/v1.10.4/nomad/structs/network.go#L201
Fixes: #26654


I'm going to reproduce the test I ran that I've described in #26654 but in the meantime, here's a microbenchmark:

func BenchmarkParsePortRangesOld(b *testing.B) {
       spec := "22,8000-9000"
       for b.Loop() {
               ParsePortRanges(spec)
       }
}

func BenchmarkParsePortRangesNew(b *testing.B) {
       spec := "22,8000-9000"
       for b.Loop() {
               ParsePortRangesNew(spec)
       }
}
$ go test -v -count=1 ./nomad/structs -benchmem -bench BenchmarkParsePortRanges -run=^#
goos: linux
goarch: amd64
pkg: github.com/hashicorp/nomad/nomad/structs
cpu: Intel(R) Core(TM) Ultra 7 165H
BenchmarkParsePortRangesOld
BenchmarkParsePortRangesOld-22              7041            149205 ns/op           99616 B/op         37 allocs/op
BenchmarkParsePortRangesNew
BenchmarkParsePortRangesNew-22            233440              4926 ns/op           25288 B/op         15 allocs/op
PASS
ok      github.com/hashicorp/nomad/nomad/structs        2.214s

Contributor Checklist

  • Changelog Entry If this PR changes user-facing behavior, please generate and add a
    changelog entry using the make cl command.
  • Testing Please add tests to cover any new functionality or to demonstrate bug fixes and
    ensure regressions will be caught.
  • Documentation If the change impacts user-facing functionality such as the CLI, API, UI,
    and job configuration, please update the Nomad website documentation to reflect this. Refer to
    the website README for docs guidelines. Please also consider whether the
    change requires notes within the upgrade guide.

Reviewer Checklist

  • Backport Labels Please add the correct backport labels as described by the internal
    backporting document.
  • Commit Type Ensure the correct merge method is selected which should be "squash and merge"
    in the majority of situations. The main exceptions are long-lived feature branches or merges where
    history should be preserved.
  • Enterprise PRs If this is an enterprise only PR, please add any required changelog entry
    within the public repository.
  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.


Overview of commits

@tgross tgross merged commit d64497c into release/1.10.x Sep 8, 2025
30 of 31 checks passed
@tgross tgross deleted the backport/NMD942-no-sorting-on-parseportranges/virtually-arriving-antelope branch September 8, 2025 17:26
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.

2 participants