Reindex core fixes and test enhancements #5264
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ures Add RetryFactAttribute and supporting infrastructure to enable automatic retry of flaky tests in xunit. This is particularly useful for integration and end-to-end tests that may experience transient failures. Changes: - Add RetryFactAttribute with configurable max retries, delay, and assertion failure handling - Add RetryFactDiscoverer for test case discovery - Add RetryTestCase to implement retry logic with detailed diagnostics - Refactor existing tests to use RetryFact where appropriate for transient failures The retry mechanism defaults to 3 attempts with 5 second delays and skips assertion failures unless explicitly configured, helping distinguish between genuine test bugs and environmental issues.
…tests - Set SearchParameterCacheRefreshIntervalSeconds to 1 second in tests for faster execution - Add SearchParameterCacheRefreshMaxInitialDelaySeconds config option with default of 15 seconds to prevent thundering herd on timer startup - Set initial delay to 0 in tests to eliminate random startup delays - Increase test timeout from 200ms to 2000ms to account for timer initialization - Fix reindex operation status logic to check for cancelled jobs and in-flight jobs to prevent incorrect status transitions during race conditions - Fix to Gen1 to ensure all records are retrieve when continuation token is supplied - Fix to Gen1 GetReindexResults query to ensure we only get Active records to reindex - Fix to ensure we have correct hashmap at time of execution of reindex jobs vs. what was suppliled during creation in handler
jestradaMS
added
Enhancement-Test
1 task
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
SergeyGaluzo
reviewed
Dec 9, 2025
src/Microsoft.Health.Fhir.Core/Features/Operations/FhirOperationDataStoreBase.cs
Outdated
Show resolved
Hide resolved
SergeyGaluzo
reviewed
Dec 9, 2025
src/Microsoft.Health.Fhir.Core/Features/Operations/FhirOperationDataStoreBase.cs
Show resolved
Hide resolved
SergeyGaluzo
reviewed
Dec 9, 2025
src/Microsoft.Health.Fhir.Core/Features/Operations/Reindex/ReindexOrchestratorJob.cs
Show resolved
Hide resolved
Base automatically changed from
users/jestrada/retryfact-infrastructure
to
main
December 9, 2025 21:41
|
|
||
| /// <summary> | ||
| /// Gets or sets the maximum initial delay in seconds for the SearchParameter cache background service timer. | ||
| /// This random delay (0 to this value) staggers timer startup across instances to prevent thundering herd. |
Contributor
There was a problem hiding this comment.
I have never heard the term 'thundering herd' used to describe an influx of traffic due to start up across instances. I like it :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
##Dependent on #5263
Description
This pull request introduces several improvements and bug fixes to the reindexing workflow and search parameter cache refresh logic in the FHIR server codebase. The main focus is on ensuring more robust handling of search parameter updates, improving job orchestration for reindexing, and addressing concurrency and cancellation scenarios. Key changes include enhancements to the search parameter cache refresh configuration, more accurate job status updates, improved resource counting logic, and better handling of job cancellation and heartbeat conflicts.
Search Parameter Cache Refresh Improvements
SearchParameterCacheRefreshMaxInitialDelaySecondstoCoreFeatureConfigurationto allow for a randomized initial delay when starting the cache refresh background service, helping to prevent thundering herd issues. The default is 15 seconds, and it can be set to 0 for testing.Reindex Job Orchestration and Processing
ReindexOrchestratorJobto always refresh the search parameter cache before and after job creation, and after updating zero-count search parameters, ensuring the most up-to-date search parameter definitions are used throughout the reindex process. [1] [2] [3]OperationCanceledExceptionpromptly when cancellation is requested, both before job creation and when enqueuing processing jobs.Job Status and Error Handling
Failedonly when there are no in-flight or cancelled jobs, preventing premature failure status in race conditions. Also, if there are still in-flight jobs, status is set toRunningto avoid sending incorrect consumer status. [1] [2]JobConflictExceptionwithout failing the job, allowing for concurrent updates by multiple workers.Resource Counting and Hash Map Handling
ReindexJobRecordtointernal setfor better encapsulation and updated the logic for retrieving hash values to ensure correctness. [1] [2]CheckJobRecordForAnyWorkto correctly identify when there is work to be done based on resource counts.Other Codebase Improvements
Related issues
Addresses AB#175213, AB#178077, AB#178083.
Testing
Describe how this change was tested.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)