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

Limit size of the search range for VNIs when creating VPCs #4298

Merged
merged 1 commit into from
Oct 21, 2023

Conversation

bnaecker
Copy link
Collaborator

  • Fixes Memory budget exceeded error when attempting to create a new vpc #4283.
  • Adds a relatively small limit to the NextItem query used for finding a free VNI during VPC creation. This limits the memory consumption to something very reasonable, but is big enough that we should be extremely unlikely to find no available VNIs in the range.
  • Add an application-level retry loop when inserting customer VPCs, which catches the unlikely event that there really are no VNIs available, and retries a few times.
  • Adds tests for the computation of the limited search range.
  • Adds tests for the actual exhaustion-detection and retry behavior.

Copy link
Collaborator

@smklein smklein left a comment

Choose a reason for hiding this comment

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

Great tests!

nexus/db-queries/src/db/queries/vpc.rs Show resolved Hide resolved
nexus/db-model/src/vpc.rs Outdated Show resolved Hide resolved
nexus/db-queries/src/db/datastore/vpc.rs Outdated Show resolved Hide resolved
@bnaecker bnaecker requested a review from smklein October 20, 2023 18:36
@bnaecker
Copy link
Collaborator Author

Hmm looks like this hit the NTP test flake John noted earlier today. I’ll try the check again, but let me know if you’ve any suggestions in the meantime @smklein

- Fixes #4283.
- Adds a relatively small limit to the `NextItem` query used for
  finding a free VNI during VPC creation. This limits the memory
  consumption to something very reasonable, but is big enough that we
  should be extremely unlikely to find _no_ available VNIs in the range.
- Add an application-level retry loop when inserting _customer_ VPCs,
  which catches the unlikely event that there really are no VNIs
  available, and retries a few times.
- Adds tests for the computation of the limited search range.
- Adds tests for the actual exhaustion-detection and retry behavior.

Review feedback

- Add const-assert that VNI search range is valid
- Rename `Vpc::with_random_vni()`

Search entire VNI range in chunks

- Remove `IncompleteVpc::with_random_vni()`
- Add iterator to search ranges of VNIs sequentially, yielding all
  range starts in the valid guest VNI range from a random starting VNI.
- Instead of a limited retry loop when creating VPCs, search until the
  iterator is exhausted and we've searched the whole range.

Throw in some DTrace probes

fmt
@bnaecker bnaecker merged commit aef679c into main Oct 21, 2023
21 of 22 checks passed
@bnaecker bnaecker deleted the limit-vni-search-range branch October 21, 2023 17:28
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.

Memory budget exceeded error when attempting to create a new vpc
2 participants