Reindex testing improvements and fixes #5240
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
Simplified the search parameter status filtering in ReindexOrchestratorJob by removing unnecessary dictionary creation. The new implementation directly filters the collection to get valid URIs, which is more straightforward and handles case-variant duplicates naturally. Changes: - Removed intermediate dictionary for O(1) lookups by URI - Changed to direct LINQ filtering on the status collection - Removed case-insensitive string comparer (now uses default equality) - Updated comment to clarify handling of case-variant duplicates - Added comprehensive unit tests for the reindex orchestrator job - Removed unused MediatR import and added System.Threading.Tasks from ReindexOrchestratorJobTests This refactoring maintains the same functional behavior while improving code readability and includes better test coverage.
**Changes:** - Modified GetReindexRequestHandler to check for active reindex jobs before listing all jobs - Added GetActiveReindexJob method to retrieve single active job when available - Enhanced ReindexOrchestratorJob to verify resource counts without ignoring hash values - Fixed logic error in CheckJobRecordForAnyWork (changed <= to > for count comparison) - Added zero-count resource detection and JobRecordCount adjustment to prevent processing empty ranges
- Adding E2E tests for Reindex - Fixing SortTests
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
- Add check for cancelled jobs when determining failure status - Prevent incorrect status updates when in-flight jobs exist during race conditions - Only create reindex processing jobs if none exist to avoid duplicates - Add early cancellation checks before creating and enqueueing jobs - Ensure status remains Running if in-flight jobs exist - Added loop to handle continuation tokens when working with resources from Cosmos - Added missing filter to QueryBuilder for Cosmos DB to filter is history as well as is deleted - Added check to CosmosQueueClient to prevent race condition of submitting new jobs when cancellation was requested - Added MaxRunningTaskCount = 4 to Reindex Operation
…ndex tests - Reduce SearchParameterCacheRefreshIntervalSeconds from 5 to 2 seconds - Reduce JobHosting PollingFrequencyInSeconds from 5 to 2 seconds - Add Person resource cleanup in test fixture initialization - Add comprehensive test for mixed zero and non-zero count resources - Include Person resources (2000) alongside Specimen resources (5000) in test data - Create separate search parameters for different resource types - Verify search parameter status updates only after all reindex jobs complete These changes improve test execution speed and ensure proper cleanup of Person resources between test runs while adding coverage for multi-resource reindexing scenarios.
enhancement(ci): added seperate e2e tests task for IndexAndReindex categories Adding in new template (e2e-test-reindex.yml) for handling IndexAndReindex test execution in parallel to other tests to not add additional time to verall e2e tests. Extract the Azure PowerShell variable setup logic from e2e-tests.yml into a separate reusable template (e2e-set-variables.yml). This improves maintainability by centralizing the Key Vault secret retrieval, storage account configuration, and environment variable setup logic that can be shared across multiple pipeline jobs.
- Add retry to Verify to account for race conditions - Added specimen.subject
- Removed unncessary check in orchestrator - updated run-test.yml job ordering
…n ReindexProcessingJob to ensure that should any race conditions occur where a job is dequeued before next background sync runs it does not fail the jobs.
testing change to prevent status from impacting hashmap.
changing opperand on staus check to !=
jestradaMS
added
Azure Healthcare APIs
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…s. force deleting before deleting RG and prevent restore failures
…if it exists, if not delete it.
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…h the exception of KeyVaults.
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Description
This pull request introduces several improvements and fixes to the FHIR reindexing operation, as well as enhancements to end-to-end (E2E) testing infrastructure. The main changes include more accurate resource counting during reindexing, configuration overrides for faster E2E reindex tests, and improved logging and error handling. Additionally, new E2E test files and documentation for reindexing have been added.
Reindexing logic and correctness improvements:
ReindexOrchestratorJob: After calculating initial resource ranges, the code now double-checks and corrects counts for each resource type, ensuring that zero-count resource types are accurately reflected and subtracted from the total job count. This prevents unnecessary processing of empty resource ranges.Testing infrastructure and configuration:
ReindexTestFixtureclass for E2E tests, which overrides configuration to minimize reindex delays and polling intervals, and ensures test data is cleaned up before and after tests. This results in faster and more reliable E2E reindex tests.ReindexTestFixture.cs,ReindexTests.cs) and related documentation (README.md) in the project items. [1] [2]Logging and error handling:
ReindexProcessingJobby using structured logging with named parameters, making logs easier to parse and analyze.Related issues
Addresses AB#178077, AB#175213, AB#178083
Testing
Describe how this change was tested.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)