Skip to content

Commit

Permalink
Fix problems with inproc node confinement
Browse files Browse the repository at this point in the history
  • Loading branch information
cdmihai committed Jun 29, 2021
1 parent 1f78f95 commit 7a6152f
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 4 deletions.
108 changes: 108 additions & 0 deletions src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,30 @@ public enum ErrorKind
LoggedError
}

public class ConfigurableMockCache : ProjectCachePluginBase
{
public Func<BuildRequestData, PluginLoggerBase, CancellationToken, Task<CacheResult>>? GetCacheResultImplementation { get; set; }
public override Task BeginBuildAsync(CacheContext context, PluginLoggerBase logger, CancellationToken cancellationToken)
{
return Task.CompletedTask;
}

public override Task<CacheResult> 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;
Expand Down Expand Up @@ -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 =
@$"
<Project>
<Target Name='{cacheNotApplicableTarget}'>
<Exec Command=`{Helpers.GetSleepCommand(TimeSpan.FromMilliseconds(200))}` />
<Message Text='{cacheNotApplicableTarget} in $(MSBuildThisFile)' />
</Target>
<Target Name='{cacheHitTarget}'>
<Message Text='{cacheHitTarget} in $(MSBuildThisFile)' />
</Target>
<Target Name='{proxyTarget}'>
<Message Text='{proxyTarget} in $(MSBuildThisFile)' />
</Target>
</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<string, string> {{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<Task<BuildResult>>();
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);
Expand Down
10 changes: 7 additions & 3 deletions src/Build/BackEnd/Components/Scheduler/Scheduler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1781,7 +1781,7 @@ private void HandleRequestBlockedByNewRequests(SchedulableRequest parentRequest,
existingRequestAffinity,
config.ProjectFullPath,
globalProperties
)));
)));
response = GetResponseForResult(nodeForResults, request, result);
responses.Add(response);
continue;
Expand Down Expand Up @@ -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;
}
Expand Down
7 changes: 6 additions & 1 deletion src/Build/BackEnd/Components/Scheduler/SchedulingData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,12 @@ public int GetAssignedNodeForRequestConfiguration(int configurationId)
/// </summary>
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;
}

Expand Down

0 comments on commit 7a6152f

Please sign in to comment.