mirror of
https://github.com/martinvonz/jj.git
synced 2024-12-25 05:29:39 +00:00
fallback: replace use of backoff
crate of by own implementation
https://rustsec.org/advisories/RUSTSEC-2024-0384 says to migrate off of the `instant` crate because it's unmaintained. We depend on it only via the `backoff` crate. That crate also seems unmaintained. So this patch replaces our use of `backoff` by a custom implementation. I initially intended to migrate to the `backon` crate, but that made `lock::tests::lock_concurrent` tests fail. The test case spawns 72 threads (on my machine) and lets them all lock a file, and then it waits 1 millisecond before releasing the file lock. I think the problem is that their version of jitter is implemented as a random addition of up to the initial backoff value. In our case, that means we would add at most a millisecond. The `backoff` crate, on the other hand does it by adding -50% to +50% of the current backoff value. I think that leads to a much better distribution of backoffs, while `backon`'s implementation means that only a few threads can lock the file for each backoff exponent.
This commit is contained in:
parent
6739dccc6d
commit
02486dc064
4 changed files with 54 additions and 56 deletions
21
Cargo.lock
generated
21
Cargo.lock
generated
|
@ -178,17 +178,6 @@ version = "1.3.0"
|
|||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "0c4b4d0bd25bd0b74681c0ad21497610ce1b7c91b1022cd21c80c6fbdd9476b0"
|
||||
|
||||
[[package]]
|
||||
name = "backoff"
|
||||
version = "0.4.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "b62ddb9cb1ec0a098ad4bbf9344d0713fa193ae1a80af55febcff2627b6a00c1"
|
||||
dependencies = [
|
||||
"getrandom",
|
||||
"instant",
|
||||
"rand",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "backtrace"
|
||||
version = "0.3.73"
|
||||
|
@ -1737,15 +1726,6 @@ dependencies = [
|
|||
"similar",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "instant"
|
||||
version = "0.1.13"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e0242819d153cba4b4b05a5a8f2a7e9bbf97b6055b2a002b395c96b5ff3c0222"
|
||||
dependencies = [
|
||||
"cfg-if",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "is-terminal"
|
||||
version = "0.4.13"
|
||||
|
@ -1893,7 +1873,6 @@ version = "0.23.0"
|
|||
dependencies = [
|
||||
"assert_matches",
|
||||
"async-trait",
|
||||
"backoff",
|
||||
"blake2",
|
||||
"bstr",
|
||||
"chrono",
|
||||
|
|
|
@ -21,7 +21,6 @@ anyhow = "1.0.93"
|
|||
assert_cmd = "2.0.8"
|
||||
assert_matches = "1.5.0"
|
||||
async-trait = "0.1.83"
|
||||
backoff = "0.4.0"
|
||||
blake2 = "0.10.6"
|
||||
bstr = "1.10.0"
|
||||
clap = { version = "4.5.20", features = [
|
||||
|
|
|
@ -34,7 +34,6 @@ version_check = { workspace = true }
|
|||
|
||||
[dependencies]
|
||||
async-trait = { workspace = true }
|
||||
backoff = { workspace = true }
|
||||
blake2 = { workspace = true }
|
||||
bstr = { workspace = true }
|
||||
chrono = { workspace = true }
|
||||
|
|
|
@ -17,8 +17,6 @@ use std::fs::OpenOptions;
|
|||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
|
||||
use backoff::retry;
|
||||
use backoff::ExponentialBackoff;
|
||||
use tracing::instrument;
|
||||
|
||||
pub struct FileLock {
|
||||
|
@ -26,42 +24,65 @@ pub struct FileLock {
|
|||
_file: File,
|
||||
}
|
||||
|
||||
struct BackoffIterator {
|
||||
next_sleep_secs: f32,
|
||||
elapsed_secs: f32,
|
||||
}
|
||||
|
||||
impl BackoffIterator {
|
||||
fn new() -> Self {
|
||||
Self {
|
||||
next_sleep_secs: 0.001,
|
||||
elapsed_secs: 0.0,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Iterator for BackoffIterator {
|
||||
type Item = Duration;
|
||||
|
||||
fn next(&mut self) -> Option<Self::Item> {
|
||||
if self.elapsed_secs >= 10.0 {
|
||||
None
|
||||
} else {
|
||||
let current_sleep = self.next_sleep_secs * (rand::random::<f32>() + 0.5);
|
||||
self.next_sleep_secs *= 1.5;
|
||||
self.elapsed_secs += current_sleep;
|
||||
return Some(Duration::from_secs_f32(current_sleep));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl FileLock {
|
||||
pub fn lock(path: PathBuf) -> FileLock {
|
||||
let mut options = OpenOptions::new();
|
||||
options.create_new(true);
|
||||
options.write(true);
|
||||
let try_write_lock_file = || match options.open(&path) {
|
||||
Ok(file) => Ok(FileLock {
|
||||
path: path.clone(),
|
||||
_file: file,
|
||||
}),
|
||||
Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {
|
||||
Err(backoff::Error::Transient {
|
||||
err,
|
||||
retry_after: None,
|
||||
})
|
||||
let mut backoff_iterator = BackoffIterator::new();
|
||||
loop {
|
||||
match options.open(&path) {
|
||||
Ok(file) => {
|
||||
return FileLock { path, _file: file };
|
||||
}
|
||||
Err(err)
|
||||
if err.kind() == std::io::ErrorKind::AlreadyExists
|
||||
|| (cfg!(windows)
|
||||
&& err.kind() == std::io::ErrorKind::PermissionDenied) =>
|
||||
{
|
||||
if let Some(duration) = backoff_iterator.next() {
|
||||
std::thread::sleep(duration);
|
||||
} else {
|
||||
panic!(
|
||||
"Timed out while trying to create lock file {}: {}",
|
||||
path.display(),
|
||||
err
|
||||
);
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
panic!("Failed to create lock file {}: {}", path.display(), err);
|
||||
}
|
||||
}
|
||||
Err(err) if cfg!(windows) && err.kind() == std::io::ErrorKind::PermissionDenied => {
|
||||
Err(backoff::Error::Transient {
|
||||
err,
|
||||
retry_after: None,
|
||||
})
|
||||
}
|
||||
Err(err) => Err(backoff::Error::Permanent(err)),
|
||||
};
|
||||
let backoff = ExponentialBackoff {
|
||||
initial_interval: Duration::from_millis(1),
|
||||
max_elapsed_time: Some(Duration::from_secs(10)),
|
||||
..Default::default()
|
||||
};
|
||||
match retry(backoff, try_write_lock_file) {
|
||||
Err(err) => panic!(
|
||||
"failed to create lock file {}: {}",
|
||||
path.to_string_lossy(),
|
||||
err
|
||||
),
|
||||
Ok(file_lock) => file_lock,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -69,6 +90,6 @@ impl FileLock {
|
|||
impl Drop for FileLock {
|
||||
#[instrument(skip_all)]
|
||||
fn drop(&mut self) {
|
||||
std::fs::remove_file(&self.path).expect("failed to delete lock file");
|
||||
std::fs::remove_file(&self.path).expect("Failed to delete lock file");
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue