Skip to content
This repository has been archived by the owner on Jul 30, 2024. It is now read-only.
/ NuGet.Jobs Public archive

Commit

Permalink
Optimize packageid:{id} version:{version} ignoreFilter=true case (#605)
Browse files Browse the repository at this point in the history
This case covered the gallery hijack for OData Packages(Id=,Version=).
This accounts for 22.47% of all requests in the past 90 days.
As with the previous optimization PR (#602) switching from a search to a document lookup cut the query time in roughly a half.
Progress on NuGet/NuGetGallery#6466
  • Loading branch information
joelverhagen committed Jul 22, 2019
1 parent 465245a commit 895f444
Show file tree
Hide file tree
Showing 11 changed files with 619 additions and 11 deletions.
14 changes: 14 additions & 0 deletions src/NuGet.Services.AzureSearch/AzureSearchTelemetryService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -373,5 +373,19 @@ public void TrackV3GetDocument(TimeSpan elapsed)
Prefix + "V3GetDocumentMs",
elapsed.TotalMilliseconds);
}

public void TrackV2GetDocumentWithSearchIndex(TimeSpan elapsed)
{
_telemetryClient.TrackMetric(
Prefix + "V2GetDocumentWithSearchIndexMs",
elapsed.TotalMilliseconds);
}

public void TrackV2GetDocumentWithHijackIndex(TimeSpan elapsed)
{
_telemetryClient.TrackMetric(
Prefix + "V2GetDocumentWithHijackIndexMs",
elapsed.TotalMilliseconds);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,7 @@ void TrackDownloadCountDecrease(
void TrackAuxiliary2AzureSearchCompleted(JobOutcome outcome, TimeSpan elapsed);
IDisposable TrackUploadDownloadsSnapshot(int packageIdCount);
void TrackV3GetDocument(TimeSpan elapsed);
void TrackV2GetDocumentWithSearchIndex(TimeSpan elapsed);
void TrackV2GetDocumentWithHijackIndex(TimeSpan elapsed);
}
}
50 changes: 50 additions & 0 deletions src/NuGet.Services.AzureSearch/SearchService/AzureSearchService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Threading.Tasks;
using NuGet.Services.AzureSearch.Wrappers;
using NuGetGallery;

namespace NuGet.Services.AzureSearch.SearchService
{
Expand Down Expand Up @@ -119,6 +120,42 @@ private async Task<V2SearchResponse> UseHijackIndexAsync(V2SearchRequest request
V2SearchResponse output;
switch (operation.Type)
{
case IndexOperationType.Get:
var documentResult = await Measure.DurationWithValueAsync(
() => _hijackIndex.Documents.GetOrNullAsync<HijackDocument.Full>(operation.DocumentKey));

// If the request is excluding SemVer 2.0.0 packages and the document is SemVer 2.0.0, filter it
// out. The must be done after fetching the document because some SemVer 2.0.0 packages are
// SemVer 2.0.0 because of a dependency version or because of build metadata (e.g. 1.0.0+metadata).
// Neither of these reasons is apparent from the request. Build metadata is not used for comparison
// so if someone searchs for "version:1.0.0+foo" and the actual package version is "1.0.0" or
// "1.0.0+bar" the document will still be returned.
//
// A request looking for a specific package version that is SemVer 2.0.0 due to dots in the
// prerelease label (e.g. 1.0.0-beta.1) could no-op if the request is not including SemVer 2.0.0
// but that's not worth the effort since we can catch that case after fetching the document anyway.
// It's more consistent with the search operation path to make the SemVer 2.0.0 filtering decision
// solely based on the document data.
//
// Note that the prerelease filter is ignored here by design. This is legacy behavior from the
// previous search implementation.
var document = documentResult.Value;
if (document != null
&& !request.IncludeSemVer2
&& document.SemVerLevel == SemVerLevelKey.SemVer2)
{
document = null;
}

output = _responseBuilder.V2FromHijackDocument(
request,
operation.DocumentKey,
document,
documentResult.Duration);

_telemetryService.TrackV2GetDocumentWithHijackIndex(documentResult.Duration);
break;

case IndexOperationType.Search:
var result = await Measure.DurationWithValueAsync(() => _hijackIndex.Documents.SearchAsync<HijackDocument.Full>(
operation.SearchText,
Expand Down Expand Up @@ -148,6 +185,19 @@ private async Task<V2SearchResponse> UseSearchIndexAsync(V2SearchRequest request
V2SearchResponse output;
switch (operation.Type)
{
case IndexOperationType.Get:
var documentResult = await Measure.DurationWithValueAsync(
() => _searchIndex.Documents.GetOrNullAsync<SearchDocument.Full>(operation.DocumentKey));

output = _responseBuilder.V2FromSearchDocument(
request,
operation.DocumentKey,
documentResult.Value,
documentResult.Duration);

_telemetryService.TrackV2GetDocumentWithSearchIndex(documentResult.Duration);
break;

case IndexOperationType.Search:
var result = await Measure.DurationWithValueAsync(() => _searchIndex.Documents.SearchAsync<SearchDocument.Full>(
operation.SearchText,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,22 @@ V2SearchResponse V2FromSearch(
SearchParameters parameters,
DocumentSearchResult<SearchDocument.Full> result,
TimeSpan duration);
V2SearchResponse V2FromHijackDocument(
V2SearchRequest request,
string documentKey,
HijackDocument.Full document,
TimeSpan duration);
V3SearchResponse V3FromSearch(
V3SearchRequest request,
string text,
SearchParameters parameters,
DocumentSearchResult<SearchDocument.Full> result,
TimeSpan duration);
V2SearchResponse V2FromSearchDocument(
V2SearchRequest request,
string documentKey,
SearchDocument.Full document,
TimeSpan duration);
V3SearchResponse V3FromSearchDocument(
V3SearchRequest request,
string documentKey,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Linq;
using NuGet.Indexing;
using NuGet.Packaging;
using NuGet.Versioning;

namespace NuGet.Services.AzureSearch.SearchService
{
Expand Down Expand Up @@ -39,6 +40,13 @@ public IndexOperation V3Search(V3SearchRequest request)
public IndexOperation V2SearchWithSearchIndex(V2SearchRequest request)
{
var parsed = _textBuilder.ParseV2Search(request);

IndexOperation indexOperation;
if (TryGetSearchDocumentByKey(request, parsed, out indexOperation))
{
return indexOperation;
}

var text = _textBuilder.Build(parsed);
var parameters = _parametersBuilder.V2Search(request);
return IndexOperation.Search(text, parameters);
Expand All @@ -47,6 +55,13 @@ public IndexOperation V2SearchWithSearchIndex(V2SearchRequest request)
public IndexOperation V2SearchWithHijackIndex(V2SearchRequest request)
{
var parsed = _textBuilder.ParseV2Search(request);

IndexOperation indexOperation;
if (TryGetHijackDocumentByKey(request, parsed, out indexOperation))
{
return indexOperation;
}

var text = _textBuilder.Build(parsed);
var parameters = _parametersBuilder.V2Search(request);
return IndexOperation.Search(text, parameters);
Expand Down Expand Up @@ -79,6 +94,26 @@ private bool TryGetSearchDocumentByKey(
return false;
}

private bool TryGetHijackDocumentByKey(
SearchRequest request,
ParsedQuery parsed,
out IndexOperation indexOperation)
{
if (PagedToFirstItem(request)
&& parsed.Grouping.Count == 2
&& TryGetSinglePackageId(parsed, out var packageId)
&& TryGetSingleVersion(parsed, out var normalizedVersion))
{
var documentKey = DocumentUtilities.GetHijackDocumentKey(packageId, normalizedVersion);

indexOperation = IndexOperation.Get(documentKey);
return true;
}

indexOperation = null;
return false;
}

private bool TryGetSinglePackageId(
ParsedQuery parsed,
out string packageId)
Expand All @@ -98,6 +133,24 @@ private bool TryGetSinglePackageId(
return false;
}

private bool TryGetSingleVersion(
ParsedQuery parsed,
out string normalizedVersion)
{
if (parsed.Grouping.TryGetValue(QueryField.Version, out var terms)
&& terms.Count == 1)
{
if (NuGetVersion.TryParse(terms.First(), out var parsedVersion))
{
normalizedVersion = parsedVersion.ToNormalizedString();
return true;
}
}

normalizedVersion = null;
return false;
}

private static bool PagedToFirstItem(SearchRequest request)
{
return request.Skip <= 0 && request.Take >= 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,36 @@ public V2SearchResponse V2FromSearch(
p => ToV2SearchPackage(p));
}

public V2SearchResponse V2FromSearchDocument(
V2SearchRequest request,
string documentKey,
SearchDocument.Full document,
TimeSpan duration)
{
return ToResponse(
request,
_options.Value.SearchIndexName,
documentKey,
document,
duration,
p => ToV2SearchPackage(p));
}

public V2SearchResponse V2FromHijackDocument(
V2SearchRequest request,
string documentKey,
HijackDocument.Full document,
TimeSpan duration)
{
return ToResponse(
request,
_options.Value.HijackIndexName,
documentKey,
document,
duration,
p => ToV2SearchPackage(p, request.IncludeSemVer2));
}

public V3SearchResponse V3FromSearchDocument(
V3SearchRequest request,
string documentKey,
Expand Down Expand Up @@ -206,6 +236,52 @@ private static string TitleThenId(IBaseMetadataDocument document)
return document.PackageId;
}

private V2SearchResponse ToResponse<T>(
V2SearchRequest request,
string indexName,
string documentKey,
T document,
TimeSpan duration,
Func<T, V2SearchPackage> toPackage)
where T : class
{
var data = new List<V2SearchPackage>();
if (document != null)
{
var package = toPackage(document);
package.Debug = request.ShowDebug ? new DebugDocumentResult { Document = document } : null;
data.Add(package);
}

if (request.CountOnly)
{
return new V2SearchResponse
{
TotalHits = data.Count,
Debug = DebugInformation.CreateFromGetOrNull(
request,
indexName,
documentKey,
duration,
AuxiliaryData.Metadata),
};
}
else
{
return new V2SearchResponse
{
TotalHits = data.Count,
Data = data,
Debug = DebugInformation.CreateFromGetOrNull(
request,
indexName,
documentKey,
duration,
AuxiliaryData.Metadata),
};
}
}

private V2SearchResponse ToResponse<T>(
V2SearchRequest request,
SearchParameters parameters,
Expand Down
7 changes: 6 additions & 1 deletion tests/BasicSearchTests.FunctionalTests.Core/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@ public static class Constants
{
// Predefined Texts
public const string TestPackageId = "BaseTestPackage";
public const string TestPackageIdSemVer2 = "BaseTestPackage.SemVer2";
public const string TestPackageId_SearchFilters = "BaseTestPackage.SearchFilters";
public const string TestPackageId_SemVer2 = "BaseTestPackage.SemVer2";
public const string TestPackageVersion = "1.0.0";
public const string TestPackageVersion_SearchFilters_Default = "1.1.0";
public const string TestPackageVersion_SearchFilters_Prerel = "1.2.0-beta";
public const string TestPackageVersion_SearchFilters_SemVer2 = "1.3.0+metadata";
public const string TestPackageVersion_SearchFilters_PrerelSemVer2 = "1.4.0-delta.4";
public const string TestPackageTitle = "BaseTestPackage";
public const string TestPackageDescription = "Package description";
public const string TestPackageSummary = "";
Expand Down
Loading

0 comments on commit 895f444

Please sign in to comment.