Skip to content

Commit

Permalink
Batch background review update when parser version changes (#5238)
Browse files Browse the repository at this point in the history
  • Loading branch information
@praveenkuttappan
praveenkuttappan authored Jan 31, 2023
1 parent 63c91f7 commit 01fc067
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 76 deletions.
19 changes: 17 additions & 2 deletions src/dotnet/APIView/APIViewWeb/HostedServices/ReviewBackgroundHostedService.cs
View file
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))
{
_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
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
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

0 comments on commit 01fc067

Please sign in to comment.