From 391a11dc2c21751a29b67fcf3447be8007eec0b2 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Wed, 10 Apr 2019 20:01:18 -0700 Subject: [PATCH] actually, we need to drop the error on the write, not the read --- src/rust/engine/fs/src/store.rs | 35 ++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/rust/engine/fs/src/store.rs b/src/rust/engine/fs/src/store.rs index 06a1a35aefab..d7aa811137f9 100644 --- a/src/rust/engine/fs/src/store.rs +++ b/src/rust/engine/fs/src/store.rs @@ -1892,7 +1892,7 @@ mod remote { digest, err )) .to_boxed(), - Ok(((sender, receiver), client)) => { + Ok(((sender, receiver), _client)) => { let chunk_size_bytes = store.chunk_size_bytes; let resource_name = resource_name.clone(); let bytes = bytes.clone(); @@ -1918,11 +1918,26 @@ mod remote { } }); - future::ok(client) - .join(sender.send_all(stream).map_err(move |e| { - format!("Error attempting to upload digest {:?}: {:?}", digest, e) - })) - .and_then(move |_| { + sender + .send_all(stream) + .map(|_| ()) + .or_else(move |e| { + match e { + // NB(#7422): This can sometimes occur when pants sends many process execution + // requests which depend on the same large file at the same time. It appears that + // the check_alive() method in grpc-rs which returns RpcFinished(None) is used as + // a way to exit early out of methods which return a Result, if the future's + // contents are already available. This may be a race condition that deserves + // respect, but right now it's not clear it's an error. See #6344 for a case where + // we override this type of behavior by forking gprcio. + grpcio::Error::RpcFinished(None) => Ok(()), + e => Err(format!( + "Error attempting to upload digest {:?}: {:?}", + digest, e + )), + } + }) + .and_then(move |()| { receiver.map_err(move |e| { format!( "Error from server when uploading digest {:?}: {:?}", @@ -1996,14 +2011,6 @@ mod remote { status: grpcio::RpcStatusCode::NotFound, .. }) => Ok(None), - // NB: This can sometimes occur when pants sends many process execution requests - // which depend on the same large file at the same time. It appears that the - // check_alive() method in grpc-rs which returns RpcFinished(None) is used as a - // way to exit early out of methods which return a Result, if the future's - // contents are already available. This may be a race condition that deserves - // respect, but right now it's not clear it's an error. See #6344 for a case - // where we override this type of behavior by forking gprcio. - grpcio::Error::RpcFinished(None) => Ok(None), _ => Err(format!( "Error from server in response to CAS read request: {:?}", e