[sforster-meteomatics]

Description

When preemption is enabled for at least one scheduler, the network state can inflate dramatically as available ports are duplicated over and over again, on each evaluation that checks if preempting a lower priority allocation can make a higher priorioity allocation fit.
This happens here since there is no deduplication of the ports:
https://github.com/hashicorp/nomad/blob/main/nomad/structs/structs.go#L2957-L2963

This PR fixes that bug by performing a deep copy on the Flattened object in ComparableResources.

Let me know if I should change anything about the tests I added. I tried to keep the change itself as minimal as possible, to avoid any unnecessary side effects.

Testing & Reproduction steps

I created a repo with the aid of Codex to reproduce the issue in a docker compose environment.

https://github.com/sforster-meteomatics/nomad-preemption-lab

git clone https://github.com/hashicorp/nomad.git nomad-src
git -C nomad-src checkout --detach fd2b0dea13c7144970e8408689cfa34edd1b7389

bash scripts/run-tests.sh
bash scripts/run-tests.sh --patched
bash scripts/run-tests.sh --continuous
bash scripts/run-tests.sh --patched --continuous

Links

https://github.com/hashicorp/nomad/blob/main/nomad/structs/structs.go#L2957-L2963

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 not applicable to this change
• LLM Usage If an LLM was used to generate any code, please ensure and confirm you have read
  and followed the AI usage 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.

No changes to security controls were performed.

[hashicorp-cla-app]

All committers have signed the CLA.

[mismithhisler]

We probably also want to copy shared via c.Shared.Copy().

[sforster-meteomatics]

I looked at that as well, but it could affect how AllocatedSharedResources.Subtract() works. Since after the deep copy, it wouldn't match the Networks that are actually the same anymore.
https://github.com/hashicorp/nomad/blob/main/nomad/structs/structs.go#L4008-L4025

I'm not sure if anything currently relies on that behaviour, but in any case I didn't want to affect it and keep this change minimal. And the current change with only copying Flattened is enough to fix the bug that I found.

[mismithhisler]

Thanks for this contribution! It's worth noting that this ComparableResources struct is incredibly costly because of all it's memory allocations. These additional copies are necessary, but will only make it more costly. A longer term fix is proposed in #28340 which removes ComparableResources entirely in favor of smaller more purpose build structs.

[sforster-meteomatics]

Thanks for this contribution! It's worth noting that this ComparableResources struct is incredibly costly because of all it's memory allocations. These additional copies are a necessary, but will only make it more costly. A longer term fix is proposed in #28340 which removes ComparableResources entirely in favor of smaller more purpose build structs.

I can try and measure the performance impact. If it's not too high, it might anyway be worth it to include this fix in an upcoming release, since the long-term fix you shared might take longer to be finished? Of course that's up to the Nomad team though.