revlogindex: update outdated nodemap more aggressively

Summary:
If the nodemap file is seriously out of sync, it's harder to recover on Windows,
because nodemap mmap in other processes prevents `atomic_write`.

Use `util::path::remove_file` to remove the old mmap files and improve the
success rate of nodemap rewrites.

Reviewed By: xavierd

Differential Revision: D22168134

fbshipit-source-id: 577df978b3175eac3257794f5e5ff7522cf21871
This commit is contained in:
Jun Wu 2020-06-23 22:51:50 -07:00 committed by Facebook GitHub Bot
parent bc09039a7d
commit d59189b0d9
2 changed files with 14 additions and 5 deletions

View File

@ -11,3 +11,4 @@ indexedlog = { path = "../indexedlog" }
minibytes = { path = "../minibytes" }
parking_lot = "0.10"
radixbuf = { path = "../radixbuf" }
util = { path = "../util" }

View File

@ -394,10 +394,9 @@ impl RevlogIndex {
let empty_nodemap_data = Bytes::from(nodemap::empty_index_buffer());
let nodemap_data = read_path(nodemap_path, empty_nodemap_data.clone())?;
let changelogi_data = read_path(changelogi_path, Bytes::default())?;
let nodemap =
NodeRevMap::new(changelogi_data.into(), nodemap_data.into()).or_else(|_| {
// Attempt to rebuild the index automatically.
let changelogi_data = read_path(changelogi_path, Bytes::default())?;
let nodemap = NodeRevMap::new(changelogi_data.clone().into(), nodemap_data.into())
.or_else(|_| {
// Attempt to rebuild the index (in-memory) automatically.
NodeRevMap::new(changelogi_data.into(), empty_nodemap_data.into())
})?;
// 20000 is chosen as it takes a few milliseconds to build up.
@ -409,7 +408,16 @@ impl RevlogIndex {
let slice =
unsafe { slice::from_raw_parts(buf.as_ptr() as *const u8, buf.len() * 4) };
// Not fatal if we cannot update the on-disk index.
let _ = atomic_write(nodemap_path, slice, false);
if atomic_write(nodemap_path, slice, false).is_err() && cfg!(windows) {
// On Windows it can fail if nodemap_path is mmap-ed. Retry after deleting the file.
// Note: This makes the file disappear temporarily. Another process reading the
// revlog at this time might have to rebuild the nodemap. The `remove_file`
// strategy should be used with care and should not be used for cases where the
// file needs to be always present.
if util::path::remove_file(nodemap_path).is_ok() {
let _ = atomic_write(nodemap_path, slice, false);
}
}
}
}
let result = Self {