Adds background task which automatically removes zone bundles #3868

bnaecker · 2023-08-10T21:16:50Z

Adds a background task to the sled agent, running zone_bundle::cleanup_task. That periodically combs through all the zone bundles, and removes "low priority" ones to remain within a percentage of the storage dataset quota.
Adds tools for controlling the priority of a bundle. This currently sorts bundles by their timestamp and cause, where for example explicitly requested bundles have the highest priority.
Adds RPC-like mechanism for asking the cleanup task to report usage information, the number of bytes consumed in the storage directories, for zone bundles.
Adds RPC and sled-agent API for updating the cleanup context, including period of auto cleanups, priority order, and the percentage of the dataset quota allowed.
Adds RPC and sled agent API for making explicit request to trigger a cleanup.

bnaecker · 2023-08-11T22:08:01Z

This is the last piece of #1598. It adds mechanisms for cleaning up zone bundles, both on-demand and periodically in a background task run by the sled-agent. The zone-bundle tool now features new subcommands for inspecting and manipulating the cleanup context, and triggering a cleanup manually.

I've added a bunch of tests of the cleanup behavior itself, but here are a few highlights from the perspective of the zone-bundle tool.

The cleanup context

The cleanup task maintains some context about how to do its job. This includes the directories it's looking at; the period on which it runs; the priority for the bundles; and the percentage of the dataset's quota that its limits zone-bundle storage to:

bnaecker@shale : ~/omicron $ ./target/release/zone-bundle --host fd00:1122:3344:101::1 context
Storage directories: ["/pool/int/a462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone", "/pool/int/b462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone"]
Period: 6000s
Priority: [Cause, Time]
Storage limit: 1%

zone-bundle can update the latter three of these using the set-context subcommand:

bnaecker@shale : ~/omicron $ ./target/release/zone-bundle --host fd00:1122:3344:101::1 set-context --help
Set parameters of the zone bundle cleanup context

Usage: zone-bundle set-cleanup-context <--period <PERIOD>|--priority <PRIORITY>|--storage-limit <STORAGE_LIMIT>>

Options:
      --period <PERIOD>
          The new period on which to run automatic cleanups, in seconds

      --priority <PRIORITY>
          The new order used to determine priority when cleaning up bundles

      --storage-limit <STORAGE_LIMIT>
          The limit on the underlying dataset quota allowed for zone bundles.

          This should be expressed as percentage of the dataset quota.

  -h, --help
          Print help (see a summary with '-h')

Usage information

The cleanup task can also fetch and report the disk space consumed by bundles. I added this since the task already needs to compute it, and it's easy enough to report:

bnaecker@shale : ~/omicron $ ./target/release/zone-bundle --host fd00:1122:3344:101::1 usage
Directory                                                        Bytes used       Bytes available  Dataset quota    % of available$ % of quota
/pool/int/a462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 5.44 KiB         10.24 KiB        1.00 MiB         53%            0%
/pool/int/b462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 5.44 KiB         10.24 KiB        1.00 MiB         53%            0%
bnaecker@shale : ~/omicron $
bnaecker@shale : ~/omicron $
bnaecker@shale : ~/omicron $ ./target/release/zone-bundle --host fd00:1122:3344:101::1 usage
Directory                                                        Bytes used       Bytes available  Dataset quota    % of available$ % of quota
/pool/int/a462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 5.44 KiB         10.24 KiB        1.00 MiB         53%            0%
/pool/int/b462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 5.44 KiB         10.24 KiB        1.00 MiB         53%            0%
bnaecker@shale : ~/omicron $ ./target/release/zone-bundle --host fd00:1122:3344:101::1 create oxz_crucible_6d9e3de9-ed01-46f8-a17d-54f6d0d72ff2
Created zone bundle: oxz_crucible_6d9e3de9-ed01-46f8-a17d-54f6d0d72ff2/82a75372-135a-4817-90f6-d792b5eb0e6b
bnaecker@shale : ~/omicron $ ./target/release/zone-bundle --host fd00:1122:3344:101::1 usage
Directory                                                        Bytes used       Bytes available  Dataset quota    % of available$ % of quota
/pool/int/b462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 10.86 KiB        10.24 KiB        1.00 MiB         106%           1%
/pool/int/a462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 10.86 KiB        10.24 KiB        1.00 MiB         106%           1%

