Add resource limits #4605

zephraph · 2023-12-04T14:32:44Z

This PR aims to introduce quotas as a concept into Nexus for allowing operators to enforce virtual resource limits at the silo level. The initial implementation will be limited to checks during instance start, disk creation, and snapshot creation. We will not being doing advanced quota recalculation as system resources change. We will not yet be enforcing intelligent quota caps where the sum of all quotas must be less than the theoretical available system virtual resources.

The implementation of this functionality is shaped by RFD-427 but some desired functionality will be deferred given time/complexity constraints.

Longer term I believe the shape of quotas and perhaps even their relationship to silos may change. This PR implements a simplified version that matches closely to how the virtual resource provisioning tables are already built out. I know there's some oddness around the shape of the quotas table with it not having its own ID and otherwise being mildly divergent from other resources, but this was largely to ensure we could migrate to another solution and not overcomplicate the initial implementation.

TODO

Add quota creation as a step of silo creation
Add initialization checks in CTEs for instance create, etc to only proceed when quota unmet
Wire up CTE sentinels in upstream callsites
Add backfill migration for existing customers
Add tests for quota enforcement
Delete the quotas when the silo is deleted

In RFD-427 we specify that for the initial implementation we want to require that all silos have a quota. In considering the API and implementaiton more carefully I decided that at least temporarily we should drop create/delete given that deletions should never happen and creations should only happen when the silo is created (or via some internal process which creates quotas for pre-existing silos)

There will be a follow up PR to add capacity/utilization to the API both at the silo and system levels. Given that, and the general lack of actionability of quotas to silo users, I've just dropped the quota view from the API.

zephraph · 2023-12-06T05:17:52Z

nexus/db-queries/src/db/queries/virtual_provisioning_collection_update.rs

+                return external::Error::InvalidRequest { message: "Insufficient Capacity: Not enough CPUs to complete request. Either stop unused instances to free up resources or contact the rack operator to request a capacity increase.".to_string() }
+            }


Eventually it would be nice to use @sunshowers's InsufficientCapacity error from #4573

On second thought, maybe not. I think we should distinguish between hitting up against human set capacity limits vs hardware limits.

@zephraph and I chatted about this and post- this is what I suggested post-#4573:

Have the InsufficientCapacity variant carry around an enum, e.g. InsufficientCapacityKind::UserQuota and ::System, and the insufficient_capacity constructor accept that as an argument (forcing callers to specify which InsufficientCapacityKind variant they want)

In the message, include the kind: "Insufficient capacity (user quota)" and "... (system)"

Consider returning different HTTP error codes for each variant.

cc @askfongjojo who I think has some thoughts about this.

@sunshowers @zephraph: Thanks for looping me in. Once silo quotas are fully in place, user should rarely hit the System capacity error, assuming that the sum of all silo quotas should not exceed the total usable capacity of the rack. But I can think of two situations when the System capacity error would be hit before the UserQuota one:

Some sleds are in maintenance or taken out of the provisioning pool

All sleds are relatively full and the user is trying to provision a very large VM.

As such, perhaps the UserQuota kind error will be a HTTP 400 (end user needs to free up some capacity themselves) whereas the System one is a 507 (operator needs to migrate some instances, check the sled status, force some silos to reduce their usage temporarily, etc.)?

Useful distinction. I think we should definitely make the difference clear in the error code. I don't know if they should have different status codes, though. I can imagine a situation where the system quota violation is also user-correctable by freeing up enough stuff. Or, like you say, if the instance they're trying to create is simply very large.

For now I'm just using the InsufficientCapacity error wholesale. This isn't the ideal state, it would be good to follow up with a better error state, but it's sufficient for the time being.

I created an issue to denote the need for better error messages: #4680

zephraph · 2023-12-08T07:53:23Z

nexus/tests/output/nexus_tags.txt

 silo_user_list                           GET      /v1/system/users
 silo_user_view                           GET      /v1/system/users/{user_id}
 silo_view                                GET      /v1/system/silos/{silo}
+system_quotas_list                       GET      /v1/system/silo-quotas


I renamed this endpoint from /v1/system/quotas to /v1/system/silo-quotas just b/c it's going under the silos tag now (as per @ahl's suggestion) and it seemed to fit better. I will likely just delete this endpoint in the capacity and utilization branch in favor of an endpoint that returns both quotas and provisioned counts.

nexus/types/src/external_api/params.rs

askfongjojo · 2023-12-12T21:35:40Z

schema/crdb/20.0.0/up02.sql

+  -- ~70% of 128 threads leaving 30% for internal use
+  90 AS cpus,
+  -- 708 GiB (~70% of memory leaving 30% for internal use)
+  760209211392 AS memory_bytes,
+  -- 850 GiB (total storage / 3.5)
+  912680550400 AS storage_bytes


There are on ongoing discussions about abandoning the use of "assumed" usable capacity figures. But if we still want to use them, these numbers should be multiplied by the number of sleds and physical disks (excluding the M2s). So a half-rack will provide 90*16 = 1440 threads, and so on.

This is replaced with an arbitrarily high limit, significantly more than can be provisioned on even a full rack.

smklein

This looks great, thank you for taking on the heroic effort of tackling this. The CTE and DB migration code make sense to me -- this cut all the way across Nexus, and looks great.

A few comments below, but the only thing I'd consider a "blocker" is the from_sled_count function, which is IMO kinda misleading. Everything else is just nits!

nexus/types/src/external_api/params.rs

schema/crdb/20.0.0/up02.sql

nexus/types/src/external_api/params.rs

nexus/db-queries/src/db/queries/virtual_provisioning_collection_update.rs

nexus/tests/integration_tests/quotas.rs

…n_update.rs Co-authored-by: Sean Klein <[email protected]>

david-crespo

Unbelievable work. The core diesel bit is wild. Had a couple of comments, nothing important.

david-crespo · 2023-12-13T03:57:55Z

nexus/db-model/src/quota.rs

+            storage: silo_quotas.storage.into(),
+        }
+    }
+}


Why would you need to go from view to model? Is this used?

david-crespo · 2023-12-13T03:59:26Z

schema/crdb/20.0.0/up02.sql

+-- Adds quotas for any existing silos without them. 
+-- The selected quotas are based on the resources of a half rack
+-- with 30% CPU and memory reserved for internal use and a 3.5x tax
+-- on storage for replication, etc.


Comment no longer accurate I think 😂

david-crespo · 2023-12-13T04:06:55Z

nexus/db-queries/src/db/datastore/silo.rs

