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

[WIP] bazel: Set RBE downloads to minimal #22017

Closed
wants to merge 3 commits into from

Conversation

phlax
Copy link
Member

@phlax phlax commented Jul 4, 2022

Signed-off-by: Ryan Northey ryan@synca.io

Commit Message:
Additional Description:

Testing RBE locally and it can in many cases take longer downloading files than it would have done just running locally

Partly i think (mis)use of pkg_tars is to blame, and there are defo some optis that we could add, but i also think we want this set to minimal if possible i wasnt using the CI config

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@phlax phlax self-assigned this Jul 4, 2022
@phlax phlax changed the title [WIP/TESTING] bazel: Set RBE downloads to minimal bazel: Set RBE downloads to minimal Jul 4, 2022
@phlax phlax changed the title bazel: Set RBE downloads to minimal [WIP] bazel: Set RBE downloads to minimal Jul 4, 2022
@phlax phlax marked this pull request as draft July 4, 2022 09:36
@phlax
Copy link
Member Author

phlax commented Jul 4, 2022

this seems to make the problem described in bazelbuild/bazel#11834 worse, and prevents the workaround from working - investigating further...

@phlax phlax force-pushed the bazel-rbe-dl-minimal branch 2 times, most recently from 435d81c to df44548 Compare July 4, 2022 11:50
@phlax phlax changed the title [WIP] bazel: Set RBE downloads to minimal bazel: Set RBE downloads to minimal Jul 4, 2022
@phlax phlax marked this pull request as ready for review July 4, 2022 11:50
@phlax
Copy link
Member Author

phlax commented Jul 4, 2022

this seems to speed up CI considerably (and local RBE)

the one gotcha is the workaround in CI - as this is kinda already the case - just the workaround has changed - im thinking we should still add

we might want to add some docs somewhere about the known issue if it affects local RBE without running through the ci codepath

one option might be to use pkg_tar and bundle the bins before downloading, in my local testing nothing seemed to prevent these downloading wherever they are in the pipeline - but potentially doing this could resolve the TODO about using bazel to build all of the release bins together

@phlax phlax assigned lizan and unassigned phlax Jul 4, 2022
@phlax
Copy link
Member Author

phlax commented Jul 4, 2022

@phlax phlax force-pushed the bazel-rbe-dl-minimal branch 2 times, most recently from 1d94f7b to 96aa4dc Compare July 4, 2022 12:59
@phlax
Copy link
Member Author

phlax commented Jul 4, 2022

not sure why it is different locally

because im building docs and not running any tests that consume the docs output beforehand

@phlax phlax changed the title bazel: Set RBE downloads to minimal [WIP] bazel: Set RBE downloads to minimal Jul 4, 2022
@phlax phlax marked this pull request as draft July 4, 2022 13:44
@phlax
Copy link
Member Author

phlax commented Jul 4, 2022

because im building docs and not running any tests that consume the docs output beforehand

that doesnt seem to be it - docs fail in CI and dont run anything that consumes the docs beforehand and it is failing - locally never fails

Fix envoyproxy#21946

Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax force-pushed the bazel-rbe-dl-minimal branch 3 times, most recently from 541bc35 to 72ae31e Compare July 4, 2022 15:49
@phlax
Copy link
Member Author

phlax commented Jul 4, 2022

so it seems the reason i was downloading so much is that i didnt have the toplevel config activated

i now also understand that download_minimal means more like download_nothing - so if this is the default then build jobs that use targets locally need to add a flag - probably if we enable this it should just be for ci

not sure exactly why but this still saves 25% from the job to build x86 - i guess this is more to do with test than build

phlax added 2 commits July 5, 2022 09:56
Signed-off-by: Ryan Northey <ryan@synca.io>
Signed-off-by: Ryan Northey <ryan@synca.io>
@phlax phlax force-pushed the bazel-rbe-dl-minimal branch from 72ae31e to 271abe2 Compare July 5, 2022 08:56
@github-actions
Copy link

github-actions bot commented Aug 4, 2022

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Aug 4, 2022
@phlax phlax removed the stale stalebot believes this issue/PR has not been touched recently label Aug 4, 2022
@github-actions
Copy link

github-actions bot commented Sep 3, 2022

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Sep 3, 2022
@github-actions
Copy link

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants