Skip to content

Conversation

@bisgaard-itis
Copy link
Contributor

@bisgaard-itis bisgaard-itis commented Jun 26, 2025

What do these changes do?

  • Remove disable_on_disconnect decorator from 2 endpoints in the api-server: The one for getting upload links for a file multipart upload and the one for completing the multipart upload

Motivation for this PR

Joel (from the sim4life desktop team) has experienced that the api-server can sometimes hang in requests. I recently introduced timeouts in the api-server clients "speaking" with storage. So that is no longer the issue. I.e. the issue must be in the api-server itself. In order to reproduce I wrote a small script which uploads 100 different files concurrently via the api-server and observed that on my local deployment I could indeed make some of the requests hang. The api-server was still responsive, but some requests never received a response. I put a breakpoint in the api-server and investigated the tasks scheduled in the event loop and I could see that some of the "poller tasks" from the disable_on_disconnect decorator were missing their "twin" (these tasks are supposed to come in pairs). This suggests that, after the api-server had processed the request, it didn't manage to kill the "poller task" and the response is not being sent back to the client until that task is gone.

Follow up

  • This is pushed to master so that Joel can try out if this fix also works for him. In that case me and @giancarloromeo will sit down and propose a proper solution. If the cancle_on_disconnect is indeed the issue then it is important that we find a proper solution because that guy is used everywhere.

Related issue/s

How to test

Dev-ops

@bisgaard-itis bisgaard-itis requested a review from pcrespov as a code owner June 26, 2025 09:37
@bisgaard-itis bisgaard-itis self-assigned this Jun 26, 2025
@bisgaard-itis bisgaard-itis added bug buggy, it does not work as expected a:apiserver api-server service labels Jun 26, 2025
@bisgaard-itis bisgaard-itis added this to the Engage milestone Jun 26, 2025
@bisgaard-itis bisgaard-itis changed the title ⚗️ Remove cancle_on_disconnect decorator from certain api-server endpoints 🐛⚗️ Remove cancle_on_disconnect decorator from certain api-server endpoints Jun 26, 2025
@bisgaard-itis bisgaard-itis changed the title 🐛⚗️ Remove cancle_on_disconnect decorator from certain api-server endpoints 🐛⚗️ Remove cancel_on_disconnect decorator from certain api-server endpoints Jun 26, 2025
@codecov
Copy link

codecov bot commented Jun 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.69%. Comparing base (e7d9064) to head (7628257).
Report is 1 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (e7d9064) and HEAD (7628257). Click for more details.

HEAD has 31 uploads less than BASE
Flag BASE (e7d9064) HEAD (7628257)
unittests 32 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #7986       +/-   ##
===========================================
- Coverage   87.88%   69.69%   -18.20%     
===========================================
  Files        1841      796     -1045     
  Lines       71081    36610    -34471     
  Branches     1231      176     -1055     
===========================================
- Hits        62473    25516    -36957     
- Misses       8252    11036     +2784     
+ Partials      356       58      -298     
Flag Coverage Δ
integrationtests 64.34% <ø> (+0.04%) ⬆️
unittests 92.64% <ø> (+6.15%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 77.22% <ø> (-7.77%) ⬇️
agent ∅ <ø> (∅)
api_server 92.64% <ø> (-0.01%) ⬇️
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 77.68% <ø> (-13.45%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 88.33% <ø> (-1.77%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 59.08% <ø> (-28.55%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7d9064...7628257. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud
Copy link

Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the result of your tests is that we should now add those, please make sure that it is commented in the endpoint (or even better add a failing test) so we do not "undo" it

@bisgaard-itis bisgaard-itis merged commit 7f10771 into ITISFoundation:master Jun 26, 2025
94 of 97 checks passed
@bisgaard-itis bisgaard-itis deleted the 1922-disable-client-disconnect-mechanism branch June 26, 2025 11:12
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:apiserver api-server service bug buggy, it does not work as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants