fix(inmemory): stabilize task ordering and lifecycle#643
Open
GefMar wants to merge 2 commits into
Open
Conversation
Ensure post_send completes before local execution while preserving per-invocation ownership and await_inplace semantics. Drain accepted tasks during shutdown, retain background failures, and clean up middleware, result backend, and executor resources reliably. Add concurrency, cancellation, lifecycle, and compatibility coverage. Closes: #586
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #643 +/- ##
==========================================
+ Coverage 81.29% 82.36% +1.07%
==========================================
Files 69 69
Lines 2577 2711 +134
==========================================
+ Hits 2095 2233 +138
+ Misses 482 478 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Identify producer and consumer spans by SpanKind instead of relying on platform-dependent start-time ordering, fixing Windows CI failures.
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
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.
Ensure post_send completes before local execution while preserving per-invocation ownership and await_inplace semantics.
Drain accepted tasks during shutdown, retain background failures, and clean up middleware, result backend, and executor resources reliably.
Add concurrency, cancellation, lifecycle, and compatibility coverage.
Closes: #586
Why This Approach Differs from #639
PR #639 correctly identifies the original problem and the required hook ordering. However, its implementation covers only the sequential happy path and does not establish reliable ownership of locally executed tasks.
Per-Invocation Ownership
PR #639 stores inline tasks in a broker-wide
_inplace_tasksset shared by all concurrent.kiq()calls. One invocation can therefore collect and await another invocation’s task, while the second invocation observes an empty set and returns before its own task completes.This breaks the per-invocation contract of
await_inplace=True.The current implementation assigns each invocation its own execution task and synchronization gate. Every caller waits only for the work it submitted.
Strict
post_sendOrderingCreating the receiver with
asyncio.create_task()before runningpost_senddoes not guarantee strict ordering. When an asynchronouspost_sendhook suspends, the event loop may startpre_executeand the task body concurrently.The implementation in this PR keeps the receiver behind a per-invocation gate. Execution cannot begin until the complete
post_sendmiddleware chain for that invocation has finished.Error and Cancellation Ownership
If
post_sendfails or is cancelled after the message has been accepted, the execution task still needs an explicit owner.This implementation transfers such work to broker-managed background ownership.
wait_all()andshutdown()can then drain it and propagate execution failures instead of leaving detached tasks behind.It also preserves the existing cancellation behavior for normal inline execution.
Lifecycle Integration
The ownership model is integrated with the complete
InMemoryBrokerlifecycle:wait_all()does not cancel broker-owned work;shutdown()drains accepted tasks before closing middleware, the result backend, and the executor;Test Coverage
The behavior is protected by deterministic tests covering:
post_sendhooks;.kiq()calls;post_sendfailures and cancellation;kick()compatibility;The additional implementation is therefore not ceremony around hook ordering. It defines the complete lifecycle of an accepted message: gated execution, per-invocation or broker ownership, failure propagation, draining, and cleanup.