+            .transaction_async(|conn| async move {
+                diesel::insert_into(quotas_dsl::silo_quotas)
+                    .values(SiloQuotas::arbitrarily_high_default(
+                        DEFAULT_SILO.id(),


Could use a comment on the possibly-surprising-in-the-future fact that we do this for default silo but not internal silo.

This PR is a follow up to #4605 which adds views into capacity and utilization both at the silo and system level. API: |op|method|url| |--|--|--| |silo_utilization_list | GET | /v1/system/utilization/silos | |silo_utilization_view | GET | /v1/system/utilization/silos/{silo} | |utilization_view | GET | /v1/utilization | I'm not entirely satisfied w/ the silo utilization endpoints. They could be this instead: |op|method|url| |--|--|--| |silo_utilization_list | GET | /v1/system/silos-utilization | |silo_utilization_view | GET | /v1/system/silos/{silo}/utilization | Also take special note of the views ```rust // For the eyes of end users /// View of the current silo's resource utilization and capacity #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct Utilization { /// Accounts for resources allocated to running instances or storage allocated via disks or snapshots /// Note that CPU and memory resources associated with a stopped instances are not counted here /// whereas associated disks will still be counted pub provisioned: VirtualResourceCounts, /// The total amount of resources that can be provisioned in this silo /// Actions that would exceed this limit will fail pub capacity: VirtualResourceCounts, } // For the eyes of an operator /// View of a silo's resource utilization and capacity #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct SiloUtilization { pub silo_id: Uuid, pub silo_name: Name, /// Accounts for resources allocated by in silos like CPU or memory for running instances and storage for disks and snapshots /// Note that CPU and memory resources associated with a stopped instances are not counted here pub provisioned: VirtualResourceCounts, /// Accounts for the total amount of resources reserved for silos via their quotas pub allocated: VirtualResourceCounts, } ``` For users in the silo I use `provisioned` and `capacity` as the language. Their `capacity` is represented by the quota set by an operator. For the operator `provisioned` is the same but `allocated` is used to denote the amount of resources allotted via quotas. --- Note: I had planned to add a full system utilization endpoint to this PR but that would increase the scope. Instead will ship that API as a part of the next release. We can calculate some version of the full system utilization on the client by listing all the silos and their utilization. --------- Co-authored-by: Sean Klein <[email protected]>

zephraph added 6 commits November 20, 2023 23:05

Outline basic CRUD actions for quotas

b59092c

Start outlining quota database type

21ba99c

Simplify quota model to be flatter

0127dd1

Rough API shape of resource quotas

5c50ce5

Create quotas during silo creation

3346a33

zephraph mentioned this pull request Dec 4, 2023

Tracking Issue: Resource Allocation (Silo) #4554

Closed

zephraph added 4 commits December 4, 2023 13:36

Update comment for internal silo

501f5ac

Merge branch 'main' into add-resource-limits

1894eb3

Drop inner silo quota view

9e51431

There will be a follow up PR to add capacity/utilization to the API both at the silo and system levels. Given that, and the general lack of actionability of quotas to silo users, I've just dropped the quota view from the API.

Convert storage/memory to bytecount instead of ints

caf371b

zephraph mentioned this pull request Dec 6, 2023

[nexus] improve external messages and make more available to clients #4573

Merged

zephraph force-pushed the add-resource-limits branch 2 times, most recently from 0afe397 to e34542b Compare December 6, 2023 01:48

WIP quota check CTE

96877b2

zephraph force-pushed the add-resource-limits branch from e34542b to 96877b2 Compare December 6, 2023 01:48

zephraph added 8 commits December 5, 2023 21:10

Add quota checks for cpu, memory, storage

8025899

Join do_update and quota_check into a single CTE step

a4002ff

Try (and fail) to resolve CTE type issues

76c722f

Resolve CTE compliation issues... ???

f8e2afc

Convert quota detlas to sql types; fix borrow issues

3d4ebe2

Merge branch 'main' into add-resource-limits

d317629

Remove a todo

dcf974e

Wire up quota limit error handling

83c9872

zephraph commented Dec 6, 2023

View reviewed changes

Fix missing quota specifier

7e779a0

zephraph requested review from ahl, smklein, david-crespo and augustuswm December 6, 2023 16:11

Bump schema version

2c11210

zephraph commented Dec 8, 2023

View reviewed changes

nexus/types/src/external_api/params.rs Outdated Show resolved Hide resolved

zephraph added 7 commits December 11, 2023 14:37

Merge branch 'main' into add-resource-limits

025cd84

Use insufficient capacity error

8a7794b

half_rack -> from_sled_count

f8955a5

Merge branch 'main' into add-resource-limits

77f6d29

Merge branch 'main' into add-resource-limits

760675d

Fix db deadlock caused by checking authz before silo is created

371bc98

Add quotas for silo cert tests

de951a7

zephraph mentioned this pull request Dec 12, 2023

Add a better error for when a user tries to provision past their quota #4680

Open

Fix type failures

410d3ae

smklein self-assigned this Dec 12, 2023

Fix docs build

cd22388

askfongjojo reviewed Dec 12, 2023

View reviewed changes

smklein approved these changes Dec 12, 2023

View reviewed changes

smklein removed their assignment Dec 12, 2023

zephraph and others added 6 commits December 12, 2023 16:51

Update nexus/db-queries/src/db/queries/virtual_provisioning_collectio…

e86ec72

…n_update.rs Co-authored-by: Sean Klein <[email protected]>

Use an arbitrarily high value over something that looks real but isn't

5974325

Enhance tests to check errors; fix overflow

c602e29

Drop expects for unwraps when we don't expect them to fail

fe87ca8

Update comment

11ab326

Fix clippy errors

e369851

zephraph enabled auto-merge (squash) December 13, 2023 01:15

david-crespo approved these changes Dec 13, 2023

View reviewed changes

ci: add quota setup to deploy test

635d1ba

zephraph merged commit 877a886 into main Dec 13, 2023
21 checks passed

zephraph deleted the add-resource-limits branch December 13, 2023 06:19

zephraph mentioned this pull request Dec 14, 2023

Capacity and utilization #4696

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add resource limits #4605

Add resource limits #4605

zephraph commented Dec 4, 2023 •

edited

Loading

zephraph Dec 6, 2023 •

edited

Loading

zephraph Dec 8, 2023

sunshowers Dec 9, 2023 •

edited

Loading

askfongjojo Dec 10, 2023

david-crespo Dec 11, 2023

zephraph Dec 11, 2023

zephraph Dec 12, 2023

zephraph Dec 8, 2023

askfongjojo Dec 12, 2023 •

edited

Loading

zephraph Dec 13, 2023

smklein left a comment

david-crespo left a comment

david-crespo Dec 13, 2023

david-crespo Dec 13, 2023

david-crespo Dec 13, 2023

		return external::Error::InvalidRequest { message: "Insufficient Capacity: Not enough CPUs to complete request. Either stop unused instances to free up resources or contact the rack operator to request a capacity increase.".to_string() }
		}

Add resource limits #4605

Add resource limits #4605

Conversation

zephraph commented Dec 4, 2023 • edited Loading

TODO

zephraph Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunshowers Dec 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

askfongjojo Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smklein left a comment

Choose a reason for hiding this comment

david-crespo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zephraph commented Dec 4, 2023 •

edited

Loading

zephraph Dec 6, 2023 •

edited

Loading

sunshowers Dec 9, 2023 •

edited

Loading

askfongjojo Dec 12, 2023 •

edited

Loading