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.

Sign up for GitHub

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

Batch background review update when parser version changes #5238

Merged
merged 1 commit into from
Jan 31, 2023
Merged

Batch background review update when parser version changes #5238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 17 additions & 2 deletions src/dotnet/APIView/APIViewWeb/HostedServices/ReviewBackgroundHostedService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using APIViewWeb.Managers;
Expand All @@ -16,6 +17,8 @@ public class ReviewBackgroundHostedService : BackgroundService
private readonly bool _isDisabled;
private readonly IReviewManager _reviewManager;
private readonly int _autoArchiveInactiveGracePeriodMonths; // This is inactive duration in months
private readonly HashSet<string> _upgradeDisabledLangs = new HashSet<string>();
private readonly int _backgroundBatchProcessCount;

static TelemetryClient _telemetryClient = new(TelemetryConfiguration.CreateDefault());

Expand All @@ -33,6 +36,18 @@ public ReviewBackgroundHostedService(IReviewManager reviewManager, IConfiguratio
{
_autoArchiveInactiveGracePeriodMonths = 4;
}
var backgroundTaskDisabledLangs = configuration["ReviewUpdateDisabledLanguages"];
if(!string.IsNullOrEmpty(backgroundTaskDisabledLangs))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(!string.IsNullOrEmpty(backgroundTaskDisabledLangs))
if (!string.IsNullOrEmpty(backgroundTaskDisabledLangs))

Copy link
Member Author

Choose a reason for hiding this comment

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

I had missed this comment. I will pull in this change in my next PR. thanks @konrad-jamrozik

{
_upgradeDisabledLangs.UnionWith(backgroundTaskDisabledLangs.Split(','));
}

// Number of review revisions to be passed to pipeline when updating review with a new parser version
var batchCount = configuration["ReviewUpdateBatchCount"];
if (String.IsNullOrEmpty(batchCount) || !int.TryParse(batchCount, out _backgroundBatchProcessCount))
{
_backgroundBatchProcessCount = 20;
}
}

