Skip to content

Commit

Permalink
dedupStoreHttpClient honors redirect timeout from client settings (#4504
Browse files Browse the repository at this point in the history
)

Co-authored-by: Adam Tseng <[email protected]>
  • Loading branch information
kutsen99 and Adam Tseng authored Nov 3, 2023
1 parent 18fc151 commit 6ee2a6b
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/Agent.Listener/Agent.Listener.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.9.8" Condition="$(CodeAnalysis)=='true'" />
<PackageReference Include="Microsoft.TeamFoundationServer.Client" Version="16.205.1" />
<PackageReference Include="Microsoft.VisualStudio.Services.InteractiveClient" Version="16.205.1" />
<PackageReference Include="Microsoft.Win32.Registry" Version="4.7.0" />
<PackageReference Include="Microsoft.Win32.Registry" Version="5.0.0" />
<PackageReference Include="System.IO.FileSystem.AccessControl" Version="6.0.0-preview.5.21301.5" />
<PackageReference Include="System.ServiceProcess.ServiceController" Version="4.4.0" />
<PackageReference Include="System.ServiceProcess.ServiceController" Version="6.0.1" />
<PackageReference Include="vss-api-netcore" Version="$(VssApiVersion)" />
</ItemGroup>

Expand Down
1 change: 1 addition & 0 deletions src/Agent.Plugins/Artifact/ArtifactItemFilters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Agent.Sdk;
using Microsoft.TeamFoundation.Build.WebApi;
using Microsoft.VisualStudio.Services.BlobStore.Common;
using Microsoft.VisualStudio.Services.BlobStore.WebApi;
using Microsoft.VisualStudio.Services.Content.Common.Tracing;
using Microsoft.VisualStudio.Services.FileContainer;
using Microsoft.VisualStudio.Services.WebApi;
Expand Down
7 changes: 6 additions & 1 deletion src/Agent.Plugins/Artifact/FileContainerProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,12 @@ private async Task DownloadFileContainerAsync(IEnumerable<FileContainerItem> ite
try
{
(dedupClient, clientTelemetry) = await DedupManifestArtifactClientFactory.Instance.CreateDedupClientAsync(
false, (str) => this.tracer.Info(str), this.connection, DedupManifestArtifactClientFactory.Instance.GetDedupStoreClientMaxParallelism(context), cancellationToken);
false,
(str) => this.tracer.Info(str),
this.connection,
DedupManifestArtifactClientFactory.Instance.GetDedupStoreClientMaxParallelism(context),
Microsoft.VisualStudio.Services.BlobStore.WebApi.Contracts.Client.BuildArtifact,
cancellationToken);
}
catch (SocketException e)
{
Expand Down
5 changes: 3 additions & 2 deletions src/Agent.Sdk/Agent.Sdk.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.9.8" Condition="$(CodeAnalysis)=='true'" />
<PackageReference Include="Microsoft.Win32.Registry" Version="4.7.0" />
<PackageReference Include="System.ServiceProcess.ServiceController" Version="4.4.0" />
<PackageReference Include="Microsoft.Win32.Registry" Version="5.0.0" />
<PackageReference Include="System.IO.FileSystem.AccessControl" Version="6.0.0-preview.5.21301.5" />
<PackageReference Include="System.ServiceProcess.ServiceController" Version="6.0.1" />
<PackageReference Include="System.Security.Principal.Windows" Version="6.0.0-preview.5.21301.5" />
<PackageReference Include="System.Text.Encoding.CodePages" Version="4.4.0" />
<PackageReference Include="vss-api-netcore" Version="$(VssApiVersion)" />
Expand Down
2 changes: 1 addition & 1 deletion src/Agent.Worker/Agent.Worker.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<ItemGroup>
<PackageReference Include="Azure.Identity" Version="1.6.1" />
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.9.8" Condition="$(CodeAnalysis)=='true'" />
<PackageReference Include="System.ServiceProcess.ServiceController" Version="4.4.0" />
<PackageReference Include="System.ServiceProcess.ServiceController" Version="6.0.1" />
<PackageReference Include="vss-api-netcore" Version="$(VssApiVersion)" />
</ItemGroup>
</Project>
8 changes: 7 additions & 1 deletion src/Agent.Worker/Build/FileContainerServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,13 @@ private async Task<UploadResult> BlobUploadAsync(IAsyncCommandContext context, I
var verbose = String.Equals(context.GetVariableValueOrDefault("system.debug"), "true", StringComparison.InvariantCultureIgnoreCase);
int maxParallelism = context.GetHostContext().GetService<IConfigurationStore>().GetSettings().MaxDedupParallelism;
(dedupClient, clientTelemetry) = await DedupManifestArtifactClientFactory.Instance
.CreateDedupClientAsync(verbose, (str) => context.Output(str), this._connection, maxParallelism, token);
.CreateDedupClientAsync(
verbose,
(str) => context.Output(str),
this._connection,
maxParallelism,
BlobStore.WebApi.Contracts.Client.BuildArtifact,
token);

// Upload to blobstore
var results = await BlobStoreUtils.UploadBatchToBlobstore(verbose, files, (level, uri, type) =>
Expand Down
2 changes: 1 addition & 1 deletion src/Common.props
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<OSPlatform>OS_UNKNOWN</OSPlatform>
<OSArchitecture>ARCH_UNKNOWN</OSArchitecture>
<DebugConstant></DebugConstant>
<VssApiVersion>0.5.222-private</VssApiVersion>
<VssApiVersion>0.5.227-262a3469</VssApiVersion>
<CodeAnalysis>$(CodeAnalysis)</CodeAnalysis>
<InvariantGlobalization>false</InvariantGlobalization>
</PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using BuildXL.Cache.ContentStore.Hashing;
using Microsoft.VisualStudio.Services.BlobStore.WebApi.Contracts;
using Agent.Sdk.Knob;
using System.Collections.Generic;

namespace Microsoft.VisualStudio.Services.Agent.Blob
{
Expand All @@ -27,7 +28,7 @@ public interface IDedupManifestArtifactClientFactory
/// <param name="verbose">If true emit verbose telemetry.</param>
/// <param name="traceOutput">Action used for logging.</param>
/// <param name="connection">VssConnection</param>
/// <param name="maxParallelism">Maximum number of parallel threads that should be used for download. If 0 then
/// <param name="maxParallelism">Maximum number of parallel threads that should be used for download. If 0 then
/// use the system default. </param>
/// <param name="cancellationToken">Cancellation token used for both creating clients and verifying client conneciton.</param>
/// <returns>Tuple of the client and the telemtery client</returns>
Expand All @@ -40,7 +41,7 @@ public interface IDedupManifestArtifactClientFactory
ClientSettingsInfo clientSettings,
AgentTaskPluginExecutionContext context,
CancellationToken cancellationToken);

/// <summary>
/// Creates a DedupManifestArtifactClient client and retrieves any client settings from the server
/// </summary>
Expand All @@ -60,15 +61,17 @@ public interface IDedupManifestArtifactClientFactory
/// <param name="verbose">If true emit verbose telemetry.</param>
/// <param name="traceOutput">Action used for logging.</param>
/// <param name="connection">VssConnection</param>
/// <param name="maxParallelism">Maximum number of parallel threads that should be used for download. If 0 then
/// <param name="maxParallelism">Maximum number of parallel threads that should be used for download. If 0 then
/// use the system default. </param>
/// <param name="clientType">The client type for client settings</param>
/// <param name="cancellationToken">Cancellation token used for both creating clients and verifying client conneciton.</param>
/// <returns>Tuple of the client and the telemtery client</returns>
Task<(DedupStoreClient client, BlobStoreClientTelemetryTfs telemetry)> CreateDedupClientAsync(
bool verbose,
Action<string> traceOutput,
VssConnection connection,
int maxParallelism,
BlobStore.WebApi.Contracts.Client? clientType,
CancellationToken cancellationToken);

/// <summary>
Expand All @@ -84,7 +87,7 @@ public class DedupManifestArtifactClientFactory : IDedupManifestArtifactClientFa
// NOTE: this should be set to ClientSettingsConstants.DefaultDomainId when the latest update from Azure Devops is added.
private static string DefaultDomainIdKey = "DefaultDomainId";

// Old default for hosted agents was 16*2 cores = 32.
// Old default for hosted agents was 16*2 cores = 32.
// In my tests of a node_modules folder, this 32x parallelism was consistently around 47 seconds.
// At 192x it was around 16 seconds and 256x was no faster.
private const int DefaultDedupStoreClientMaxParallelism = 192;
Expand Down Expand Up @@ -115,7 +118,7 @@ private DedupManifestArtifactClientFactory()
client,
CreateArtifactsTracer(verbose, traceOutput),
cancellationToken);

return CreateDedupManifestClient(
context.IsSystemDebugTrue(),
(str) => context.Output(str),
Expand All @@ -124,7 +127,7 @@ private DedupManifestArtifactClientFactory()
domainId,
clientSettings,
context,
cancellationToken);
cancellationToken);
}

public (DedupManifestArtifactClient client, BlobStoreClientTelemetry telemetry) CreateDedupManifestClient(
Expand Down Expand Up @@ -153,13 +156,13 @@ private DedupManifestArtifactClientFactory()
tracer,
cancellationToken);

var helper = new HttpRetryHelper(maxRetries,e => true);
var helper = new HttpRetryHelper(maxRetries, e => true);

IDedupStoreHttpClient dedupStoreHttpClient = helper.Invoke(
() =>
{
// since our call below is hidden, check if we are cancelled and throw if we are...
cancellationToken.ThrowIfCancellationRequested();
cancellationToken.ThrowIfCancellationRequested();
IDedupStoreHttpClient dedupHttpclient;
// this is actually a hidden network call to the location service:
Expand All @@ -185,7 +188,9 @@ private DedupManifestArtifactClientFactory()
}
traceOutput($"Hashtype: {this.HashType.Value}");

var dedupClient = new DedupStoreClientWithDataport(dedupStoreHttpClient, new DedupStoreClientContext(maxParallelism), this.HashType.Value);
dedupStoreHttpClient.SetRedirectTimeout(GetRedirectTimeoutFromClientSettings(clientSettings));

var dedupClient = new DedupStoreClientWithDataport(dedupStoreHttpClient, new DedupStoreClientContext(maxParallelism), this.HashType.Value);
return (new DedupManifestArtifactClient(telemetry, dedupClient, tracer), telemetry);
}

Expand All @@ -194,6 +199,7 @@ private DedupManifestArtifactClientFactory()
Action<string> traceOutput,
VssConnection connection,
int maxParallelism,
BlobStore.WebApi.Contracts.Client? clientType,
CancellationToken cancellationToken)
{
const int maxRetries = 5;
Expand All @@ -203,6 +209,15 @@ private DedupManifestArtifactClientFactory()
maxParallelism = DefaultDedupStoreClientMaxParallelism;
}
traceOutput($"Max dedup parallelism: {maxParallelism}");

ClientSettingsInfo clientSettings = clientType is null
? null
: await GetClientSettingsAsync(
connection,
clientType.Value,
tracer,
cancellationToken);

var dedupStoreHttpClient = await AsyncHttpRetryHelper.InvokeAsync(
() =>
{
Expand All @@ -222,6 +237,8 @@ private DedupManifestArtifactClientFactory()
cancellationToken: cancellationToken,
continueOnCapturedContext: false);

dedupStoreHttpClient.SetRedirectTimeout(GetRedirectTimeoutFromClientSettings(clientSettings));

var telemetry = new BlobStoreClientTelemetryTfs(tracer, dedupStoreHttpClient.BaseAddress, connection);
var client = new DedupStoreClient(dedupStoreHttpClient, maxParallelism);
return (client, telemetry);
Expand Down Expand Up @@ -305,7 +322,7 @@ public static async Task<ClientSettingsInfo> GetClientSettingsAsync(

var blobUri = connection.GetClient<ClientSettingsHttpClient>().BaseAddress;
var clientSettingsHttpClient = factory.CreateVssHttpClient<IClientSettingsHttpClient, ClientSettingsHttpClient>(blobUri);
return await clientSettingsHttpClient.GetSettingsAsync(client, userState: null, cancellationToken);
return await clientSettingsHttpClient.GetSettingsAsync(client, userState: null, cancellationToken);
}
catch (Exception exception)
{
Expand All @@ -329,7 +346,7 @@ public static IDomainId GetDefaultDomainId(ClientSettingsInfo clientSettings, IA
tracer.Info($"Error converting the domain id '{clientSettings.Properties[DefaultDomainIdKey]}': {exception.Message}. Falling back to default.");
}
}

return domainId;
}

Expand All @@ -355,5 +372,17 @@ private static HashType GetClientHashType(ClientSettingsInfo clientSettings, Age

return ChunkerHelper.IsHashTypeChunk(hashType) ? hashType : ChunkerHelper.DefaultChunkHashType;
}
}

private int? GetRedirectTimeoutFromClientSettings(ClientSettingsInfo clientSettings)
{
if (int.TryParse(clientSettings?.Properties.GetValueOrDefault(ClientSettingsConstants.RedirectTimeout), out int redirectTimeoutSeconds))
{
return redirectTimeoutSeconds;
}
else
{
return null;
}
}
}
}
10 changes: 8 additions & 2 deletions src/Microsoft.VisualStudio.Services.Agent/JobServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public async Task<BlobIdentifierWithBlocks> UploadLogToBlobStore(Stream blob, st

BlobIdentifier blobId = VsoHash.CalculateBlobIdentifierWithBlocks(blob).BlobId;

// Since we read this while calculating the hash, the position needs to be reset before we send this
// Since we read this while calculating the hash, the position needs to be reset before we send this
blob.Position = 0;

using (var blobClient = CreateArtifactsClient(_connection, default(CancellationToken)))
Expand All @@ -173,7 +173,13 @@ public async Task<BlobIdentifierWithBlocks> UploadLogToBlobStore(Stream blob, st
{
int maxParallelism = HostContext.GetService<IConfigurationStore>().GetSettings().MaxDedupParallelism;
var (dedupClient, clientTelemetry) = await DedupManifestArtifactClientFactory.Instance
.CreateDedupClientAsync(verbose, (str) => Trace.Info(str), this._connection, maxParallelism, cancellationToken);
.CreateDedupClientAsync(
verbose,
(str) => Trace.Info(str),
this._connection,
maxParallelism,
clientType: null,
cancellationToken);

var results = await BlobStoreUtils.UploadToBlobStore(verbose, itemPath, (level, uri, type) =>
new TimelineRecordAttachmentTelemetryRecord(level, uri, type, nameof(UploadAttachmentToBlobStore), planId, jobId, Guid.Empty), (str) => Trace.Info(str), dedupClient, clientTelemetry, cancellationToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.9.8" Condition="$(CodeAnalysis)=='true'" />
<PackageReference Include="Microsoft.Win32.Registry" Version="4.7.0" />
<PackageReference Include="Microsoft.Win32.Registry" Version="5.0.0" />
<PackageReference Include="System.Security.Principal.Windows" Version="6.0.0-preview.5.21301.5" />
<PackageReference Include="System.Text.Encoding.CodePages" Version="4.4.0" />
<PackageReference Include="vss-api-netcore" Version="$(VssApiVersion)" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,13 @@ public class MockDedupManifestArtifactClientFactory : IDedupManifestArtifactClie
telemetrySender));
}

public Task<(DedupStoreClient client, BlobStoreClientTelemetryTfs telemetry)> CreateDedupClientAsync(bool verbose, Action<string> traceOutput, VssConnection connection, int maxParallelism, CancellationToken cancellationToken)
public Task<(DedupStoreClient client, BlobStoreClientTelemetryTfs telemetry)> CreateDedupClientAsync(
bool verbose,
Action<string> traceOutput,
VssConnection connection,
int maxParallelism,
BlobStore.WebApi.Contracts.Client? clientType,
CancellationToken cancellationToken)
{
telemetrySender = new TestTelemetrySender();
return Task.FromResult((client: (DedupStoreClient)null, telemetry: new BlobStoreClientTelemetryTfs(
Expand Down

0 comments on commit 6ee2a6b

Please sign in to comment.