Skip to content

Account for local storage during region allocation #9670

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.

Sign up for GitHub

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

Open
jmpesp wants to merge 1 commit into
base: main
Choose a base branch
from
+68 −22
Open

Account for local storage during region allocation #9670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 36 additions & 14 deletions nexus/db-queries/src/db/queries/region_allocation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,25 +262,47 @@ pub fn allocation_query(

let mut builder = QueryBuilder::new();

builder.sql(
// Find all old regions associated with a particular volume
"WITH
builder
Copy link
Collaborator

@smklein smklein Jan 16, 2026

Choose a reason for hiding this comment

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

We really should have at least one regression test for this!

.sql(
// Find all old regions associated with a particular volume
"WITH
old_regions AS (
SELECT ").sql(AllColumnsOfRegion::with_prefix("region")).sql("
FROM region WHERE (region.volume_id = ").param().sql(")),")
.bind::<sql_types::Uuid, _>(*volume_id.as_untyped_uuid())

// Calculates the old size being used by zpools under consideration as targets for region
// allocation.
.sql("
SELECT ",
)
.sql(AllColumnsOfRegion::with_prefix("region"))
.sql(
"
FROM region WHERE (region.volume_id = ",
)
.param()
.sql(")),")
.bind::<sql_types::Uuid, _>(*volume_id.as_untyped_uuid())
// Calculates the old size being used by zpools under consideration as
// targets for region allocation.
//
// Account for the local storage dataset rendezvous tables not having been
// created yet (or for the integration tests, where blueprint execution is
// currently disabled) by performing a LEFT JOIN on pool_id and a coalesce
// for the size_used column.
.sql(
"
old_zpool_usage AS (
SELECT
crucible_dataset.pool_id,
sum(crucible_dataset.size_used) AS size_used
FROM crucible_dataset
(
sum(crucible_dataset.size_used) +
sum(coalesce(rendezvous_local_storage_dataset.size_used, 0))
) AS size_used
FROM
crucible_dataset LEFT JOIN rendezvous_local_storage_dataset
Copy link
Collaborator

@smklein smklein Jan 16, 2026

Choose a reason for hiding this comment

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

By using a "LEFT JOIN" here, we're fully ignoring the local storage dataset in the case where "no crucible datasets have been allocated". That seems wrong.

Copy link
Collaborator

@smklein smklein Jan 16, 2026

Choose a reason for hiding this comment

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

(Also, seems unlikely I suppose? I guess all our current configs have "crucible, and may or may not have the rendezvous_local_storage_dataset -- but this still seems like defensive programming worth doing)

ON
crucible_dataset.pool_id = rendezvous_local_storage_dataset.pool_id AND
rendezvous_local_storage_dataset.time_tombstoned is NULL
WHERE
((crucible_dataset.size_used IS NOT NULL) AND (crucible_dataset.time_deleted IS NULL))
GROUP BY crucible_dataset.pool_id),");
crucible_dataset.size_used IS NOT NULL AND
crucible_dataset.time_deleted IS NULL
GROUP BY crucible_dataset.pool_id),",
);
Copy link
Collaborator

@smklein smklein Jan 16, 2026

Choose a reason for hiding this comment

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

I think this query is incorrectly calculating size, overcounting usage, which will inhibit our ability to actually allocate storage on the rack.

For example, if pool A has:

  • 3 crucible_datasets (c1, c2, c3) with sizes 100, 200, 300
  • 2 rendezvous_local_storage_datasets (r1, r2) with sizes 50, 60

The JOIN condition is "match on pool ID" -- every crucible dataset will match every local storage dataset, returning 6 rows:

  • c1, r1 (100 + 50)
  • c1, r2 (100 + 60)
  • c2, r1 (200 + 50)
  • c2, r2 (200 + 60)
  • c3, r1 (300 + 50)
  • c3, r2 (300 + 60)

I think the correct answer for space used should actually be: 100 + 200 + 300 + 50 + 60 = 710.
However, your query as-is more than double counts: I think the sum of all the rows in this example is 1530 instead.

Presumably we want to account for "each crucible dataset, once" and "each local storage dataset, once" within each pool, right?

Maybe try separating the two?

  old_zpool_usage AS (                                                                                                                                                                                                                        
    SELECT                                                                                                                                                                                                                                    
      COALESCE(crucible_usage.pool_id, local_usage.pool_id) AS pool_id,                                                                                                                                                                       
      COALESCE(crucible_usage.size_used, 0) + COALESCE(local_usage.size_used, 0) AS size_used                                                                                                                                                 
    FROM (                                                                                                                                                                                                                                    
      SELECT pool_id, sum(size_used) AS size_used                                                                                                                                                                                             
      FROM crucible_dataset                                                                                                                                                                                                                   
      WHERE size_used IS NOT NULL AND time_deleted IS NULL                                                                                                                                                                                    
      GROUP BY pool_id                                                                                                                                                                                                                        
    ) crucible_usage                                                                                                                                                                                                                          
    FULL OUTER JOIN (                                                                                                                                                                                                                         
      SELECT pool_id, sum(size_used) AS size_used                                                                                                                                                                                             
      FROM rendezvous_local_storage_dataset                                                                                                                                                                                                   
      WHERE time_tombstoned IS NULL                                                                                                                                                                                                           
      GROUP BY pool_id                                     
    ) local_usage                                          
    ON crucible_usage.pool_id = local_usage.pool_id        
  )

This also should be testable. We should have coverage for cases where we're making an allocation, we need a precise amount of space to fit, and we have...

  • A pool with only crucible datasets
  • A pool with only local storage
  • A pool with both
  • A pool with neither

Copy link
Collaborator

@smklein smklein Jan 16, 2026

Choose a reason for hiding this comment

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

welp, I'm not sure how I managed to overlook this - maybe got too in the weeds on this query - but as you pointed out in chat, we really should have "zero or one" of these datasets on a zpool, not multiple.

So although my complaint is accurate in an academic point-of-view, based on the schema, practically, we shouldn't have "a bunch of crucible datasets" all residing on one zpool


if let Some(snapshot_id) = snapshot_id {
// Any zpool already have this volume's existing regions, or host the
Expand Down
10 changes: 8 additions & 2 deletions nexus/db-queries/tests/output/region_allocate_distinct_sleds.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,17 @@ WITH
old_zpool_usage
AS (
SELECT
crucible_dataset.pool_id, sum(crucible_dataset.size_used) AS size_used
crucible_dataset.pool_id,
sum(crucible_dataset.size_used)
+ sum(COALESCE(rendezvous_local_storage_dataset.size_used, 0))
AS size_used
FROM
crucible_dataset
LEFT JOIN rendezvous_local_storage_dataset ON
crucible_dataset.pool_id = rendezvous_local_storage_dataset.pool_id
AND rendezvous_local_storage_dataset.time_tombstoned IS NULL
WHERE
(crucible_dataset.size_used IS NOT NULL) AND (crucible_dataset.time_deleted IS NULL)
crucible_dataset.size_used IS NOT NULL AND crucible_dataset.time_deleted IS NULL
GROUP BY
crucible_dataset.pool_id
),
Expand Down
10 changes: 8 additions & 2 deletions nexus/db-queries/tests/output/region_allocate_random_sleds.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,17 @@ WITH
old_zpool_usage
AS (
SELECT
crucible_dataset.pool_id, sum(crucible_dataset.size_used) AS size_used
crucible_dataset.pool_id,
sum(crucible_dataset.size_used)
+ sum(COALESCE(rendezvous_local_storage_dataset.size_used, 0))
AS size_used
FROM
crucible_dataset
LEFT JOIN rendezvous_local_storage_dataset ON
crucible_dataset.pool_id = rendezvous_local_storage_dataset.pool_id
AND rendezvous_local_storage_dataset.time_tombstoned IS NULL
WHERE
(crucible_dataset.size_used IS NOT NULL) AND (crucible_dataset.time_deleted IS NULL)
crucible_dataset.size_used IS NOT NULL AND crucible_dataset.time_deleted IS NULL
GROUP BY
crucible_dataset.pool_id
),
Expand Down
10 changes: 8 additions & 2 deletions nexus/db-queries/tests/output/region_allocate_with_snapshot_distinct_sleds.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,17 @@ WITH
old_zpool_usage
AS (
SELECT
crucible_dataset.pool_id, sum(crucible_dataset.size_used) AS size_used
crucible_dataset.pool_id,
sum(crucible_dataset.size_used)
+ sum(COALESCE(rendezvous_local_storage_dataset.size_used, 0))
AS size_used
FROM
crucible_dataset
LEFT JOIN rendezvous_local_storage_dataset ON
crucible_dataset.pool_id = rendezvous_local_storage_dataset.pool_id
AND rendezvous_local_storage_dataset.time_tombstoned IS NULL
WHERE
(crucible_dataset.size_used IS NOT NULL) AND (crucible_dataset.time_deleted IS NULL)
crucible_dataset.size_used IS NOT NULL AND crucible_dataset.time_deleted IS NULL
GROUP BY
crucible_dataset.pool_id
),
Expand Down
10 changes: 8 additions & 2 deletions nexus/db-queries/tests/output/region_allocate_with_snapshot_random_sleds.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,17 @@ WITH
old_zpool_usage
AS (
SELECT
crucible_dataset.pool_id, sum(crucible_dataset.size_used) AS size_used
crucible_dataset.pool_id,
sum(crucible_dataset.size_used)
+ sum(COALESCE(rendezvous_local_storage_dataset.size_used, 0))
AS size_used
FROM
crucible_dataset
LEFT JOIN rendezvous_local_storage_dataset ON
crucible_dataset.pool_id = rendezvous_local_storage_dataset.pool_id
AND rendezvous_local_storage_dataset.time_tombstoned IS NULL
WHERE
(crucible_dataset.size_used IS NOT NULL) AND (crucible_dataset.time_deleted IS NULL)
crucible_dataset.size_used IS NOT NULL AND crucible_dataset.time_deleted IS NULL
GROUP BY
crucible_dataset.pool_id
),
Expand Down
Loading