-
Notifications
You must be signed in to change notification settings - Fork 32
🐛Sticky connection: Ensure emitted socketio messages for logs, progress, status updates and payments are not lost #7967
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🐛Sticky connection: Ensure emitted socketio messages for logs, progress, status updates and payments are not lost #7967
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7967 +/- ##
==========================================
+ Coverage 87.82% 87.86% +0.04%
==========================================
Files 1849 1842 -7
Lines 71343 71153 -190
Branches 1250 1250
==========================================
- Hits 62657 62519 -138
+ Misses 8324 8272 -52
Partials 362 362
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR restores RabbitMQ-backed delivery for Socket.IO messages by default to prevent loss of logs, progress updates, status changes, and payment events.
- Default
ignore_queueflag is switched toFalsein the core API and redundantignore_queue=Truecalls are removed. - Tests and Docker Compose sticky-routing labels are updated to reflect the new default behavior.
- RabbitMQ consumers and context managers are adjusted to use queued emits.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| services/web/server/tests/integration/02/notifications/test_rabbitmq_consumers.py | Test now expects ignore_queue=False. |
| services/web/server/src/simcore_service_webserver/socketio/messages.py | Updated send_message_to_user signature default and docstring. |
| services/web/server/src/simcore_service_webserver/projects/_projects_service.py | Removed explicit ignore_queue=True in project notifications. |
| services/web/server/src/simcore_service_webserver/payments/_socketio.py | Removed ignore_queue=True for payment notification emits. |
| services/web/server/src/simcore_service_webserver/notifications/_rabbitmq_exclusive_queue_consumers.py | Stripped redundant ignore_queue=True usage and refactored with block syntax. |
| services/docker-compose.yml | Added sticky-routing rules for open/close node operations and set router priority. |
Comments suppressed due to low confidence (3)
services/web/server/src/simcore_service_webserver/socketio/messages.py:65
- [nitpick] The docstring’s default notation
{False}may be confusing; consider using a clearer style (e.g., “Defaults to False”) or a standard Sphinx/Google doc param block for consistency.
ignore_queue: bool = False,
services/web/server/src/simcore_service_webserver/payments/_socketio.py:31
- With
ignore_queue=Trueremoved here, consider adding an integration or unit test to verify that payment completion events now correctly route through RabbitMQ when clients are connected to different server instances.
),
services/web/server/src/simcore_service_webserver/projects/_projects_service.py:1961
- Since
ignore_queue=Truewas removed for project state updates, adding a test to cover cross-instance delivery of project notifications could prevent regressions.
app,
YuryHrytsuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick fix!
Note based on internal discussion:
- this PR also fixes priorities for sticky router (they were wrong before)
matusdrobuliak66
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
@mergify queue |
🟠 Waiting for conditions to match
|
0a6268d to
2cca3b1
Compare
2cca3b1 to
3620d3f
Compare
|
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 2d1134f |
|
…ss, status updates and payments are not lost (#7967)



What do these changes do?
Since #7839 (>2 weeks ago), the flow of logs/progresses was broken, as messages sent via SocketIO were not sent through the socketIO RabbitMQ Pub/Sub system but purposely ignoring the queuing since with a sticky connection it was unnecessary.
A second identified problem is that subscription/unsubscription to the project logs/progress RabbitMQ queue needs to be sticky as otherwise the webserver replica will not properly unsubscribe from a topic.
This PR fixes this issue.
Related issue/s
How to test
Dev-ops