We can see that the percentage of the available storage limit can go above 100%, since bundles are cleaned up periodically, rather than as they're created.

Triggering a cleanup

The cleanup task will periodically run, but it can also be explicitly triggered by the API:

bnaecker@shale : ~/omicron $ ./target/release/zone-bundle --host fd00:1122:3344:101::1 usage
Directory                                                        Bytes used       Bytes available  Dataset quota    % of available$ % of quota
/pool/int/b462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 10.86 KiB        10.24 KiB        1.00 MiB         106%           1%
/pool/int/a462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 10.86 KiB        10.24 KiB        1.00 MiB         106%           1%
bnaecker@shale : ~/omicron $ ./target/release/zone-bundle --host fd00:1122:3344:101::1 ls
Zone                                                             Bundle ID                            Created
oxz_crucible_6d9e3de9-ed01-46f8-a17d-54f6d0d72ff2                0655285a-6163-4319-bc5d-600fec679386 2023-08-11 21:13:19.944230245 UTC
oxz_crucible_6d9e3de9-ed01-46f8-a17d-54f6d0d72ff2                82a75372-135a-4817-90f6-d792b5eb0e6b 2023-08-11 21:56:46.245772954 UTC
bnaecker@shale : ~/omicron $ ./target/release/zone-bundle --host fd00:1122:3344:101::1 cleanup
Directory                                                        Count Bytes
/pool/int/b462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 1     5561
/pool/int/a462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 1     5561
bnaecker@shale : ~/omicron $ ./target/release/zone-bundle --host fd00:1122:3344:101::1 usage
Directory                                                        Bytes used       Bytes available  Dataset quota    % of available$ % of quota
/pool/int/b462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 5.43 KiB         10.24 KiB        1.00 MiB         52%            0%
/pool/int/a462a7f7-b628-40fe-80ff-4e4189e2d62b/debug/bundle/zone 5.43 KiB         10.24 KiB        1.00 MiB         52%            0%
bnaecker@shale : ~/omicron $ ./target/release/zone-bundle --host fd00:1122:3344:101::1 ls
Zone                                                             Bundle ID                            Created
oxz_crucible_6d9e3de9-ed01-46f8-a17d-54f6d0d72ff2                82a75372-135a-4817-90f6-d792b5eb0e6b 2023-08-11 21:56:46.245772954 UTC

So we can see that this pruned the oldest zone bundle. Users can also select a priority based on the cause for a zone bundle. For example, bundles are created automatically when a guest instance is destroyed, or in response to explicit requests from the API. The latter are higher-priority than the former, and so are preferentially preserved. Users can change the priority to be based on time, then cause, if they wish, which would prune the older bundles even if they were created in response to an explicit user request. (The default is cause, then time.)

bnaecker · 2023-08-14T15:25:17Z

@smklein @jgallagher Unless y'all have already started, I'd like to ask that you wait a bit for reviews. I've been mulling this over, and although the functionality is good, I'm not really satisfied with the interface. Too much duplication, and the message-passing is based on a different end-state I had in mind when I started. I'm going to package up more of the functionality in a type and pass that around to the folks that need it.

smklein · 2023-08-14T16:27:03Z

@smklein @jgallagher Unless y'all have already started, I'd like to ask that you wait a bit for reviews. I've been mulling this over, and although the functionality is good, I'm not really satisfied with the interface. Too much duplication, and the message-passing is based on a different end-state I had in mind when I started. I'm going to package up more of the functionality in a type and pass that around to the folks that need it.

I got halfway through a review on friday -- I'll shoot out the draft comments I had, you can take 'em or leave em.

Lemme know when to re-review!

openapi/sled-agent.json

schema/zone-bundle-metadata.json

sled-agent/src/http_entrypoints.rs

sled-agent/src/bin/zone-bundle.rs

smklein · 2023-08-12T02:11:13Z

