Ensures datasets get propagated into RSS blueprint #7000

smklein · 2024-11-05T23:22:45Z

Ensures that RSS actually tracks datasets and adds them to the initial blueprint
Ensures that RSS adds all datasets to CockroachDB, including the Debug dataset, Zone filesystems, etc
Ensures that RSS sets the datasets during initialization

smklein · 2024-11-05T23:24:51Z

openapi/nexus-internal.json

@@ -3128,6 +3128,7 @@
        "type": "object",
        "properties": {
          "address": {
+            "nullable": true,


Before this PR: The public-facing API to "add a dataset to the DB" required a SocketAddr type.

This is an artifact which was partially fixed by #2000 - unfortunately, there is code which still uses these IP/port pairs if they exist, but they are not required to exist for a dataset record to be created.

Anyway - TL;DR, they're optional, as they should be.

smklein · 2024-11-05T23:25:33Z

sled-agent/src/rack_setup/plan/service.rs

+impl SledConfig {
+    /// Adds a zone to the Sled's configuration, as well as any number of
+    /// durable datasets.
+    pub fn add_zone_and_datasets(&mut self, zone: BlueprintZoneConfig) {


This is RSS's version of "whenever you add a zone, make sure you track all possible datasets used by that zone too".

smklein · 2024-11-05T23:27:02Z

sled-agent/src/rack_setup/plan/service.rs

@@ -119,6 +124,54 @@ pub struct SledConfig {
    pub zones: Vec<BlueprintZoneConfig>,


Struggling to comment the right line here -- I'm trying to comment on the pub datasets: DatasetsConfig line above -- but basically:

We added this field in [nexus] Manage datasets via reconfigurator #6229

but we never populated it correctly

That's what this PR is most significantly trying to fix

smklein · 2024-11-05T23:27:52Z

sled-agent/src/rack_setup/service.rs

-                .await
+                .await?;
+
+                // Ensure all datasets are initialized


This is the equivalent of the "blueprint executor", as controlled by RSS. "Request datasets before requesting zones".

smklein · 2024-11-06T01:32:43Z

sled-agent/src/rack_setup/service.rs

-                            .pool_name
-                            .id()
-                            .into_untyped_uuid(),
-                        dataset_id: zone.id.into_untyped_uuid(),


This was nasty - we re-used the zone UUID as a dataset UUID 😬

We aren't doing that anymore - datasets have their own Uuids that are provisioned as they are created within the plan.

andrewjstone

This looks great. Just one bit of confusion from me.

andrewjstone · 2024-11-06T19:00:14Z

common/src/disk.rs

@@ -107,6 +107,7 @@ impl DatasetName {
        &self.pool_name
    }

+    // TODO: Maybe rename this to "kind"?


👍 I figure this will have ripple effects which is why you didn't do it in this PR.

Yeah, it's a mechanical change, so I figured I'd punt it out when I get to a less conflict-y place

andrewjstone · 2024-11-06T19:21:53Z

sled-agent/src/rack_setup/service.rs

+                >(());
+            };
+
+            if let sled_agent_client::Error::ErrorResponse(response) = &error {


I'm failing to see how this could happen during an initial rack setup. We won't handoff to nexus until after RSS, which we won't do until we finish calling this method. The only way this can fail in this scenario is if we call RSS again after the rack is already initialized. That is gated elsewhere though and we shouldn't even get here right?

I think this may have been a result of copying "how we initialize zones" to "disks" and now to "datasets".

I've removed this in 6f6a87c - any conflict is now a hard error, for both disk and dataset initialization.

smklein · 2024-11-06T19:38:32Z

Tested this in a4x2, saw the following:

The root blueprint has all datasets
The blueprint diff shows no change in zones. I can try to merge [nexus] Expand BlueprintDiff to identify changed datasets #6481 and confirm there are no changes in the datasets as well.

smklein · 2024-11-08T22:40:25Z

Okay, took this for a spin again on a4x2. Turns out, investing in the blueprint diff logic in #6481 was worthwhile.

Here's what I did:

Spun up Nexus
Observed a blueprint from RSS (bp1)
Ran omdb nexus blueprints regenerate to make bp2
Ran omdb nexus blueprints diff bp1 bp1

The results? They're almost the same, but turns out they have differing opinions on the compression level:

*   oxp_3a4d0477-79cc-417e-a2b4-ee1cd472e899/crypt/debug                                                           82fb3783-d55a-4335-8f14-09288f0081f3   100 GiB   none          - gzip-9   
     └─                                                                                                                                                                           + off      
*   oxp_59a3fa5a-e598-43a1-a67c-4b7149faf65b/crypt/debug                                                           0c038f13-e88e-485c-89ae-037df52b5fa0   100 GiB   none          - gzip-9   
     └─                                                                                                                                                                           + off      
*   oxp_83972e7f-2bfc-432b-9414-f2d0b0604713/crypt/debug                                                           53559bae-12ec-491c-9380-17cad82bf2a6   100 GiB   none          - gzip-9   
     └─                                                                                                                                                                           + off      
*   oxp_849647bf-82ee-452a-86b6-a5a230b6c897/crypt/debug                                                           87d4d31a-1eed-447e-a2fc-9ffeeef91a73   100 GiB   none          - gzip-9   
     └─                                                                                                                                                                           + off      
*   oxp_ffe80101-a099-477b-b06e-7e26f3d92ed1/crypt/debug                                                           21e4641e-8665-40af-ac98-5d55e44ef7b4   100 GiB   none          - gzip-9   
     └─                                                                                                                                                                           + off

Seems that's because the Sled Agent initializes this dataset with Gzip-9:

omicron/sled-storage/src/dataset.rs

Lines 67 to 70 in 15305ae

    
           // For storing full kernel RAM dumps 
        
           ExpectedDataset::new(DUMP_DATASET) 
        
               .quota(DUMP_DATASET_QUOTA) 
        
               .compression(DUMP_DATASET_COMPRESSION),

But the reconfigurator is hard-coded to use CompressionAlgorithm::Off:

omicron/nexus/reconfigurator/planning/src/blueprint_builder/builder.rs

Line 897 in 15305ae

CompressionAlgorithm::Off,

In 084d6f8 , I went ahead and updated this to be "gzip-9" everywhere.

End result: This RSS-generated blueprint should now be identical to one immediately generated by Nexus.

smklein added 4 commits November 4, 2024 18:06

Use local properties when querying oxide values

73bd549

Better dataset population during RSS

2f448f0

adding debug, zone datasets too

a7c6f1b

Merge branch 'main' into rss-datasets

a62b283

smklein commented Nov 5, 2024

View reviewed changes

smklein mentioned this pull request Nov 5, 2024

PUT /omicron-zone -- stop provisioning datasets! #6991

Open

Add tests for plan creation, ensuring datasets are populated

9a9fe45

smklein marked this pull request as ready for review November 6, 2024 01:31

smklein commented Nov 6, 2024

View reviewed changes

smklein requested a review from andrewjstone November 6, 2024 01:52

andrewjstone reviewed Nov 6, 2024

View reviewed changes

smklein added 8 commits November 6, 2024 17:28

Merge branch 'main' into rss-datasets

5a92eac

Merge branch 'main' into rss-datasets

2aa4a0e

Merge branch 'main' into rss-datasets

6506ce0

merge

b100385

fmt

784e377

Merge branch 'main' into rss-datasets

96bf17f

Bail on generation number conflict

6f6a87c

More consistent usage of Gzip-9 for debug dataset

084d6f8

smklein added 2 commits November 8, 2024 14:41

Merge branch 'main' into rss-datasets

03ea021

EXPECTORATE

1ae6ba7

andrewjstone approved these changes Nov 12, 2024

View reviewed changes

smklein merged commit e8e5dd9 into main Nov 12, 2024
17 checks passed

smklein deleted the rss-datasets branch November 12, 2024 18:23

jgallagher mentioned this pull request Nov 13, 2024

Use a typed UUID for Dataset #7048

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensures datasets get propagated into RSS blueprint #7000

Ensures datasets get propagated into RSS blueprint #7000

smklein commented Nov 5, 2024 •

edited

Loading

smklein Nov 5, 2024

smklein Nov 5, 2024

smklein Nov 5, 2024

smklein Nov 5, 2024

smklein Nov 6, 2024

andrewjstone left a comment

andrewjstone Nov 6, 2024

smklein Nov 7, 2024

andrewjstone Nov 6, 2024

smklein Nov 8, 2024

smklein commented Nov 6, 2024

smklein commented Nov 8, 2024

		@@ -119,6 +124,54 @@ pub struct SledConfig {
		pub zones: Vec<BlueprintZoneConfig>,

Ensures datasets get propagated into RSS blueprint #7000

Ensures datasets get propagated into RSS blueprint #7000

Conversation

smklein commented Nov 5, 2024 • 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

andrewjstone 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

Choose a reason for hiding this comment

smklein commented Nov 6, 2024

smklein commented Nov 8, 2024

smklein commented Nov 5, 2024 •

edited

Loading