protected override async Task ExecuteAsync(CancellationToken stoppingToken)
Expand All @@ -41,7 +56,7 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
{
try
{
await _reviewManager.UpdateReviewBackground();
await _reviewManager.UpdateReviewBackground(_upgradeDisabledLangs, _backgroundBatchProcessCount);
await ArchiveInactiveReviews(stoppingToken, _autoArchiveInactiveGracePeriodMonths);
}
catch (Exception ex)
Expand Down
2 changes: 1 addition & 1 deletion src/dotnet/APIView/APIViewWeb/Managers/IReviewManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public interface IReviewManager
public Task ApprovePackageNameAsync(ClaimsPrincipal user, string id);
public Task<bool> IsReviewSame(ReviewRevisionModel revision, RenderedCodeFile renderedCodeFile);
public Task<ReviewRevisionModel> CreateMasterReviewAsync(ClaimsPrincipal user, string originalName, string label, Stream fileStream, bool compareAllRevisions);
public Task UpdateReviewBackground();
public Task UpdateReviewBackground(HashSet<string> updateDisabledLanguages, int backgroundBatchProcessCount);
public Task<CodeFile> GetCodeFile(string repoName, string buildId, string artifactName, string packageName, string originalFileName, string codeFileName,
MemoryStream originalFileStream, string baselineCodeFileName = "", MemoryStream baselineStream = null, string project = "public");
public Task<ReviewRevisionModel> CreateApiReview(ClaimsPrincipal user, string buildId, string artifactName, string originalFileName, string label,
Expand Down
182 changes: 109 additions & 73 deletions src/dotnet/APIView/APIViewWeb/Managers/ReviewManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Linq;
using System.Security.Claims;
using System.Text.Json;
using System.Threading;
using System.Threading.Tasks;
using ApiView;
using APIView.DIff;
Expand Down Expand Up @@ -338,26 +339,97 @@ public async Task<ReviewRevisionModel> CreateMasterReviewAsync(ClaimsPrincipal u
return await CreateMasterReviewAsync(user, codeFile, originalName, label, memoryStream, compareAllRevisions);
}

public async Task UpdateReviewBackground()
public async Task UpdateReviewBackground(HashSet<string> updateDisabledLanguages, int backgroundBatchProcessCount)
{
var reviews = await _reviewsRepository.GetReviewsAsync(false, "All", fetchAllPages: true);
foreach (var review in reviews.Where(r => IsUpdateAvailable(r)))
foreach(var language in LanguageService.SupportedLanguages)
{
var requestTelemetry = new RequestTelemetry { Name = "Updating Review " + review.ReviewId };
var operation = _telemetryClient.StartOperation(requestTelemetry);
try
if (updateDisabledLanguages.Contains(language))
{
await Task.Delay(500);
await UpdateReviewAsync(review);
_telemetryClient.TrackTrace("Background task to update API review at startup is disabled for langauge " + language);
continue;
}
catch (Exception e)

var languageService = GetLanguageService(language);
if (languageService == null)
return;

// If review is updated using devops pipeline then batch process update review requests
if (languageService.IsReviewGenByPipeline)
{
_telemetryClient.TrackException(e);
await UpdateReviewsUsingPipeline(language, languageService, backgroundBatchProcessCount);
}
finally
else
{
_telemetryClient.StopOperation(operation);
var reviews = await _reviewsRepository.GetReviewsAsync(false, language, fetchAllPages: true);
foreach (var review in reviews.Where(r => IsUpdateAvailable(r)))
{
var requestTelemetry = new RequestTelemetry { Name = "Updating Review " + review.ReviewId };
var operation = _telemetryClient.StartOperation(requestTelemetry);
try
{
await Task.Delay(500);
await UpdateReviewAsync(review, languageService);
}
catch (Exception e)
{
_telemetryClient.TrackException(e);
}
finally
{
_telemetryClient.StopOperation(operation);
}
}
}
}
}

// Languages that full ysupport sandboxing updates reviews using Azure devops pipeline
// We should batch all eligible reviews to avoid a pipeline run storm
private async Task UpdateReviewsUsingPipeline(string language, LanguageService languageService, int backgroundBatchProcessCount)
{
var reviews = await _reviewsRepository.GetReviewsAsync(false, language, fetchAllPages: true);
var paramList = new List<ReviewGenPipelineParamModel>();

foreach(var review in reviews)
{
foreach (var revision in review.Revisions.Reverse())
{
foreach (var file in revision.Files)
{
//Don't include current revision if file is not required to be updated.
// E.g. json token file is uploaded for a language, specific revision was already upgraded.
if (!file.HasOriginal || file.FileName == null || !languageService.IsSupportedFile(file.FileName) || !languageService.CanUpdate(file.VersionString))
{
continue;
}

_telemetryClient.TrackTrace($"Updating review: {review.ReviewId}, revision: {revision.RevisionId}");
paramList.Add(new ReviewGenPipelineParamModel()
{
FileID = file.ReviewFileId,
ReviewID = review.ReviewId,
RevisionID = revision.RevisionId,
FileName = Path.GetFileName(file.FileName)
});
}
}

// This should be changed to configurable batch count
if (paramList.Count >= backgroundBatchProcessCount)
{
_telemetryClient.TrackTrace($"Running pipeline to update reviews for {language} with batch size {paramList.Count}");
await RunReviewGenPipeline(paramList, languageService.Name);
// Delay of 10 minute before starting next batch
// We should try to increase the number of revisions in the batch than number of runs.
await Task.Delay(600000);
paramList.Clear();
}
}

if (paramList.Count > 0)
{
_telemetryClient.TrackTrace($"Running pipeline to update reviews for {language} with batch size {paramList.Count}");
await RunReviewGenPipeline(paramList, languageService.Name);
}
}

Expand Down Expand Up @@ -627,74 +699,36 @@ public async Task<bool> IsApprovedForFirstRelease(string language, string packag
return reviews.Any();
}

private async Task UpdateReviewOffline(ReviewModel review)

private async Task UpdateReviewAsync(ReviewModel review, LanguageService languageService)
{
var paramList = new List<ReviewGenPipelineParamModel>();
var languageService = GetLanguageService(review.Language);
foreach (var revision in review.Revisions.Reverse())
{
foreach (var file in revision.Files)
{
//Don't include current revision if file is not required to be updated.
// E.g. json token file is uploaded for a language, specific revision was already upgraded.
if (!file.HasOriginal || file.FileName == null || !languageService.IsSupportedFile(file.FileName) || !languageService.CanUpdate(file.VersionString))
if (!file.HasOriginal || !languageService.CanUpdate(file.VersionString))
{
continue;
}
paramList.Add(new ReviewGenPipelineParamModel()
{
FileID = file.ReviewFileId,
ReviewID = review.ReviewId,
RevisionID = revision.RevisionId,
FileName = Path.GetFileName(file.FileName)
});
}
}
await RunReviewGenPipeline(paramList, languageService.Name);
}

private async Task UpdateReviewAsync(ReviewModel review)
{
var languageService = GetLanguageService(review.Language);
if (languageService == null)
return;

if (languageService.IsReviewGenByPipeline)
{
// Wait 30 seconds before running next review gen using pipeline to reduce queueing pipeline jobs
await Task.Delay(30000);
await UpdateReviewOffline(review);
}
else
{
foreach (var revision in review.Revisions.Reverse())
{
foreach (var file in revision.Files)
try
{
if (!file.HasOriginal || !languageService.CanUpdate(file.VersionString))
{
continue;
}

try
{
var fileOriginal = await _originalsRepository.GetOriginalAsync(file.ReviewFileId);
// file.Name property has been repurposed to store package name and version string
// This is causing issue when updating review using latest parser since it expects Name field as file name
// We have added a new property FileName which is only set for new reviews
// All older reviews needs to be handled by checking review name field
var fileName = file.FileName ?? (Path.HasExtension(review.Name) ? review.Name : file.Name);
var codeFile = await languageService.GetCodeFileAsync(fileName, fileOriginal, review.RunAnalysis);
await _codeFileRepository.UpsertCodeFileAsync(revision.RevisionId, file.ReviewFileId, codeFile);
// update only version string
file.VersionString = codeFile.VersionString;
await _reviewsRepository.UpsertReviewAsync(review);
}
catch (Exception ex)
{
_telemetryClient.TrackTrace("Failed to update review " + review.ReviewId);
_telemetryClient.TrackException(ex);
}
var fileOriginal = await _originalsRepository.GetOriginalAsync(file.ReviewFileId);
// file.Name property has been repurposed to store package name and version string
// This is causing issue when updating review using latest parser since it expects Name field as file name
// We have added a new property FileName which is only set for new reviews
// All older reviews needs to be handled by checking review name field
var fileName = file.FileName ?? (Path.HasExtension(review.Name) ? review.Name : file.Name);
var codeFile = await languageService.GetCodeFileAsync(fileName, fileOriginal, review.RunAnalysis);
await _codeFileRepository.UpsertCodeFileAsync(revision.RevisionId, file.ReviewFileId, codeFile);
// update only version string
file.VersionString = codeFile.VersionString;
await _reviewsRepository.UpsertReviewAsync(review);
}
catch (Exception ex)
{
_telemetryClient.TrackTrace("Failed to update review " + review.ReviewId);
_telemetryClient.TrackException(ex);
}
}
}
Expand Down Expand Up @@ -965,9 +999,11 @@ private async Task GenerateReviewOffline(ReviewModel review, string revisionId,
throw new Exception($"Failed to run pipeline for review: {param.ReviewID}, file: {param.FileName}");
}

var paramList = new List<ReviewGenPipelineParamModel>();
paramList.Add(param);

var paramList = new List<ReviewGenPipelineParamModel>
{
param
};

await RunReviewGenPipeline(paramList, languageService.Name);
}

Expand Down