[DRAFT] Test: Remove ConfigureAwait(false) only - no locking changes #7896
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
…e timing issues This change removes ConfigureAwait(false) from the ExecuteBatch method to ensure async continuations run on the actor's dispatcher thread instead of arbitrary thread pool threads. This maintains proper synchronization context and prevents potential race conditions and deadlocks when WriteMessagesAsync implementations access actor state or context. Testing hypothesis: ConfigureAwait(false) may have been the root cause of intermittent MemoryJournal test failures, not the locking mechanism.
|
Closing this draft PR as investigation revealed the hypothesis was incorrect. The concurrency specialist analysis showed that:
This was a valuable investigation that confirmed the architecture is sound. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Purpose
This is an experimental draft PR to test whether removing
ConfigureAwait(false)fromAsyncWriteJournal.ExecuteBatchis sufficient to fix the intermittentInMemoryEventsByTagSpectest failures, WITHOUT any of the locking or data structure changes.What This Tests
This branch is based on commit
59b2cbb87(before PR #7869 and #7887) and applies ONLY the ConfigureAwait fix.No other changes:
ConfigureAwait(false)from line 395 of AsyncWriteJournal.csHypothesis
The
ConfigureAwait(false)was causing async continuations to run on thread pool threads instead of the actor's dispatcher thread. This created race conditions whenWriteMessagesAsyncimplementations tried to access actor state or when timing-sensitive operations occurred.If this PR's tests pass consistently, it would suggest:
Testing
Focus on:
Akka.Persistence.Query.InMemory.Tests.InMemoryEventsByTagSpec.ReadJournal_live_query_EventsByTag_should_find_events_from_offset_exclusiveThis test has been failing intermittently for a long time and was the motivation for both PR #7869 and PR #7887.