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.

Sign up for GitHub

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

Jump to bottom

Rework ByteStreamUploader early return logic. #17791

Closed
wants to merge 1 commit into from
Closed

Rework ByteStreamUploader early return logic. #17791

wants to merge 1 commit into from

Commits on Mar 16, 2023

  1. Rework ByteStreamUploader early return logic. 

    There are several points where ByteStreamUploader may discover that the server already has the blob fully uploaded. These points tried to effect an early return from the upload code, generally by "lying" to higher layers that the upload fully finished. That could lead to bugs. For example, consider the added test case: A compressed upload completes its writing but receives an error instead of a server ACK. On retry, QueryWriteStatus reveals the blob exists and returns its uncompressed size. This confused the checkCommittedSize logic, which expected the final committed size of a compressed upload to be the total compressed data size or -1. The code added by bazelbuild@daa3dbe also looks broken in the case of compressed uploads.

Rework the uploader code, so that early returns throw a AlreadyExists exception. The exception control flow naturally reflects the desire to escape quickly to the top level.
    @benjaminp
    benjaminp committed Mar 16, 2023
    Configuration menu
    Copy the full SHA
    a91fe7f View commit details
    Browse the repository at this point in the history