From 7a6152f99a981f9ccffa522419cd2a3acd4d233c Mon Sep 17 00:00:00 2001 From: Mihai Codoban Date: Fri, 25 Jun 2021 17:54:31 -0700 Subject: [PATCH] Fix problems with inproc node confinement --- .../ProjectCache/ProjectCacheTests.cs | 108 ++++++++++++++++++ .../BackEnd/Components/Scheduler/Scheduler.cs | 10 +- .../Components/Scheduler/SchedulingData.cs | 7 +- 3 files changed, 121 insertions(+), 4 deletions(-) diff --git a/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs b/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs index 65b60c15a0c..d617ecc609b 100644 --- a/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs +++ b/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs @@ -233,6 +233,30 @@ public enum ErrorKind LoggedError } + public class ConfigurableMockCache : ProjectCachePluginBase + { + public Func>? GetCacheResultImplementation { get; set; } + public override Task BeginBuildAsync(CacheContext context, PluginLoggerBase logger, CancellationToken cancellationToken) + { + return Task.CompletedTask; + } + + public override Task GetCacheResultAsync( + BuildRequestData buildRequest, + PluginLoggerBase logger, + CancellationToken cancellationToken) + { + return GetCacheResultImplementation != null + ? GetCacheResultImplementation(buildRequest, logger, cancellationToken) + : Task.FromResult(CacheResult.IndicateNonCacheHit(CacheResultType.CacheNotApplicable)); + } + + public override Task EndBuildAsync(PluginLoggerBase logger, CancellationToken cancellationToken) + { + return Task.CompletedTask; + } + } + public class InstanceMockCache : ProjectCachePluginBase { private readonly GraphCacheResponse? _testData; @@ -1475,6 +1499,90 @@ public void ParallelStressTest(bool useSynchronousLogging, bool disableInprocNod cache.QueryStartStops.Count.ShouldBe(graph.ProjectNodes.Count * 2); } + [Fact] + public void ProxyCacheHitsOnPreviousCacheMissesShouldWork() + { + var cacheNotApplicableTarget = "NATarget"; + var cacheHitTarget = "CacheHitTarget"; + var proxyTarget = "ProxyTarget"; + + var project = +@$" + + + + + + + + + + + + + + +".Cleanup(); + + var projectPaths = Enumerable.Range(0, NativeMethodsShared.GetLogicalCoreCount()) + .Select(i => _env.CreateFile($"project{i}.proj", project).Path) + .ToArray(); + + var cacheHitCount = 0; + var nonCacheHitCount = 0; + + var buildParameters = new BuildParameters + { + ProjectCacheDescriptor = ProjectCacheDescriptor.FromInstance( + new ConfigurableMockCache + { + GetCacheResultImplementation = (request, _, _) => + { + var projectFile = request.ProjectFullPath; + + if (request.TargetNames.Contains(cacheNotApplicableTarget)) + { + Interlocked.Increment(ref nonCacheHitCount); + return Task.FromResult(CacheResult.IndicateNonCacheHit(CacheResultType.CacheNotApplicable)); + } + else + { + Interlocked.Increment(ref cacheHitCount); + return Task.FromResult( + CacheResult.IndicateCacheHit( + new ProxyTargets(new Dictionary {{proxyTarget, cacheHitTarget}}))); + } + } + }, + projectPaths.Select(p => new ProjectGraphEntryPoint(p)).ToArray(), + projectGraph: null), + MaxNodeCount = NativeMethodsShared.GetLogicalCoreCount() + }; + + using var buildSession = new Helpers.BuildManagerSession(_env, buildParameters); + + var buildRequests = new List<(string, string)>(); + buildRequests.AddRange(projectPaths.Select(r => (r, cacheNotApplicableTarget))); + buildRequests.AddRange(projectPaths.Select(r => (r, cacheHitTarget))); + + var buildTasks = new List>(); + foreach (var (projectPath, target) in buildRequests) + { + buildTasks.Add(buildSession.BuildProjectFileAsync(projectPath, new[] {target})); + } + + foreach (var buildResult in buildTasks.Select(buildTask => buildTask.Result)) + { + buildResult.Exception.ShouldBeNull(); + buildResult.OverallResult.ShouldBe(BuildResultCode.Success); + } + + buildSession.Logger.ProjectStartedEvents.Count.ShouldBe(2 * projectPaths.Length); + + cacheHitCount.ShouldBe(projectPaths.Length); + nonCacheHitCount.ShouldBe(projectPaths.Length); + } + private static void StringShouldContainSubstring(string aString, string substring, int expectedOccurrences) { aString.ShouldContain(substring); diff --git a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs index c89de3b594c..807ceaee76d 100644 --- a/src/Build/BackEnd/Components/Scheduler/Scheduler.cs +++ b/src/Build/BackEnd/Components/Scheduler/Scheduler.cs @@ -1385,7 +1385,7 @@ private void AssignUnscheduledRequestToNode(SchedulableRequest request, int node void WarnWhenProxyBuildsGetScheduledOnOutOfProcNode() { - if (request.IsProxyBuildRequest() && nodeId != InProcNodeId) + if (request.IsProxyBuildRequest() && nodeId != InProcNodeId && _schedulingData.CanScheduleRequestToNode(request, InProcNodeId)) { ErrorUtilities.VerifyThrow( _componentHost.BuildParameters.DisableInProcNode || _forceAffinityOutOfProc, @@ -1781,7 +1781,7 @@ private void HandleRequestBlockedByNewRequests(SchedulableRequest parentRequest, existingRequestAffinity, config.ProjectFullPath, globalProperties - ))); + ))); response = GetResponseForResult(nodeForResults, request, result); responses.Add(response); continue; @@ -2125,7 +2125,11 @@ private NodeAffinity GetNodeAffinityForRequest(BuildRequest request) return NodeAffinity.InProc; } - if (request.IsProxyBuildRequest()) + ErrorUtilities.VerifyThrow(request.ConfigurationId != BuildRequestConfiguration.InvalidConfigurationId, "Requests should have a valid configuration id at this point"); + // If this configuration has been previously built on an out of proc node, scheduling it on the inproc node can cause either an affinity mismatch error when + // there are other pending requests for the same configuration or "unscheduled requests remain in the presence of free out of proc nodes" errors if there's no pending requests. + // So only assign proxy builds to the inproc node if their config hasn't been previously assigned to an out of proc node. + if (_schedulingData.CanScheduleConfigurationToNode(request.ConfigurationId, InProcNodeId) && request.IsProxyBuildRequest()) { return NodeAffinity.InProc; } diff --git a/src/Build/BackEnd/Components/Scheduler/SchedulingData.cs b/src/Build/BackEnd/Components/Scheduler/SchedulingData.cs index 0edc83f296e..9aeb9009c80 100644 --- a/src/Build/BackEnd/Components/Scheduler/SchedulingData.cs +++ b/src/Build/BackEnd/Components/Scheduler/SchedulingData.cs @@ -647,7 +647,12 @@ public int GetAssignedNodeForRequestConfiguration(int configurationId) /// public bool CanScheduleRequestToNode(SchedulableRequest request, int nodeId) { - int requiredNodeId = GetAssignedNodeForRequestConfiguration(request.BuildRequest.ConfigurationId); + return CanScheduleConfigurationToNode(request.BuildRequest.ConfigurationId, nodeId); + } + + public bool CanScheduleConfigurationToNode(int configurationId, int nodeId) + { + int requiredNodeId = GetAssignedNodeForRequestConfiguration(configurationId); return requiredNodeId == Scheduler.InvalidNodeId || requiredNodeId == nodeId; }