Skip to content

[do not review] OOM tets #5434

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

Open
SergeyGaluzo wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[do not review] OOM tets #5434

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
45 changes: 10 additions & 35 deletions src/Microsoft.Health.Fhir.Core/Features/Search/Parameters/SearchParameterOperations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,26 +308,13 @@ public async Task GetAndApplySearchParameterUpdates(CancellationToken cancellati
_searchParameterDefinitionManager.UpdateSearchParameterStatus(searchParam.Uri.OriginalString, SearchParameterStatus.PendingDelete);
}

// Identify all System Defined Search Parameters and filter them from statuses
var systemDefinedSearchParameterUris = new HashSet<string>(
_searchParameterDefinitionManager.AllSearchParameters
.Where(p => p.IsSystemDefined)
.Select(p => p.Url.OriginalString),
StringComparer.Ordinal);

var statusesToFetch = statuses
.Where(p => p.Status == SearchParameterStatus.Enabled || p.Status == SearchParameterStatus.Supported)
.Where(p => !systemDefinedSearchParameterUris.Contains(p.Uri.OriginalString)).ToList();
var statusesToProcess = statuses.Where(p => p.Status == SearchParameterStatus.Enabled || p.Status == SearchParameterStatus.Supported).ToList();

// Batch fetch all SearchParameter resources in one call
var searchParamResources = await GetSearchParametersByUrls(
statusesToFetch
.Select(p => p.Uri.OriginalString)
.ToList(),
cancellationToken);
var searchParamResources = await GetSearchParametersByUrls(statusesToProcess.Select(p => p.Uri.OriginalString).ToList(), cancellationToken);

var paramsToAdd = new List<ITypedElement>();
foreach (var searchParam in statusesToFetch)
foreach (var searchParam in statusesToProcess)
{
if (!searchParamResources.TryGetValue(searchParam.Uri.OriginalString, out var searchParamResource))
{
Expand All @@ -345,16 +332,9 @@ public async Task GetAndApplySearchParameterUpdates(CancellationToken cancellati
}

paramsToAdd.Add(searchParamResource);

// Add parameters incrementally per chunk to reduce peak memory footprint
if (paramsToAdd.Count >= 100)
{
_searchParameterDefinitionManager.AddNewSearchParameters(paramsToAdd);
paramsToAdd.Clear();
}
}

// Add any remaining parameters
// Now add the new or updated parameters to the SearchParameterDefinitionManager
if (paramsToAdd.Any())
{
_searchParameterDefinitionManager.AddNewSearchParameters(paramsToAdd);
Expand All @@ -363,7 +343,7 @@ public async Task GetAndApplySearchParameterUpdates(CancellationToken cancellati
// Once added to the definition manager we can update their status
await _searchParameterStatusManager.ApplySearchParameterStatus(statuses, cancellationToken);

var inCache = ParametersAreInCache(statusesToFetch, cancellationToken);
var inCache = await ParametersAreInCache(statusesToProcess, cancellationToken);

if (results.LastUpdated.HasValue && inCache) // this should be the ony place in the code to assign last updated
{
Expand All @@ -374,23 +354,19 @@ public async Task GetAndApplySearchParameterUpdates(CancellationToken cancellati
// This should handle racing condition between saving new parameter on one VM and refreshing cache on the other,
// when refresh is invoked between saving status and saving resource.
// This will not be needed when order of saves is reversed (resource first, then status)
private bool ParametersAreInCache(IReadOnlyCollection<ResourceSearchParameterStatus> statuses, CancellationToken cancellationToken)
private async Task<bool> ParametersAreInCache(IReadOnlyCollection<ResourceSearchParameterStatus> statuses, CancellationToken cancellationToken)
{
var inCache = true;
foreach (var status in statuses)
{
_searchParameterDefinitionManager.TryGetSearchParameter(status.Uri.OriginalString, out var existingSearchParam);
using IScoped<ISearchService> search = _searchServiceFactory.Invoke();
if (existingSearchParam == null)
{
inCache = false;
var msg = $"Did not find in cache uri={status.Uri.OriginalString} status={status.Status}";
_logger.LogInformation(msg);

// if the parameter was updated in the last 10 minutes it's possible we hit race condition
// where status was updated but resource is not yet saved, so we should not consider this as cache miss
if (status.LastUpdated > DateTimeOffset.UtcNow.AddMinutes(-10))
{
inCache = false;
}
await search.Value.TryLogEvent("SearchParameterOperations.GetAndApplySearchParameterUpdates", "Error", msg, null, cancellationToken);
}
}

Expand All @@ -416,7 +392,7 @@ private async Task<Dictionary<string, ITypedElement>> GetSearchParametersByUrls(
return new Dictionary<string, ITypedElement>();
}

const int chunkSize = 100;
const int chunkSize = 1500;
var searchParametersByUrl = new Dictionary<string, ITypedElement>();

// Process URLs in chunks to avoid SQL query limitations
Expand All @@ -431,7 +407,6 @@ private async Task<Dictionary<string, ITypedElement>> GetSearchParametersByUrls(
var queryParams = new List<Tuple<string, string>>
{
new Tuple<string, string>("url", urlQueryValue),
new Tuple<string, string>("_count", chunkSize.ToString()), // we only need a maximum of chunkSize results back
};

var result = await search.Value.SearchAsync(KnownResourceTypes.SearchParameter, queryParams, cancellationToken);
Expand Down
Loading