Report digest of failed uploads #12507
Conversation
This will make the error message more useful because otherwise, there was no way of telling which upload failed or timed out (other than running with `-j 1`)
Yannic
force-pushed
the
artifact-upload-failure
branch
from
463954f
to
87095f8
Compare
| @@ -83,10 +84,10 @@ | |||
|
|
|||
| /** Contains the hash codes of already uploaded blobs. **/ | |||
| @GuardedBy("lock") | |||
| private final Set<HashCode> uploadedBlobs = new HashSet<>(); | |||
| private final Set<Digest> uploadedBlobs = new HashSet<>(); | |||
There was a problem hiding this comment.
This may have used HashCode because it's more compact. I think you are changing this to get the size as well. The downside is that this could increase memory consumption by 2x.
There was a problem hiding this comment.
Switched uploadedBlobs´ back to HashCode`.
| (sre) -> Futures.immediateFailedFuture( | ||
| new IOException( | ||
| String.format( | ||
| "Error while uploading artifact with digest '%s/%s'", |
There was a problem hiding this comment.
It's an improvement, but it would be even better if it gave a filename.
There was a problem hiding this comment.
Absolutely! However, we won't get a filename here because the blob may not be a named file (e.g., std{out,err}). We would need to do that further downstream.
Let's start with the hash and I'll send a followup if I can figure out how to create an error message that contains the name.
There was a problem hiding this comment.
Maybe catch the exception and add filename at the call site. e.g. https://github.com/coeuvre/bazel/blob/remote-with-rx/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java#L396
There was a problem hiding this comment.
Thanks for the improvement! Please take a look at comments and I will import afterwards.
BTW, I am working on refactoring remote module with RxJava to improve readability and maintainability. RxByteStreamUploader is the first step and is almost done.
src/main/java/com/google/devtools/build/lib/remote/ByteStreamUploader.java
Show resolved
Hide resolved
|
Thanks! Importing. |
This will make the error message more useful because otherwise, there was no way of telling which upload failed or timed out (other than running with `-j 1`) Closes #12507. PiperOrigin-RevId: 343977441
This will make the error message more useful because otherwise, there was no way of telling which upload failed or timed out (other than running with `-j 1`) Closes #12507. PiperOrigin-RevId: 343977441
This will make the error message more useful because otherwise, there
was no way of telling which upload failed or timed out (other than
running with
-j 1)