Skip to content

Commit

Permalink
actually, we need to drop the error on the write, not the read
Browse files Browse the repository at this point in the history
  • Loading branch information
cosmicexplorer committed Apr 11, 2019
1 parent 200299d commit 391a11d
Showing 1 changed file with 21 additions and 14 deletions.
35 changes: 21 additions & 14 deletions src/rust/engine/fs/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 {:?}: {:?}",
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 391a11d

Please sign in to comment.