Skip to content
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.

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

Do not 500 on snapshot delete for deleted region #1162

Merged
merged 2 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
39 changes: 37 additions & 2 deletions agent/src/datafile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,43 @@ impl DataFile {
path.push("regions");
path.push(request.id.0.clone());

let dataset =
ZFSDataset::new(path.into_os_string().into_string().unwrap())?;
let dataset = match ZFSDataset::new(
path.into_os_string().into_string().unwrap(),
) {
Ok(dataset) => dataset,
Err(_e) => {
// This branch can only be entered if `zfs list` for that
// dataset path failed to return anything.

// Did the region exist in the past, and was it already deleted?
if let Some(region) = inner.regions.get(&request.id) {
match region.state {
State::Tombstoned | State::Destroyed => {
// If so, any snapshots must have been deleted
// before the agent would allow the region to be
// deleted.
return Ok(());
}

State::Requested | State::Created => {
// This is a bug: according to the agent's datafile,
// the region exists, but according to zfs list, it
// does not
bail!("Agent thinks region {} exists but zfs list does not!", request.id.0);
}

State::Failed => {
// Something has set the region to state failed, so
// we can't delete this snapshot.
bail!("Region {} is in state failed", request.id.0);
}
}
} else {
// In here, the region never existed!
leftwo marked this conversation as resolved.
Show resolved Hide resolved
bail!("Inside region {} snapshot {} delete, region never existed!", request.id.0, request.name);
}
}
};

let snapshot_name = format!("{}@{}", dataset.dataset(), request.name);

Expand Down
5 changes: 4 additions & 1 deletion agent/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ pub struct ZFSDataset {
}

impl ZFSDataset {
// From either dataset name or path, create the ZFSDataset object
/// From either dataset name or path, create the ZFSDataset object.
///
/// Fails if the dataset name does not exist, or the path does not exist (or
/// belong to a dataset).
pub fn new(dataset: String) -> Result<ZFSDataset> {
// Validate the argument is a dataset
let cmd = std::process::Command::new("zfs")
Expand Down
Loading