-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Don't schedule proxy builds to inproc node if their configs previously built on oop nodes #6635
Merged
rainersigwald
merged 3 commits into
dotnet:vs16.11
from
cdmihai:fix_inproc_node_optimization
Jul 1, 2021
Merged
Don't schedule proxy builds to inproc node if their configs previously built on oop nodes #6635
rainersigwald
merged 3 commits into
dotnet:vs16.11
from
cdmihai:fix_inproc_node_optimization
Jul 1, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cdmihai
force-pushed
the
fix_inproc_node_optimization
branch
from
June 29, 2021 01:26
19b41c9
to
7a6152f
Compare
cdmihai
changed the title
Fix inproc node optimization
Don't schedule proxy builds to inproc node if their configs previously built on oop nodes
Jun 29, 2021
rainersigwald
requested changes
Jun 29, 2021
cdmihai
force-pushed
the
fix_inproc_node_optimization
branch
from
June 29, 2021 23:40
7a6152f
to
e9af31a
Compare
rainersigwald
approved these changes
Jun 30, 2021
cdmihai
force-pushed
the
fix_inproc_node_optimization
branch
from
July 1, 2021 16:59
e9af31a
to
06677fb
Compare
johnterickson
approved these changes
Jul 1, 2021
rokonec
pushed a commit
that referenced
this pull request
Aug 5, 2021
* Fix missing project instance in project cache requests (#6568) Context Non static graph builds using the project cache didn't set the ProjectInstance on the cache request, leading to crashes in the cache. Changes Made Recreate the BuildRequestData for the cache request after the cache service evaluates projects. I was initially using the original BuildSubmission.BuildRequestData which does not contain the project instance. * Don't launch debugger window for all tests * Convert static InitializePlugin into non-static BeginBuildAsync To allow asserting service state transition * Assert state transitions in ProjectCacheService * Only initialize once for the VS workaround * Bravely set DoNotLaunchDebugger only once for all tests * Simplify branching * [vs16.11] Update dependencies from dotnet/arcade (#6625) * Update dependencies from https://github.com/dotnet/arcade build 20210628.3 Microsoft.DotNet.Arcade.Sdk From Version 5.0.0-beta.21315.2 -> To Version 5.0.0-beta.21328.3 * Don't schedule proxy builds to inproc node if their configs previously built on oop nodes (#6635) Fixes a bug in proxy build scheduling introduced by #6386. If a the BuildRequestConfiguration associated with a proxy request has been built before on an out of proc (oop) node then the scheduler will fail with either one of: - affinity mismatch error. This happens when the proxy build is assigned to the inproc (inp) node but its configuration is already assigned to an oop node `AND` serving other existing requests, either blocked or running. - unscheduled requests remain even if there's free oop nodes that can serve them. This happens (as far as I can tell) when the proxy's configuration is already assigned to an oop node (because a previously built non proxy request was assigned there) `AND` there's no other existing requests for that configuration The fix in this PR is to not assign a proxy build to the inproc node if its configuration was previously assigned to another node. * Localized file check-in by OneLocBuild Task (#6644) * 16.11 Final Branding (#6656) Co-authored-by: Rainer Sigwald <[email protected]> Co-authored-by: Mihai Codoban <[email protected]> Co-authored-by: AR-May <[email protected]> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: dotnet bot <[email protected]> Co-authored-by: Ben Villalobos <[email protected]> Co-authored-by: Rainer Sigwald <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
Fixes a bug in proxy build scheduling introduced by #6386. If a the BuildRequestConfiguration associated with a proxy request has been built before on an out of proc (oop) node then the scheduler will fail with either one of:
AND
serving other existing requests, either blocked or running.AND
there's no other existing requests for that configurationChanges Made
The fix in this PR is to not assign a proxy build to the inproc node if its configuration was previously assigned to another node.
Testing
Unit test that builds projects in parallel with both cache misses and cache hits, thus forcing some configs to build on oop nodes prior to their cache hit proxy builds.
Notes
The bug only affects proxy builds. Does not touch non project cache scenarios, so should be no risk for 16.11