Skip to content

Commit e772737

Browse files
genevievehelselmeta-codesync[bot]
authored andcommitted
remove: add retries to client directory removal
Summary: While debugging `sl worktree add` failures, one failure came across as a failed eden removal -> corrupt eden state -> failed worktree add. The user seemed to have ran `eden doctor` which got them to a better state, but I'd like to try to address the root issue. Their eden removal failed with (https://fburl.com/scuba/edenfs_cli_usage/ogclkwez) : ``` Failed to remove client config directory for {removal_target}: Directory not empty (os error 39) ``` as per: https://doc.rust-lang.org/std/fs/fn.remove_dir_all.html#errors > This function may return `io::ErrorKind::DirectoryNotEmpty` if the directory is concurrently written into, which typically indicates some contents were removed but not all. `io::ErrorKind::NotFound` is only returned if no removal occurs. On the host I investigated, there was only a `local` directory there, which was half removed. So there must have been some outstanding overlay writes happening while the repo was being removed: P2265496617 This diff migrates to a retrying removal (used already in this file elsewhere), which gives us a better chance of a removal if there are racing writes. As a followup too, we probably want to treat this as non-fatal and continue removal. I suspect we will see more instances of removals happening with other processes using the repo, so these best effort retries might not be enough. I think its fair to honor the removal over the potential usage of a repo. Reviewed By: janezhang10 Differential Revision: D99915460 fbshipit-source-id: b27b20725a7335ae0ec3eb50721e2395612bc0a7
1 parent 639a39d commit e772737

File tree

1 file changed

+2
-1
lines changed
  • eden/fs/cli_rs/edenfs-commands/src/remove

1 file changed

+2
-1
lines changed

eden/fs/cli_rs/edenfs-commands/src/remove/utils.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ use crate::get_edenfs_instance;
2525

2626
pub fn remove_client_config_dir(context: &RemoveContext) -> Result<()> {
2727
let instance = get_edenfs_instance();
28+
let client_dir = instance.client_dir_for_mount_point(&context.canonical_path)?;
2829

29-
match fs::remove_dir_all(instance.client_dir_for_mount_point(&context.canonical_path)?) {
30+
match forcefully_remove_dir_all(&client_dir) {
3031
Ok(_) => Ok(()),
3132
Err(e) if e.kind() == ErrorKind::NotFound => Ok(()),
3233
Err(e) => Err(anyhow!(

0 commit comments

Comments
 (0)