Add quota to agent created datasets#835
Merged
Merged
Conversation
Contributor
Author
|
After a discussion with @ahl , I've updated the comments a bit, and changed the quota to be larger |
ahl
approved these changes
Jul 15, 2023
Comment on lines
+17
to
+18
| * As a safety mechanism to prevent the agent from creating more regions | ||
| * than we have physical space, we using a zfs reservation on each dataset |
Contributor
There was a problem hiding this comment.
Suggested change
| * As a safety mechanism to prevent the agent from creating more regions | |
| * than we have physical space, we using a zfs reservation on each dataset | |
| * As a safety mechanism to prevent the agent from allocating more space to regions | |
| * than we have physical space on disk, we using a zfs reservation on each dataset |
Comment on lines
+19
to
+21
| * we create. While not a complete solution, it does help in some | ||
| * situations to avoid allocation of a region that we don't have the | ||
| * space for if that region was to fill up. |
Contributor
There was a problem hiding this comment.
Suggested change
| * we create. While not a complete solution, it does help in some | |
| * situations to avoid allocation of a region that we don't have the | |
| * space for if that region was to fill up. | |
| * we create. While it does prevent us from allocating more space to regions than the disk could accommodate, it is not a complete solution in that we ideally would like Nexus to track allocations (for crucible and other significant datasets), an adequate capacity margin to ensure that disk performance doesn't become pathological, and monitors any over-use for appropriate mitigation (migration, termination, etc). |
Comment on lines
+27
to
+29
| * In addition, we throw a quota of 3x the region size. If the region | ||
| * has grown that big, then something is wrong and we should prevent it | ||
| * from growing any larger and impacting other regions in the dataset. |
Contributor
There was a problem hiding this comment.
Suggested change
| * In addition, we throw a quota of 3x the region size. If the region | |
| * has grown that big, then something is wrong and we should prevent it | |
| * from growing any larger and impacting other regions in the dataset. | |
| * In addition, we throw a quota of 3x the region size. The failure modes of exhausting the quota are potentially severe and not (as yet) well-tested so we want some buffer between the reservation and the quota. If the region | |
| * has grown that big however, something is potentially very wrong and we should prevent it | |
| * from growing any larger and impacting other regions on the disk. |
Comment on lines
+31
to
+32
| const METADATA_BUFFER: f64 = 1.25; | ||
| const REGION_QUOTA: u64 = 3; |
Contributor
There was a problem hiding this comment.
maybe: RESERVATION_FACTOR and QUOTA_FACTOR? I leave it to you... just a thought
Contributor
Author
There was a problem hiding this comment.
Oh, yeah, I tried RESERVATION_MULTIPLIER and that felt wrong.
| } | ||
|
|
||
| // If requested, set a quota and reservation here. | ||
| if let Some(reservation) = reservation { |
Contributor
There was a problem hiding this comment.
I'd suggest we might want something like this:
let mut cmd = std::process::Command::new("zfs")
.arg("create");
if let Some(reservation) = reservation {
cmd = cmd.arg("-o").arg(format!("reservation={}", reservation));
}
cmd = cmd
.arg(&dataset)
.output()?;i.e. I think you can create the dataset with the properties preset.
Contributor
Author
There was a problem hiding this comment.
Yeah, I like it. Fewer lines of code and easier cleanup.
added 2 commits
July 15, 2023 07:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds a quota and a reservation to crucible datasets created when a region
is created in the crucible agent. A somewhat arbitrary .25 overhead is reserved
for each region. 17% is the largest possible space the metadata will take, and this
gives us a little room in the unlikely event that all regions are full.
This does not consider snapshot overhead.
For issue #605