sled-agent/src/sled_agent.rs

+        let cleanup_context = zone_bundle::CleanupContext::with_defaults(
+            &storage.resources().all_zone_bundle_directories().await,
+        )?;


Hrm... my only beef with this construction is that it's a point-in-time sampling of all disks, but that may change over the lifetime of a sled.

What if we hot-plug a U.2? Shouldn't we pass a reference to storage.resources() to the cleanup_task, so it can detect and use this new disk?

So, the zone bundles are currently on the M.2s. We talked about this a while ago, but the goal was just to sidestep a bunch of issues with storing them on the U.2s, one of which is exactly this. Imagine you unplug a U.2; you do something like pick a new U.2 on which to store zone bundles; and then the U.2 is reinserted. The cleanup task needs to know about all of those locations, since it may need to remove bundles from a dataset that's no longer used to store bundles. It just gets wonky, and I think we can avoid a lot of that by storing them on the M.2s at this point.

I'm happy to pass a reference to the storage manager or resources right now though. Even if we still keep them on the M.2s, it'll be easier to generalize in the future.

The new ZoneBundler object takes a reference to the StorageResources on creation now, in a70596c. It ensures that all the directories it needs exist whenever they're accessed. I can't be sure how that'll play with hot-swapped U.2s in the future, but this should be sufficient while bundles live on the M.2s.

sled-agent/src/zone_bundle.rs

bnaecker · 2023-08-15T21:55:02Z

@smklein @jgallagher I've done a good bit of rework in a70596c, so this is now ready for review. There's less sprawl in the code, since everyone uses a ZoneBundler to create / list bundles, rather than needing to know which datasets should be used. Hopefully it's a bit clearer, and thanks for your patience!

bnaecker · 2023-08-15T21:55:52Z

I see there are some conflicts now. I'll rebase and squash after review, to avoid messing up the diff during the process.

sled-agent/src/instance.rs

sled-agent/src/zone_bundle.rs

jgallagher · 2023-08-17T16:50:58Z

sled-agent/src/zone_bundle.rs

