Skip to content

[APIPUB-114] - Make knownUnremediatedRequests a ConcurrentDictionary#138

Open
ea-mtenhoor wants to merge 1 commit into
Ed-Fi-Alliance-OSS:mainfrom
edanalytics:bug/non-concurrent-collection
Open

[APIPUB-114] - Make knownUnremediatedRequests a ConcurrentDictionary#138
ea-mtenhoor wants to merge 1 commit into
Ed-Fi-Alliance-OSS:mainfrom
edanalytics:bug/non-concurrent-collection

Conversation

@ea-mtenhoor

Copy link
Copy Markdown
Contributor

In API Publisher 1.3.0, if --maxDegreeOfParallelismForPostResourceItem is greater than 1 and a remediation script is specified with --remediationsScriptFile, a non-concurrent collection error will occur when a POST error occurs that is not handled by the remediation script. The code in this PR resolves the error by changing knownUnremediatedRequests from HashSet to ConcurrentDictionary.

Explanation from Cursor:

What’s failing
knownUnremediatedRequests is a normal HashSet<>, which is not safe for use from multiple threads at the same time.

PostResourceProcessingBlocksFactory.cs
Lines 77-77

            var knownUnremediatedRequests = new HashSet<(string resourceUrl, HttpStatusCode statusCode)>();

That same instance is passed into every HandlePostItemMessage call. The block that runs that method is configured with parallelism:

PostResourceProcessingBlocksFactory.cs
Lines 102-112

            var postResourceBlock = new TransformManyBlock<PostItemMessage, ErrorItemMessage>(
                async msg =>
                    await HandlePostItemMessage(
                        ignoredResourceByUrl,
                        msg,
                        options,
                        javaScriptModuleFactory,
                        targetEdFiApiClient,
                        knownUnremediatedRequests,
                        missingDependencyByResourcePath),
                new ExecutionDataflowBlockOptions { MaxDegreeOfParallelism = options.MaxDegreeOfParallelismForPostResourceItem });

So whenever MaxDegreeOfParallelismForPostResourceItem is greater than 1, several posts can run at once. Polly’s retry delegate can then run on different logical threads and both:

  • Add (line 200), and
  • Contains inside MayHaveRemediation (line 464),
    can touch the HashSet concurrently. That matches the exception: “Operations that change non-concurrent collections must have exclusive access.”

The stack trace often lands on Add because that’s a mutating operation; Contains concurrent with Add is also undefined for HashSet and can corrupt internal state.

Log message:

2026-03-20 15:40:22,363] [EROR] [17] EdFi.Tools.ApiPublisher.Connections.Api.Processing.Target.Blocks.PostResourceProcessingBlocksFactory - "/ed-fi/sections" (source id: "4adf8da5661b4f039d2a43ec218f7ace"): An unhandled exception occurred in the PostResource block: "System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.\n   at System.Collections.Generic.HashSet`1.AddIfNotPresent(T value, Int32& location)\n   at System.Collections.Generic.HashSet`1.Add(T item)\n   at EdFi.Tools.ApiPublisher.Connections.Api.Processing.Target.Blocks.PostResourceProcessingBlocksFactory.<>c__DisplayClass9_1.<<HandlePostItemMessage>b__6>d.MoveNext() in /home/mark/repos/Ed-Fi-API-Publisher/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/PostResourceProcessingBlocksFactory.cs:line 200\n--- End of stack trace from previous location ---\n   at Polly.Retry.AsyncRetryEngine.ImplementationAsync[TResult](Func`3 action, Context context, ExceptionPredicates shouldRetryExceptionPredicates, ResultPredicates`1 shouldRetryResultPredicates, Func`5 onRetryAsync, CancellationToken cancellationToken, Int32 permittedRetryCount, IEnumerable`1 sleepDurationsEnumerable, Func`4 sleepDurationProvider, Boolean continueOnCapturedContext)\n   at Polly.AsyncPolicy`1.ExecuteInternalAsync(Func`3 action, Context context, Boolean continueOnCapturedContext, CancellationToken cancellationToken)\n   at EdFi.Tools.ApiPublisher.Connections.Api.Processing.Target.Blocks.PostResourceProcessingBlocksFactory.HandlePostItemMessage(ConcurrentDictionary`2 ignoredResourceByUrl, PostItemMessage postItemMessage, Options options, Func`1 javaScriptModuleFactory, EdFiApiClient targetEdFiApiClient, HashSet`1 knownUnremediatedRequests, Dictionary`2 missingDependencyByResourcePath) in /home/mark/repos/Ed-Fi-API-Publisher/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/PostResourceProcessingBlocksFactory.cs:line 231" System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
   at System.Collections.Generic.HashSet`1.AddIfNotPresent(T value, Int32& location)
   at System.Collections.Generic.HashSet`1.Add(T item)
   at EdFi.Tools.ApiPublisher.Connections.Api.Processing.Target.Blocks.PostResourceProcessingBlocksFactory.<>c__DisplayClass9_1.<<HandlePostItemMessage>b__6>d.MoveNext() in /home/mark/repos/Ed-Fi-API-Publisher/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/PostResourceProcessingBlocksFactory.cs:line 200
--- End of stack trace from previous location ---
   at Polly.Retry.AsyncRetryEngine.ImplementationAsync[TResult](Func`3 action, Context context, ExceptionPredicates shouldRetryExceptionPredicates, ResultPredicates`1 shouldRetryResultPredicates, Func`5 onRetryAsync, CancellationToken cancellationToken, Int32 permittedRetryCount, IEnumerable`1 sleepDurationsEnumerable, Func`4 sleepDurationProvider, Boolean continueOnCapturedContext)
   at Polly.AsyncPolicy`1.ExecuteInternalAsync(Func`3 action, Context context, Boolean continueOnCapturedContext, CancellationToken cancellationToken)
   at EdFi.Tools.ApiPublisher.Connections.Api.Processing.Target.Blocks.PostResourceProcessingBlocksFactory.HandlePostItemMessage(ConcurrentDictionary`2 ignoredResourceByUrl, PostItemMessage postItemMessage, Options options, Func`1 javaScriptModuleFactory, EdFiApiClient targetEdFiApiClient, HashSet`1 knownUnremediatedRequests, Dictionary`2 missingDependencyByResourcePath) in /home/mark/repos/Ed-Fi-API-Publisher/src/EdFi.Tools.ApiPublisher.Connections.Api/Processing/Target/Blocks/PostResourceProcessingBlocksFactory.cs:line 231

@github-actions

Copy link
Copy Markdown

Test Results

180 tests   180 ✅  6s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit bb08dc1.

@stevenarnold-ed-fi stevenarnold-ed-fi changed the title Make knownUnremediatedRequests a ConcurrentDictionary [APIPUB-114] - Make knownUnremediatedRequests a ConcurrentDictionary Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant