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.

Sign up for GitHub

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

Adds SnapshotError::IoWithSourceAndFile #29527

Merged
merged 1 commit into from
Jan 5, 2023
Merged

Adds SnapshotError::IoWithSourceAndFile #29527

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 15 additions & 8 deletions runtime/src/snapshot_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,9 @@ pub enum SnapshotError {
#[error("source({1}) - I/O error: {0}")]
IoWithSource(std::io::Error, &'static str),

#[error("source({1}) - I/O error: {0}, file: {2}")]
IoWithSourceAndFile(#[source] std::io::Error, &'static str, PathBuf),

#[error("could not get file name from path: {}", .0.display())]
PathToFileNameError(PathBuf),

Expand Down Expand Up @@ -328,8 +331,9 @@ pub fn archive_snapshot_package(
.parent()
.expect("Tar output path is invalid");

fs::create_dir_all(tar_dir)
.map_err(|e| SnapshotError::IoWithSource(e, "create archive path"))?;
fs::create_dir_all(tar_dir).map_err(|e| {
SnapshotError::IoWithSourceAndFile(e, "create archive path", tar_dir.into())
})?;

// Create the staging directories
let staging_dir_prefix = TMP_SNAPSHOT_ARCHIVE_PREFIX;
Expand All @@ -345,8 +349,9 @@ pub fn archive_snapshot_package(
let staging_accounts_dir = staging_dir.path().join("accounts");
let staging_snapshots_dir = staging_dir.path().join("snapshots");
let staging_version_file = staging_dir.path().join("version");
fs::create_dir_all(&staging_accounts_dir)
.map_err(|e| SnapshotError::IoWithSource(e, "create staging path"))?;
fs::create_dir_all(&staging_accounts_dir).map_err(|e| {
SnapshotError::IoWithSourceAndFile(e, "create staging path", staging_accounts_dir.clone())
})?;

// Add the snapshots to the staging directory
symlink::symlink_dir(
Expand Down Expand Up @@ -377,8 +382,9 @@ pub fn archive_snapshot_package(

// Write version file
{
let mut f = fs::File::create(staging_version_file)
.map_err(|e| SnapshotError::IoWithSource(e, "create version file"))?;
let mut f = fs::File::create(&staging_version_file).map_err(|e| {
SnapshotError::IoWithSourceAndFile(e, "create version file", staging_version_file)
})?;
f.write_all(snapshot_package.snapshot_version.as_str().as_bytes())
.map_err(|e| SnapshotError::IoWithSource(e, "write version file"))?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this IoWithSource be updated to IoWithSourceAndFile?

Wondering if there is any reason to have IoWithSource exist without providing the path info.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we could also change this one. I didn't for this PR since we already know what the version file name is (maybe the path is also useful?).

I'm going to merge the PR as-is so that your PR (#29496) can use it. Subsequent PRs can update more code here to use SnapshotError::IoWithSourceAndFile as appropriate.

}
Expand Down Expand Up @@ -437,8 +443,9 @@ pub fn archive_snapshot_package(
}

// Atomically move the archive into position for other validators to find
let metadata = fs::metadata(&archive_path)
.map_err(|e| SnapshotError::IoWithSource(e, "archive path stat"))?;
let metadata = fs::metadata(&archive_path).map_err(|e| {
SnapshotError::IoWithSourceAndFile(e, "archive path stat", archive_path.clone())
})?;
fs::rename(&archive_path, snapshot_package.path())
.map_err(|e| SnapshotError::IoWithSource(e, "archive path rename"))?;

Expand Down