+        debug!(log, "checking path as zone bundle"; "path" => path.display());
+        match extract_zone_bundle_metadata(path.clone()).await {
+            Ok(md) => {
+                trace!(log, "extracted zone bundle metadata"; "metadata" => ?md);


Related to the comment about debug logs - do we have any means of building a sled-agent with trace logs enabled? I realize they're harmless / compiled out, but if there's here it seems like we might want to see them in some context?

I believe we build slog with release_max_level_debug, so I don't think there will ever be trace messages. I am personally of the opinion that we should build with trace, and filter at runtime with a LevelFilter. We're already doing the level filter to remove debug messages on main, since the level filter's value comes from the sled agent's config.toml.

If we do the above, then we can still get the trace messages even with the level filter in place, using DTrace. If you compile them out, you can't get them with DTrace. But I think that's a larger discussion, since it might have a big impact. I guess we'd have to measure to know.

sled-agent/src/zone_bundle.rs

bnaecker · 2023-08-17T19:59:39Z

Thanks for all the detailed feedback @jgallagher, helpful as always. I think I've addressed most things in 3f6c926, but please let me know if there are things I missed.

jgallagher

Changes look great, thanks!

bnaecker · 2023-08-17T20:31:43Z

Rebasing on main to resolve conflicts, then I'll push and merge

- Adds a background task to the sled agent, running `zone_bundle::cleanup_task`. That periodically combs through all the zone bundles, and removes "low priority" ones to remain within a percentage of the storage dataset quota. - Adds tools for controlling the priority of a bundle. This currently sorts bundles by their timestamp and cause, where for example explicitly requested bundles have the highest priority. - Adds RPC-like mechanism for asking the cleanup task to report usage information, the number of bytes consumed in the storage directories, for zone bundles. - Adds RPC and sled-agent API for updating the cleanup context, including period of auto cleanups, priority order, and the percentage of the dataset quota allowed. - Adds RPC and sled agent API for making explicit request to trigger a cleanup. - Adds a bunch of tests around the priority ordering and cleanup RPC mechanics Wrap zone bundling into a single type - Adds the `ZoneBundler` for managing the creation and listing of zones, and running background cleanup task. This is created pretty early, by the `StorageManager`, and cloned to the other objects that need access to bundles. - Move public free functions from the `zone_bundle` crate into methods on the `ZoneBundler`, and call them from the instance / service managers and sled agent. - Make the "list-all-zones" endpoint in the sled agent accept a filter directly and propagate to the listing routines inside the bundler. This removes filtering on the client. - Additional tests around listing / filtering. Review feedback - Add more error variants, to better maintain context and the actual error chains. - Remove zone-specific commmands - Remove hand-rolled `PartialOrd` implementation for simplicity, but add comment about the variant order being load-bearing. - Add the handle for the cleanup task to the `ZoneBundler`, and abort it on drop.

bnaecker · 2023-08-17T20:35:52Z

Resolves #1598

bnaecker force-pushed the remove-zone-bundles branch 2 times, most recently from 857ce8d to 3948179 Compare August 10, 2023 22:25

jordanhendricks added the Sled Agent Related to the Per-Sled Configuration and Management label Aug 11, 2023

bnaecker force-pushed the remove-zone-bundles branch from 3948179 to 63686af Compare August 11, 2023 20:18

bnaecker mentioned this pull request Aug 11, 2023

Zone bundle creation can fail, leaving unremovable files #3876

Open

bnaecker force-pushed the remove-zone-bundles branch 2 times, most recently from 9eb6174 to 372bf69 Compare August 11, 2023 21:58

bnaecker requested review from smklein and jgallagher August 11, 2023 22:08

bnaecker force-pushed the remove-zone-bundles branch 2 times, most recently from 09eb0a1 to 5165fc8 Compare August 11, 2023 22:20

smklein reviewed Aug 14, 2023

View reviewed changes

bnaecker requested a review from smklein August 15, 2023 22:08

jgallagher reviewed Aug 17, 2023

View reviewed changes

bnaecker requested a review from jgallagher August 17, 2023 19:59

jgallagher approved these changes Aug 17, 2023

View reviewed changes

bnaecker force-pushed the remove-zone-bundles branch from 3f6c926 to d3f6481 Compare August 17, 2023 20:35

bnaecker enabled auto-merge (squash) August 17, 2023 20:35

bnaecker merged commit a93314d into main Aug 17, 2023

bnaecker deleted the remove-zone-bundles branch August 17, 2023 22:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds background task which automatically removes zone bundles #3868

Adds background task which automatically removes zone bundles #3868

bnaecker commented Aug 10, 2023

bnaecker commented Aug 11, 2023

bnaecker commented Aug 14, 2023

smklein commented Aug 14, 2023

smklein Aug 12, 2023

bnaecker Aug 14, 2023

bnaecker Aug 15, 2023

bnaecker commented Aug 15, 2023

bnaecker commented Aug 15, 2023

jgallagher Aug 17, 2023

bnaecker Aug 17, 2023

bnaecker commented Aug 17, 2023

jgallagher left a comment

bnaecker commented Aug 17, 2023

bnaecker commented Aug 17, 2023

Adds background task which automatically removes zone bundles #3868

Adds background task which automatically removes zone bundles #3868

Conversation

bnaecker commented Aug 10, 2023

bnaecker commented Aug 11, 2023

The cleanup context

Usage information

Triggering a cleanup

bnaecker commented Aug 14, 2023

smklein commented Aug 14, 2023

smklein Aug 12, 2023

Choose a reason for hiding this comment

bnaecker Aug 14, 2023

Choose a reason for hiding this comment

bnaecker Aug 15, 2023

Choose a reason for hiding this comment

bnaecker commented Aug 15, 2023

bnaecker commented Aug 15, 2023

jgallagher Aug 17, 2023

Choose a reason for hiding this comment

bnaecker Aug 17, 2023

Choose a reason for hiding this comment

bnaecker commented Aug 17, 2023

jgallagher left a comment

Choose a reason for hiding this comment

bnaecker commented Aug 17, 2023

bnaecker commented Aug 17, 2023