Skip to content

[rush] feat(cobuilds): allow orchestration without using the build cache #4871

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

Merged

Conversation

aramissennyeydd
Copy link
Contributor

Summary

From my thread in Zulip, here.

Currently, cobuilds rebuild un-cacheable operations across all machines. This is the expected behavior for core nodes that aren't cacheable to ensure artifacts are found for projects further up the tree. For operations like uploading/processing coverage or image building, that approach doesn't apply well and ends up in (number of machines)x more work being done. This PR attempts to add a new experiment/feature that allows users to use the cobuild orchestration engine without relying on the build cache. Inherently, this introduces risk for projects that enable this without knowing what it does as you'll be explicitly skipping build cache restoration, but I think the benefits outweigh the drawbacks here.

Details

I tried to add this similarly to how cobuild leaf-only projects are enabled. This ends up being a pretty small change with decent impact. Shards are an interesting case here that should probably be explicitly restricted since the output needs to be shared across all agents if you have a collate step, but it was an easy way to get many events to test with.

Before

Screenshot 2024-08-08 at 7 05 57 PM

After

Screenshot 2024-08-08 at 7 03 46 PM

How it was tested

Tested against the sharded-repo using

rm -rf common/temp/build-cache && RUSH_COBUILD_CONTEXT_ID=foo REDIS_PASS=redis123 RUSH_COBUILD_RUNNER_ID=runner1 RUSH_COBUILD_ORCHESTRATION_ONLY_ALLOWED=1 node ../../lib/runRush.js cobuild -p 10 --timeline

to validate that operations were being shared across the 2 machines.

and the same command without the RUSH_COBUILD_ORCHESTRATION_ONLY_ALLOWED env var set,

rm -rf common/temp/build-cache && RUSH_COBUILD_CONTEXT_ID=foo REDIS_PASS=redis123 RUSH_COBUILD_RUNNER_ID=runner1 node ../../lib/runRush.js cobuild -p 10 --timeline

for the negative cases.

Impacted documentation

It adds a new experiment that will likely need to end up on the docs and updates the rush-project JSON schema.

@dmichon-msft
Copy link
Contributor

The terminology in here needs a lot of clarification. You are trying to identify operations that have side effects that need to occur exactly once in an orchestrated build, but that have no outputs that impact downstream operations, correct?

@aramissennyeydd
Copy link
Contributor Author

@dmichon-msft 😅 I was confusing myself writing the original post. Yes, that sounds right, rephrasing a bit for my understanding.

I'm looking to use cobuilds (orchestrated builds) on operations that

  1. do things (upload coverage, write to S3) which I think is what you call side effects
  2. do not write to the build cache, but may consume previous stage artifacts, ie coverage, build artifacts
  3. need to run exactly once per build, we cannot tolerate each operation running per agent
  4. need to run on specific commits, ie merge queue HEAD - the side-effect needs to be executed on those specific commits

Copy link
Member

@iclanton iclanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aramissennyeydd - we can probably get this released soon, but I think the naming needs some work.

@aramissennyeydd
Copy link
Contributor Author

@iclanton Gentle bump here 🙏

@aramissennyeydd aramissennyeydd force-pushed the sennyeya/allow-cobuild-orchestration branch from f7deacf to 9541a93 Compare September 18, 2024 22:07
…/sharded-repo/projects/e/config/rush-project.json
@iclanton iclanton enabled auto-merge (squash) September 20, 2024 19:21
@iclanton iclanton merged commit 50449d0 into microsoft:main Sep 20, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

3 participants