Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect progress report usage in BlobPuller #1522

Open
chanseokoh opened this issue Feb 28, 2019 · 0 comments
Open

Incorrect progress report usage in BlobPuller #1522

chanseokoh opened this issue Feb 28, 2019 · 0 comments

Comments

@chanseokoh
Copy link
Member

chanseokoh commented Feb 28, 2019

From #1512 (comment).

  public Void handleResponse(Response response) ... {
    // Notifies the progress dispatcher that the total bytes we will read is the "Content-Length" value.
    // If `Content-Encoding: gzip`, the value will be the size of the compressed content.
    blobSizeConsumer.accept(response.getContentLength());

    try (OutputStream outputStream =
        new NotifyingOutputStream(destinationOutputStream, writtenByteCountListener)) {
      BlobDescriptor receivedBlobDescriptor =
          Digests.computeDigest(response.getBody(), outputStream);
      ...
    }

Now, in the case of Content-Encoding: gzip (or something similar), Google HTTP Client's HttpResponse::getContent() creates and returns GZIPInputStream that wraps the raw InputStream, which means the library does the chore of streamed unzipping on behalf of us. Content-Length will of course be the size of the original (compressed) content stream.

Then NotifyingOutputStream::write() will periodically notify the progress dispatcher how many bytes were written (through writtenByteCountListener). The problem is that the progress dispatcher is initialized with the total allocation size (through blobSizeConsumer) to be the value reported in Content-Length. Because we will write uncompressed content to outputStream in this case, we will go over the progress limit.


Another case where the progress is under reported:

.create("compressing " + file, Files.size(file));

The NotifyingOutputStream will report written byte count. The count will be smaller than the original file size, as the written content is compressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants