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

Users/oakeredolu/pipsharedcache #695

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 58 additions & 48 deletions src/Microsoft.ComponentDetection.Detectors/pip/IPyPiClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,12 @@ public async Task<IList<PipDependencySpecification>> FetchPackageDependenciesAsy
var dependencies = new List<PipDependencySpecification>();

var uri = release.Url;
var response = await this.GetAndCachePyPiResponseAsync(uri);
var response = PythonResolverSharedCache.GetFromWheelFileCache(uri);
if (response == null)
{
response = await this.GetAndCachePyPiResponseAsync(uri);
PythonResolverSharedCache.AddToWheelFileCache(uri, response);
}

if (!response.IsSuccessStatusCode)
{
Expand Down Expand Up @@ -138,61 +143,66 @@ public async Task<SortedDictionary<string, IList<PythonProjectRelease>>> GetRele
{
var requestUri = new Uri($"https://pypi.org/pypi/{spec.Name}/json");

var request = await Policy
.HandleResult<HttpResponseMessage>(message =>
{
// stop retrying if MAXRETRIES was hit!
if (message == null)
var request = PythonResolverSharedCache.GetFromProjectCache(spec.Name);
if (request == null)
{
request = await Policy
.HandleResult<HttpResponseMessage>(message =>
{
return false;
}

var statusCode = (int)message.StatusCode;

// only retry if server doesn't classify the call as a client error!
var isRetryable = statusCode < 400 || statusCode > 499;
return !message.IsSuccessStatusCode && isRetryable;
})
.WaitAndRetryAsync(1, i => RETRYDELAY, (result, timeSpan, retryCount, context) =>
{
using var r = new PypiRetryTelemetryRecord { Name = spec.Name, DependencySpecifiers = spec.DependencySpecifiers?.ToArray(), StatusCode = result.Result.StatusCode };

this.logger.LogWarning(
"Received {StatusCode} {ReasonPhrase} from {RequestUri}. Waiting {TimeSpan} before retry attempt {RetryCount}",
result.Result.StatusCode,
result.Result.ReasonPhrase,
requestUri,
timeSpan,
retryCount);

Interlocked.Increment(ref this.retries);
})
.ExecuteAsync(() =>
{
if (Interlocked.Read(ref this.retries) >= MAXRETRIES)
// stop retrying if MAXRETRIES was hit!
if (message == null)
{
return false;
}

var statusCode = (int)message.StatusCode;

// only retry if server doesn't classify the call as a client error!
var isRetryable = statusCode < 400 || statusCode > 499;
return !message.IsSuccessStatusCode && isRetryable;
})
.WaitAndRetryAsync(1, i => RETRYDELAY, (result, timeSpan, retryCount, context) =>
{
return Task.FromResult<HttpResponseMessage>(null);
}

return this.GetAndCachePyPiResponseAsync(requestUri);
});
using var r = new PypiRetryTelemetryRecord { Name = spec.Name, DependencySpecifiers = spec.DependencySpecifiers?.ToArray(), StatusCode = result.Result.StatusCode };

this.logger.LogWarning(
"Received {StatusCode} {ReasonPhrase} from {RequestUri}. Waiting {TimeSpan} before retry attempt {RetryCount}",
result.Result.StatusCode,
result.Result.ReasonPhrase,
requestUri,
timeSpan,
retryCount);

Interlocked.Increment(ref this.retries);
})
.ExecuteAsync(() =>
{
if (Interlocked.Read(ref this.retries) >= MAXRETRIES)
{
return Task.FromResult<HttpResponseMessage>(null);
}

return this.GetAndCachePyPiResponseAsync(requestUri);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this bypassing the cache?

Copy link
Contributor Author

@Omotola Omotola Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be adding to cache here instead. fixed it. There's a check to get from cache at the top of this conditional before it gets to this point

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually I think I misunderstood your question. do you mean is the shared cache bypassing this internal cache? yes it is, i'm checking the shared cache first and if it returns nothing, then it enters this retry block and tries to get from the internal cache and if that returns nothing, it sends a request to the pypi api.

});
if (request == null)
{
using var r = new PypiMaxRetriesReachedTelemetryRecord { Name = spec.Name, DependencySpecifiers = spec.DependencySpecifiers?.ToArray() };

if (request == null)
{
using var r = new PypiMaxRetriesReachedTelemetryRecord { Name = spec.Name, DependencySpecifiers = spec.DependencySpecifiers?.ToArray() };
this.logger.LogWarning($"Call to pypi.org failed, but no more retries allowed!");

this.logger.LogWarning($"Call to pypi.org failed, but no more retries allowed!");
return new SortedDictionary<string, IList<PythonProjectRelease>>();
}

return new SortedDictionary<string, IList<PythonProjectRelease>>();
}
if (!request.IsSuccessStatusCode)
{
using var r = new PypiFailureTelemetryRecord { Name = spec.Name, DependencySpecifiers = spec.DependencySpecifiers?.ToArray(), StatusCode = request.StatusCode };

if (!request.IsSuccessStatusCode)
{
using var r = new PypiFailureTelemetryRecord { Name = spec.Name, DependencySpecifiers = spec.DependencySpecifiers?.ToArray(), StatusCode = request.StatusCode };
this.logger.LogWarning("Received {StatusCode} {ReasonPhrase} from {RequestUri}", request.StatusCode, request.ReasonPhrase, requestUri);

this.logger.LogWarning("Received {StatusCode} {ReasonPhrase} from {RequestUri}", request.StatusCode, request.ReasonPhrase, requestUri);
return new SortedDictionary<string, IList<PythonProjectRelease>>();
}

return new SortedDictionary<string, IList<PythonProjectRelease>>();
PythonResolverSharedCache.AddToProjectCache(spec.Name, request);
}

var response = await request.Content.ReadAsStringAsync();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
namespace Microsoft.ComponentDetection.Detectors.Pip;

using System;
using System.Net.Http;
using Microsoft.Extensions.Caching.Memory;

public static class PythonResolverSharedCache
{
private const long DEFAULTCACHEENTRIES = 4096;
private static readonly MemoryCache SharedProjectCache = new MemoryCache(new MemoryCacheOptions { SizeLimit = DEFAULTCACHEENTRIES });
private static readonly MemoryCache SharedWheelFileCache = new MemoryCache(new MemoryCacheOptions { SizeLimit = DEFAULTCACHEENTRIES });

/// <summary>
/// Adds a uri and its response to the cache.
/// </summary>
/// <param name="specName"> The specName to store.</param>
/// <param name="content">The content to store.</param>
public static void AddToProjectCache(string specName, HttpResponseMessage content)
{
SharedProjectCache.CreateEntry(specName).Value = content;
}

/// <summary>
/// Get a response from the cache.
/// </summary>
/// <param name="specName">The spec name to retrieve.</param>
/// <returns> A HttpResponseMessage. </returns>
public static HttpResponseMessage GetFromProjectCache(string specName)
{
if (SharedProjectCache.TryGetValue(specName, out HttpResponseMessage content))
{
return content;
}

return null;
}

/// <summary>
/// Adds a uri and its response to the cache.
/// </summary>
/// <param name="uri"> The uri to store.</param>
/// <param name="content">The content to store.</param>
public static void AddToWheelFileCache(Uri uri, HttpResponseMessage content)
{
SharedWheelFileCache.CreateEntry(uri).Value = content;
}

/// <summary>
/// Get a response from the cache.
/// </summary>
/// <param name="uri">The uri to retrieve.</param>
/// <returns> A HttpResponseMessage. </returns>
public static HttpResponseMessage GetFromWheelFileCache(Uri uri)
{
if (SharedWheelFileCache.TryGetValue(uri, out HttpResponseMessage content))
{
return content;
}

return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,20 +91,25 @@ public async Task<Stream> FetchPackageFileStreamAsync(Uri releaseUrl)
/// <returns>The cached project file or a new result from Simple PyPi.</returns>
private async Task<Stream> GetAndCacheProjectFileAsync(Uri uri)
{
if (!this.checkedMaxEntriesVariable)
var response = PythonResolverSharedCache.GetFromWheelFileCache(uri);
if (response == null)
{
this.cachedProjectWheelFiles = this.InitializeNonDefaultMemoryCache(this.cachedProjectWheelFiles);
}

if (this.cachedProjectWheelFiles.TryGetValue(uri, out Stream result))
{
this.cacheTelemetry.NumProjectFileCacheHits++;
this.logger.LogDebug("Retrieved cached Python data from {Uri}", uri);
return result;
if (!this.checkedMaxEntriesVariable)
{
this.cachedProjectWheelFiles = this.InitializeNonDefaultMemoryCache(this.cachedProjectWheelFiles);
}

if (this.cachedProjectWheelFiles.TryGetValue(uri, out Stream result))
{
this.cacheTelemetry.NumProjectFileCacheHits++;
this.logger.LogDebug("Retrieved cached Python data from {Uri}", uri);
return result;
}

response = await this.GetPypiResponseAsync(uri);
PythonResolverSharedCache.AddToWheelFileCache(uri, response);
}

var response = await this.GetPypiResponseAsync(uri);

if (!response.IsSuccessStatusCode)
{
this.logger.LogWarning("Http GET at {ReleaseUrl} failed with status code {ResponseStatusCode}", uri, response.StatusCode);
Expand Down Expand Up @@ -144,7 +149,12 @@ private async Task<SimplePypiProject> GetAndCacheSimpleProjectAsync(Uri uri, Pip
return result;
}

var response = await this.RetryPypiRequestAsync(uri, spec);
var response = PythonResolverSharedCache.GetFromProjectCache(spec.Name);
if (response == null)
{
response = await this.RetryPypiRequestAsync(uri, spec);
PythonResolverSharedCache.AddToProjectCache(spec.Name, response);
}

var responseContent = await response.Content.ReadAsStringAsync();
if (string.IsNullOrEmpty(responseContent))
Expand Down Expand Up @@ -270,6 +280,7 @@ private async Task<HttpResponseMessage> GetPypiResponseAsync(Uri uri)
request.Headers.UserAgent.Add(CommentValue);
request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/vnd.pypi.simple.v1+json"));
var response = await this.httpClient.SendAsync(request);

return response;
}

Expand Down