facebook / buck2

Build system, successor to Buck
https://buck2.build/
Apache License 2.0
3.37k stars 199 forks source link

Remote Build Execution upload batching features #193

Closed benbrittain closed 1 year ago

benbrittain commented 1 year ago
benbrittain commented 1 year ago

This is a subset of the improvements made in #181 but includes batching of smaller messages into a reduced number of BatchUpdateBlobs requests instead of exclusively using on ByteStream.Write and querying the max allowed size from the remote server.

It also includes downloading large file support in addition to uploading.

benbrittain commented 1 year ago

I tried running this on my "production" codebase at work as a lazy E2e test and the size of the protobuf messages is not quite right since the blob hashes aren't the only thing in a message. Putting together a fix in a minute.

There was a chance that batched messages would be sightly over the gRPC limit.

benbrittain commented 1 year ago

Fixed and rebased on main.

benbrittain commented 1 year ago

I've also added support for not uploading blobs already present on the remote

benbrittain commented 1 year ago

I'm get to fix the missed large blobs, but it's on my TODO list. I'd also like to make it so if for some reason a request fails in the batch it gets retried with the bytestream service

Sorry for the weird close/open. I'm definitely gonna finish this. Do you want these commits squashed into logical commits or fixes in subsequent commits?

benbrittain commented 1 year ago

This is ready for another look whenever you have time :)

facebook-github-bot commented 1 year ago

@krallin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

benbrittain commented 1 year ago

I have not forgotten about this, I'll add some upload testing soon :+1:

krallin commented 1 year ago

No problem, thanks! :)

On Fri, 28 Apr 2023 at 20:12, Benjamin Brittain @.***> wrote:

I have not forgotten about this, I'll add some upload testing soon 👍

— Reply to this email directly, view it on GitHub https://github.com/facebook/buck2/pull/193#issuecomment-1527923169, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANIHVQVHTPHVBJDX4KGEILXDQCCPANCNFSM6AAAAAAXIWD3TI . You are receiving this because you were mentioned.Message ID: @.***>

benbrittain commented 1 year ago

Added a few upload test cases (which, of course, caught a couple of minor problems)!

facebook-github-bot commented 1 year ago

@krallin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

krallin commented 1 year ago

Merged this today (after making a certain number of adjustements), thanks for your contribution to Buck2!