Skip to content

Conversation

@bnaecker
Copy link
Collaborator

  • Creates the omicron_nexus::db::queries submodule, for custom /
    complex queries, organized around specific resources
  • Adds a general-purpose NextItem query, which looks for the first
    available item in a database column, with a defined starting point and
    maximum offset / scan size. This can be used to implement a linear
    search over a defined space. One of the benefits is that for
    randomly-selected items, we can avoid a retry loop, by instead
    starting from the random base item and inserting the first successful
    value. There are several places where we use a similar query, or use a
    retry loop that could be replaced by this query.
  • Use the NextGuestIpv4Address query when creating NICs.
  • Use the NextNicSlot query when creating guest NICs

@bnaecker bnaecker requested review from davepacheco and smklein May 11, 2022 04:46
@bnaecker
Copy link
Collaborator Author

bnaecker commented May 11, 2022

This PR is still pretty early, but I'd love to start getting some feedback. For context, this grew out of some work on #963. When we create MAC addresses for guests, we currently just fail the request if the address conflicts with an existing one. I started solving this the way we handle other such failures, with a retry loop. Even doing that, I came again across a familiar database query for picking the "next available item". That looks like this.

I decided to bite the bullet and implement a more generic type for building these kinds of queries, which is here. The idea is to describe the table and column we're searching, plus a starting point and maximum search size. I've implemented concrete versions of this currently for the IPv4 address and slot allocation for a guest network interface. If we're happy with where this goes, this can also be used to selection of:

  • Guest NIC MAC addresses
  • Guest NIC IPv6 addresses
  • VPC Subnet IPv6 prefixes
  • VPC VNIs
  • VPC IPv6 /48 prefixes

I didn't do any of those here, since I'd like to get some feedback first. The key point is that I believe this allows us to avoid retrying the query in a loop in the application, and the attendant complexity and performance costs.

One point to note is related to #959. In that case, we have a specific single item that we'd like to avoid in this allocation query. I have a good idea of how to implement that, by basically including a black-list of items and using something like a WHERE <item> NOT IN <blacklist> clause in the generated query. I'm not sure how well that performs, but for a handful of blacklist items it's probably acceptable.

Also note that a lot of the diff is reorganization. A number of bespoke queries were ending up in omicron_nexus::db::subnet_allocation, and I decided to split that out into resource / query-specific submodules under omicron_nexus::db::queries. To be concrete, omicron_nexus::db::subnet_allocation is now split into omicron_nexus::db::queries::{vpc_subnet,network_interface}. Those are unchanged, except for using the new NextItem query code internally.

@bnaecker bnaecker force-pushed the generic-next-item-query branch from 8e59c30 to a99069a Compare May 11, 2022 05:08
@bnaecker bnaecker force-pushed the generic-next-item-query branch from a99069a to bf7e531 Compare May 19, 2022 04:28
bnaecker added 2 commits May 19, 2022 17:16
- Creates the `omicron_nexus::db::queries` submodule, for custom /
  complex queries, organized around specific resources
- Adds a general-purpose `NextItem` query, which looks for the first
  available item in a database column, with a defined starting point and
  maximum offset / scan size. This can be used to implement a linear
  search over a defined space. One of the benefits is that for
  randomly-selected items, we can avoid a retry loop, by instead
  starting from the random base item and inserting the first successful
  value. There are several places where we use a similar query, or use a
  retry loop that could be replaced by this query.
- Use the `NextGuestIpv4Address` query when creating NICs.
- Use the `NextNicSlot` query when creating guest NICs
@bnaecker bnaecker force-pushed the generic-next-item-query branch from 0353950 to ae9a9b3 Compare May 19, 2022 17:17
@bnaecker bnaecker enabled auto-merge (squash) May 19, 2022 17:17
@bnaecker bnaecker merged commit ca46034 into main May 19, 2022
@bnaecker bnaecker deleted the generic-next-item-query branch May 19, 2022 18:11
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.

3 participants