Skip to content

Convert Region Allocation to be a CTE#1733

Merged
smklein merged 18 commits into
mainfrom
region-cte
Sep 28, 2022
Merged

Convert Region Allocation to be a CTE#1733
smklein merged 18 commits into
mainfrom
region-cte

Conversation

@smklein

@smklein smklein commented Sep 22, 2022

Copy link
Copy Markdown
Collaborator

Fixes #1688

This PR can basically be read in two pieces - I could literally split it in two if that would be helpful, but figured I'd show "new utilities" with "how they are utilized for the region allocation use-case specifically".

Part 1: Utilities to build CTEs

nexus/src/db/subquery.rs - Adds the definition of traits like Subquery, as well as a CteBuilder to help building arbitrary CTEs.
nexus/db-macros/src/lib.rs - Adds macro to derive Subquery (identifying that a query can be used in a CTE arm)
nexus/db-macros/src/subquery.rs - Implementation of Subquery macro
nexus/src/db/alias.rs - Defines expression which allows field aliasing (FOO as BAR in SELECT/RETURNING queries)

Part 2: Using those utilities to implement the region allocation CTE

nexus/src/db/datastore/region.rs - Updates actual usage of region allocation query
nexus/src/db/queries/region_allocation.rs - Defines and implements region allocation CTE
nexus/db-model/src/queries/region_allocation.rs - Defines model of subqueries (using table! macro)

Note: This particular CTE is actually kinda long, because the transaction was doing a lot. In the future, however, we'll be able to simplify the CTE significantly by relying on quotas/reservations and db constraints to help enforce some of the size limits.

@smklein smklein requested a review from jmpesp September 23, 2022 12:01
@smklein smklein marked this pull request as ready for review September 23, 2022 12:01
@smklein smklein removed the request for review from jmpesp September 23, 2022 12:02
@smklein smklein requested review from bnaecker and jmpesp September 26, 2022 16:32

@jmpesp jmpesp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is awesome! A few questions, two suggestions, but none blocking.

Comment thread nexus/src/db/datastore/region.rs Outdated
Comment thread nexus/src/db/queries/region_allocation.rs Outdated
Comment thread nexus/src/db/queries/region_allocation.rs Outdated
Comment thread nexus/src/db/queries/region_allocation.rs
Comment thread nexus/src/db/queries/region_allocation.rs Outdated
@smklein

smklein commented Sep 28, 2022

Copy link
Copy Markdown
Collaborator Author

Just confirmed that this fixes the symptom of https://github.com/oxidecomputer/remote-access-preview/issues/23

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.

Region allocation should be a CTE

2 participants