mononoke/fileblob: don't silently ignore write errors

Summary:
Right now, if we get an error when writing to fileblob, we silently ignore it
under the assumption that it's an overwrite that failed. We shouldn't do this,
since it can cause to silently discard data.

For example, when using Mononoke with a tmpdir that isn't next to your blobs,
one might hit this error:

```
Os { code: 18, kind: Other, message: "Invalid cross-device link" }
```

Right now, we just silently ignore it and move on. With this diff, we'll
properly report that the write failed.

Reviewed By: farnz

Differential Revision: D26912990

fbshipit-source-id: 1b5a3c79c80f6b445e16e352e1c78aaec5e21850
This commit is contained in:
Thomas Orozco 2021-03-09 09:40:32 -08:00 committed by Facebook GitHub Bot
parent 7e6a9da97e
commit ae96770bc0
2 changed files with 33 additions and 1 deletions

View File

@ -15,3 +15,7 @@ percent-encoding = "2.1"
tempfile = "3.1"
tokio = { version = "0.2.25", features = ["full", "test-util"] }
walkdir = "2.2.9"
[dev-dependencies]
fbinit = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "master" }
fbinit-tokio-02 = { version = "0.1.0", git = "https://github.com/facebookexperimental/rust-shed.git", branch = "master" }

View File

@ -110,7 +110,10 @@ impl BlobstorePutOps for Fileblob {
match tempfile.persist_noclobber(&p) {
Ok(_) => OverwriteStatus::New,
// Key already existed
Err(PersistError { file: f, error: _ }) if f.path() == temp_path => {
Err(PersistError { file: f, error: e })
if f.path() == temp_path
&& e.kind() == std::io::ErrorKind::AlreadyExists =>
{
if put_behaviour.should_overwrite() {
f.persist(&p)?;
OverwriteStatus::Overwrote
@ -231,3 +234,28 @@ impl BlobstoreKeySource for Fileblob {
}
}
}
#[cfg(test)]
mod test {
use super::*;
use fbinit::FacebookInit;
#[fbinit::test]
async fn test_persist_error(fb: FacebookInit) -> Result<()> {
let ctx = CoreContext::test_mock(fb);
let blob = Fileblob {
base: PathBuf::from("/mononoke/fileblob/test/path/should/not/exist"),
put_behaviour: PutBehaviour::IfAbsent,
};
let ret = blob
.put(&ctx, "key".into(), BlobstoreBytes::from_bytes("value"))
.await;
assert!(ret.is_err());
Ok(())
}
}