-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
create and canonicalize account_paths #32037
Changes from 4 commits
3efbf08
4157433
151f3c0
04e8f94
954cc4b
286a1ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,8 @@ use { | |
runtime_config::RuntimeConfig, | ||
snapshot_config::{SnapshotConfig, SnapshotUsage}, | ||
snapshot_utils::{ | ||
self, create_all_accounts_run_and_snapshot_dirs, ArchiveFormat, SnapshotVersion, | ||
self, create_all_accounts_run_and_snapshot_dirs, create_and_canonicalize_directories, | ||
steviez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ArchiveFormat, SnapshotVersion, | ||
}, | ||
}, | ||
solana_sdk::{ | ||
|
@@ -963,8 +964,7 @@ pub fn main() { | |
.map(BlockstoreRecoveryMode::from); | ||
|
||
// Canonicalize ledger path to avoid issues with symlink creation | ||
let _ = fs::create_dir_all(&ledger_path); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting that we would ignore errors here... Seems correct to not ignore those errors in the new code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. History: #7877 🤔 maybe some concerns about permissions? It's possible we have a Agree it seems better to not ignore the errors. |
||
let ledger_path = fs::canonicalize(&ledger_path).unwrap_or_else(|err| { | ||
let ledger_path = create_and_canonicalize_directories(&[ledger_path]).unwrap_or_else(|err| { | ||
eprintln!("Unable to access ledger path: {err:?}"); | ||
apfitzge marked this conversation as resolved.
Show resolved
Hide resolved
|
||
exit(1); | ||
}); | ||
|
@@ -1396,6 +1396,12 @@ pub fn main() { | |
} else { | ||
vec![ledger_path.join("accounts")] | ||
}; | ||
let account_paths = snapshot_utils::create_and_canonicalize_directories(&account_paths) | ||
.unwrap_or_else(|err| { | ||
eprintln!("Unable to access account path: {err:?}"); | ||
apfitzge marked this conversation as resolved.
Show resolved
Hide resolved
|
||
exit(1); | ||
}); | ||
|
||
let account_shrink_paths: Option<Vec<PathBuf>> = | ||
values_t!(matches, "account_shrink_path", String) | ||
.map(|shrink_paths| shrink_paths.into_iter().map(PathBuf::from).collect()) | ||
|
@@ -1413,20 +1419,10 @@ pub fn main() { | |
validator_config.account_snapshot_paths = account_snapshot_paths; | ||
|
||
validator_config.account_shrink_paths = account_shrink_paths.map(|paths| { | ||
paths | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we were doing the same thing in 2 places, just moved it to a function so we can also use this in ledger-tool when I get to checking that out. |
||
.into_iter() | ||
.map(|account_path| { | ||
match fs::create_dir_all(&account_path) | ||
.and_then(|_| fs::canonicalize(&account_path)) | ||
{ | ||
Ok(account_path) => account_path, | ||
Err(err) => { | ||
eprintln!("Unable to access account path: {account_path:?}, err: {err:?}"); | ||
exit(1); | ||
} | ||
} | ||
}) | ||
.collect() | ||
create_and_canonicalize_directories(&paths).unwrap_or_else(|err| { | ||
eprintln!("Unable to access account shrink path: {err:?}"); | ||
apfitzge marked this conversation as resolved.
Show resolved
Hide resolved
|
||
exit(1); | ||
}) | ||
}); | ||
|
||
let maximum_local_snapshot_age = value_t_or_exit!(matches, "maximum_local_snapshot_age", u64); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wasn't super happy about this returning a
SnapshotError
, since it's only somewhat related to snapshots. Somewhat of a hack because I want to keep info about which path caused the error which afaik isn't easy to do w/ IoError