Add queries to allocate external IPv6 addresses#9282
Conversation
dc77103 to
cdc8461
Compare
- Rewrite the `NextExternalIp` query to allow IPv6 address allocations. This uses queries more like the existing "next-item" queries based on a self-join, where we're joining the existing address with those addresses plus-one, and taking the first free one. - Handle a few more failure-cases from the new query to ensure we detect address exhaustion, reallocation, and so on. All the existing tests continue to pass. - Add expectorate / explain "tests" for the new queries - Add a few new tests, some specifically for IPv6 address allocations and the details / corner cases of the new query structure - Closes #9245 - Closes #1468 - Closes #1371
cdc8461 to
27cb16e
Compare
rcgoodfellow
left a comment
There was a problem hiding this comment.
Thanks @bnaecker. To the extent that I'm able to grok the database queries, things seem reasonable to me. The amount of complexity in doing this with SQL makes me wonder if we could do this using client side transactions having more of the logic in Rust to simplify things.
| project_id: Some(project_id), | ||
| explicit_ip: Some(explicit_ip.into()), | ||
| explicit_port_range: None, | ||
| explicit_port_range: Some((0, u16::MAX.into())), |
There was a problem hiding this comment.
Based on my reading of where this is used in NextExternalIp::walk_ast the new explicit value and None look like they are equivalent as the former will result in the automatic full ip subquery and the later will result in the explicit ip subquery, but since the latter here contains the entire port space those are equivalent? This seems like it's better in the sense that the function name implies explicit allocation and if automatic allocation changes later this could have a surprising butterfly effect. Just trying to make sure I understand the effect of the change.
There was a problem hiding this comment.
That's right. If we're doing an explicit allocation, we always know both the IP address and ports, so it seems simpler to enforce that at construction time.
|
Thanks @rcgoodfellow. I agree that an explicit transaction could be simpler in terms of the SQL, but my sense is that we'd pay for that with much more complicated surrounding Rust code. Not the worst tradeoff, but it felt like enough of a wash that I tried to improve the existing query instead. |
NextExternalIpquery to allow IPv6 address allocations. This uses queries more like the existing "next-item" queries based on a self-join, where we're joining the existing address with those addresses plus-one, and taking the first free one.