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

bug(rush-lib,operation-weighting): UNASSIGNED_OPERATION causing memory leak #4684

Merged
merged 2 commits into from
May 8, 2024

Conversation

aramissennyeydd
Copy link
Contributor

@aramissennyeydd aramissennyeydd commented May 8, 2024

Summary

Followup to #4672

Having a weight of 0 for UNASSIGNED_OPERATION was causing a build up of objects for the instance that was waiting. This quickly causes a GC error, locally I can trigger it in 30 seconds, in our CI it's about 2 mins.

Details

I tried to upgrade our cobuild instance this morning and ran into this issue. Monkeypatching the weight to 1 locally fixes the issue.

This is caused by the weight of 0 being an immediate execution, UNASSIGNED_OPERATIONs must have an actual weight so the process is stuck waiting for some time.

How it was tested

Tested this in our internal rush instance, I also tested this with the cobuild test project by setting the timeout in the build.js script to a long interval (2000 seconds). The instance that is waiting will throw a GC error pretty quickly.

With this change, the waiting process is stable and doesn't cause a GC error.

Impacted documentation

None.

@aramissennyeydd aramissennyeydd changed the title fix(rush-lib,operation-weighting): UNASSIGNED_OPERATION causing GC leak bug(rush-lib,operation-weighting): UNASSIGNED_OPERATION causing GC leak May 8, 2024
@aramissennyeydd aramissennyeydd changed the title bug(rush-lib,operation-weighting): UNASSIGNED_OPERATION causing GC leak bug(rush-lib,operation-weighting): UNASSIGNED_OPERATION causing memory leak May 8, 2024
@iclanton
Copy link
Member

iclanton commented May 8, 2024

This is an interesting issue. Is the underlying issue inside Async.forEachAsync?

@iclanton iclanton merged commit d8b955a into microsoft:main May 8, 2024
5 checks passed
@aramissennyeydd
Copy link
Contributor Author

@iclanton I don't think so? I think this is caused by a value of 0 being an automatic execution item, so it just keeps throwing unassigned operations at the queue.

FYI: I'm looking into a separate issue with the weighting that came up when I ran this in CI - not seeing the weighting be respected during execution.

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

Successfully merging this pull request may close these issues.

2 participants