Skip to content

Reindex job reliability improvements #5331

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

Merged
jestradaMS merged 5 commits into from
Jan 14, 2026
Merged

Reindex job reliability improvements #5331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5dadd9f
Fix OOM in ReindexProcessingJob with memory-safe batching
jestradaMS Jan 14, 2026
c3433ff
Implement OOM handling in ReindexProcessingJob to reduce batch size a…
jestradaMS Jan 14, 2026
4c8a5d8
Implement retry policies for search parameter status updates to handl…
jestradaMS Jan 14, 2026
d0a929a
Implement retry policies for bulk updates to handle SQL timeouts and …
jestradaMS Jan 14, 2026
f870b30
Add retry policy for search parameter updates to handle SQL timeouts …
jestradaMS Jan 14, 2026
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
35 changes: 31 additions & 4 deletions src/Microsoft.Health.Fhir.Core/Features/Operations/Reindex/ReindexOrchestratorJob.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Microsoft.Data.SqlClient;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Health.Abstractions.Exceptions;
using Microsoft.Health.Extensions.DependencyInjection;
using Microsoft.Health.Fhir.Core.Configs;
using Microsoft.Health.Fhir.Core.Extensions;
Expand Down Expand Up @@ -55,6 +56,27 @@ public sealed class ReindexOrchestratorJob : IJob
.Handle<SqlException>(ex => ex.IsExecutionTimeout())
.WaitAndRetryAsync(3, _ => TimeSpan.FromMilliseconds(RandomNumberGenerator.GetInt32(1000, 5000)));

/// <summary>
/// Retry policy for Cosmos DB 429 (TooManyRequests) errors.
/// Uses the RetryAfter hint from Cosmos DB if available, otherwise waits 1-5 seconds.
/// </summary>
private static readonly AsyncPolicy _requestRateRetries = Policy
Copy link
Contributor

@mikaelweave mikaelweave Jan 14, 2026

Choose a reason for hiding this comment

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

👏👏👏

But also - why only retry 3 times? I think a background job could retry many, many times and still be okay. For reindex, the job could persist through a period of heavy traffic.

Also could be worth seeing if we can use the background retry policy from this class:

https://github.com/microsoft/fhir-server/blob/main/src/Microsoft.Health.Fhir.CosmosDb/Features/Storage/RetryExceptionPolicyFactory.cs

Copy link
Contributor Author

@jestradaMS jestradaMS Jan 14, 2026

Choose a reason for hiding this comment

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

considered moving it there as well, but wanted to keep it simple and isolated to what is needed in Reindex for now.

Regarding retry count, chose 3 just to remain consistent and as a happy medium to failing fast

Copy link
Contributor

@mikaelweave mikaelweave Jan 14, 2026

Choose a reason for hiding this comment

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

Given your knowledge of customer experience, would it make sense to put these in a configuration class that can be overriden by env variable in a production environment?

.Handle<RequestRateExceededException>()
.WaitAndRetryAsync(
3,
(retryAttempt, exception, context) =>
{
var rrException = exception as RequestRateExceededException;
return rrException?.RetryAfter ?? TimeSpan.FromMilliseconds(RandomNumberGenerator.GetInt32(1000, 5000));
},
(exception, timeSpan, retryAttempt, context) => Task.CompletedTask);

/// <summary>
/// Combined retry policy for search parameter status updates.
/// Handles both SQL Server timeouts and Cosmos DB 429 errors.
/// </summary>
private static readonly AsyncPolicy _searchParameterStatusRetries = Policy.WrapAsync(_requestRateRetries, _timeoutRetries);

private HashSet<long> _processedJobIds = new HashSet<long>();
private HashSet<string> _processedSearchParameters = new HashSet<string>();
private List<JobInfo> _jobsToProcess;
Expand Down Expand Up @@ -183,7 +205,9 @@ private async Task RefreshSearchParameterCache(CancellationToken cancellationTok
_logger.LogJobInformation(_jobInfo, "Performing full SearchParameter database refresh and hash recalculation for reindex job.");

// Use the enhanced method with forceFullRefresh flag
await _searchParameterOperations.GetAndApplySearchParameterUpdates(cancellationToken, forceFullRefresh: true);
// Wrapped with retry policy for SQL timeouts and Cosmos DB 429 errors
await _searchParameterStatusRetries.ExecuteAsync(
async () => await _searchParameterOperations.GetAndApplySearchParameterUpdates(cancellationToken, forceFullRefresh: true));

// Update the reindex job record with the latest hash map
_reindexJobRecord.ResourceTypeSearchParameterHashMap = _searchParameterDefinitionManager.SearchParameterHashMap;
Expand Down Expand Up @@ -756,12 +780,14 @@ private async Task UpdateSearchParameterStatus(List<JobInfo> completedJobs, List
{
case SearchParameterStatus.PendingDisable:
_logger.LogJobInformation(_jobInfo, "Reindex job updating the status of the fully indexed search parameter, parameter: '{ParamUri}' to Disabled.", searchParameterUrl);
await _searchParameterStatusManager.UpdateSearchParameterStatusAsync(new List<string>() { searchParameterUrl }, SearchParameterStatus.Disabled, cancellationToken);
await _searchParameterStatusRetries.ExecuteAsync(
async () => await _searchParameterStatusManager.UpdateSearchParameterStatusAsync(new List<string>() { searchParameterUrl }, SearchParameterStatus.Disabled, cancellationToken));
_processedSearchParameters.Add(searchParameterUrl);
break;
case SearchParameterStatus.PendingDelete:
_logger.LogJobInformation(_jobInfo, "Reindex job updating the status of the fully indexed search parameter, parameter: '{ParamUri}' to Deleted.", searchParameterUrl);
await _searchParameterStatusManager.UpdateSearchParameterStatusAsync(new List<string>() { searchParameterUrl }, SearchParameterStatus.Deleted, cancellationToken);
await _searchParameterStatusRetries.ExecuteAsync(
async () => await _searchParameterStatusManager.UpdateSearchParameterStatusAsync(new List<string>() { searchParameterUrl }, SearchParameterStatus.Deleted, cancellationToken));
_processedSearchParameters.Add(searchParameterUrl);
break;
case SearchParameterStatus.Supported:
Expand All @@ -774,7 +800,8 @@ private async Task UpdateSearchParameterStatus(List<JobInfo> completedJobs, List
if (fullyIndexedParamUris.Count > 0)
{
_logger.LogJobInformation(_jobInfo, "Reindex job updating the status of the fully indexed search parameter, parameters: '{ParamUris} to Enabled.'", string.Join("', '", fullyIndexedParamUris));
await _searchParameterStatusManager.UpdateSearchParameterStatusAsync(fullyIndexedParamUris, SearchParameterStatus.Enabled, _cancellationToken);
await _searchParameterStatusRetries.ExecuteAsync(
async () => await _searchParameterStatusManager.UpdateSearchParameterStatusAsync(fullyIndexedParamUris, SearchParameterStatus.Enabled, _cancellationToken));
_processedSearchParameters.UnionWith(fullyIndexedParamUris);
}
}
Expand Down
Loading
Loading