🐛Computational services with large amount of inputs/outputs fail to start (🗃️) #7725
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7725 +/- ##
==========================================
+ Coverage 87.32% 87.34% +0.01%
==========================================
Files 1840 1834 -6
Lines 71475 71266 -209
Branches 1214 1214
==========================================
- Hits 62418 62249 -169
+ Misses 8714 8674 -40
Partials 343 343
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
sanderegg
force-pushed
the
bugfix/comp-tasks-hanlding-of-large-input-outputs
branch
4 times, most recently
from
5894e69 to
f26b047
Compare
sanderegg
changed the title
This was referenced May 23, 2025
sanderegg
requested review from
GitHK,
matusdrobuliak66 and
pcrespov
as code owners
sanderegg
requested review from
JavierGOrdonnez,
bisgaard-itis,
Copilot and
wvangeit
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a bug where computational task notifications fail to start due to oversized payloads by changing the trigger to emit only key identifiers and refactoring the listener to fetch full row data. Tests and mocks are updated to validate the new minimal payload and listener behavior.
- Trigger now sends only
task_id,project_id,node_id,table,action, andchanges - Listener code uses a Pydantic model and queries the DB for full task data
- Tests and fixtures revised to align with minimal payload and new retry logic
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| services/web/server/tests/unit/with_dbs/04/notifications/test_notifications__db_comp_tasks_listening_task.py | Adapted tests for minimal payload, added retry helper and spy |
| services/web/server/src/simcore_service_webserver/db_listener/plugin.py | Removed unused RabbitMQ setup |
| services/web/server/src/simcore_service_webserver/db_listener/_models.py | Added CompTaskNotificationPayload Pydantic model |
| services/web/server/src/simcore_service_webserver/db_listener/_db_comp_tasks_listening_task.py | Refactored listener to use minimal payload and fetch full row |
| packages/pytest-simcore/src/pytest_simcore/db_entries_mocks.py | Added centralized comp_task fixture for task insertion |
| packages/postgres-database/tests/test_comp_tasks.py | Updated expectations for minimal payload and table metadata |
| packages/postgres-database/src/simcore_postgres_database/models/comp_tasks.py | Modified trigger to emit only identifier fields |
| packages/postgres-database/src/simcore_postgres_database/migration/versions/278daef7e99d_remove_whole_row_in_payload.py | New migration removing full-row JSON from payload |
| .github/copilot-instructions.md | Added rules for relative imports and import ordering |
Comments suppressed due to low confidence (2)
services/web/server/tests/unit/with_dbs/04/notifications/test_notifications__db_comp_tasks_listening_task.py:105
- The names
wait_fixedandstop_after_delayare used but not imported; addfrom tenacity.wait import wait_fixedandfrom tenacity.stop import stop_after_delayat the top of the file.
wait=wait_fixed(1),
services/web/server/src/simcore_service_webserver/db_listener/_db_comp_tasks_listening_task.py:70
- The function uses
castbutcastis not imported; addfrom typing import castor importcastfromtyping.
return cast(RowProxy | None, await result.fetchone())
bisgaard-itis
approved these changes
May 24, 2025
sanderegg
force-pushed
the
bugfix/comp-tasks-hanlding-of-large-input-outputs
branch
from
e6d0723 to
81deb76
Compare
wvangeit
approved these changes
May 26, 2025
Contributor
wvangeit
left a comment
wvangeit
left a comment
There was a problem hiding this comment.
Thanks a lot @sanderegg !
sanderegg
force-pushed
the
bugfix/comp-tasks-hanlding-of-large-input-outputs
branch
from
a14b0c2 to
e0eee41
Compare
Member
Author
|
@mergify merge |
Member
Author
|
@mergify queue |
Contributor
❌ Sorry but I didn't understand the command. Please consult the commands documentation 📚. |
Contributor
🛑 The pull request has been merged manuallyThe pull request has been merged manually at 403186c |
sanderegg
force-pushed
the
bugfix/comp-tasks-hanlding-of-large-input-outputs
branch
from
e0eee41 to
a8d86cd
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
What do these changes do?
This PR fixes the issue described in #7722.
The comp_tasks table uses an event trigger and the webservers listens to that event to transmit changes in comp_tasks outputs, state. This includes payload generation for table updates and ensures proper handling of
outputsandstatefields changes. The procedure used to transmit the whole row as payload through pg_notify. This is limited to 8000 bytes.This PR changes the procedure to not pass the row in the payload anymore due to that limitation.
Therefore changes are now in the listener code of the webserver, such that an additional query to comp_tasks is necessary in order to transmit the notifications to the next nodes/frontend.
What is still not good?
Related issue/s
How to test
Dev-ops