Skip to content
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

✨ Maintenance: disable Traefik sticky connection and Socket.IO long polling 🚨 #3686

Conversation

sanderegg
Copy link
Member

@sanderegg sanderegg commented Dec 15, 2022

What do these changes do?

We are in 2022, websockets are available everywhere:

  • no need for socket.io long polling feature
  • therefore no need to continue with having sticky connection defined on traefik (this ensure one client always gets responses from the same webserver)

Removing connection stickyness means that all webserver instances can reply to a client, instead of only one, thus this should improve responsiveness.

Reference on socket.io: https://socket.io/docs/v4/using-multiple-nodes#enabling-sticky-session

This works because currently the webserver instances are all receiving the logs/progress through rabbitMQ (very innefficient, and will be one of the next changes, so keep the hope)

Related issue/s

How to test

  1. connect to osparc with debugging tools open
  2. check the websocket properly connected
  3. use osparc as usual (run computational services, dynamic ones)
  4. check P2P responsivity tests, there might be a slight increase in RPS

Checklist

@codecov
Copy link

codecov bot commented Dec 15, 2022

Codecov Report

Merging #3686 (bd7501d) into master (2fae099) will increase coverage by 0.9%.
The diff coverage is n/a.

❗ Current head bd7501d differs from pull request most recent head 6041a7a. Consider uploading reports for the commit 6041a7a to get more accurate results

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3686     +/-   ##
========================================
+ Coverage    83.4%   84.3%   +0.9%     
========================================
  Files         883     848     -35     
  Lines       37398   35495   -1903     
  Branches      786     786             
========================================
- Hits        31197   29939   -1258     
+ Misses       5992    5347    -645     
  Partials      209     209             
Flag Coverage Δ
integrationtests 67.8% <ø> (+15.0%) ⬆️
unittests 81.3% <ø> (-0.3%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ore_service_director_v2/utils/dask_client_utils.py 80.8% <0.0%> (-6.1%) ⬇️
..._dynamic_sidecar/modules/outputs/_event_handler.py
.../simcore_service_dynamic_sidecar/api/containers.py
...rc/simcore_service_dynamic_sidecar/api/_routing.py
...rvice_dynamic_sidecar/models/schemas/containers.py
...mcore_service_dynamic_sidecar/core/remote_debug.py
.../src/simcore_service_dynamic_sidecar/core/utils.py
.../src/simcore_service_dynamic_sidecar/api/health.py
...ervice_dynamic_sidecar/modules/outputs/__init__.py
...idecar/src/simcore_service_dynamic_sidecar/main.py
... and 50 more

Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

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

Please add a 🚨 icon so we remember to test this (and, if you know some, maybe give instructions for checking this works)

Thanks a lot

@sanderegg sanderegg changed the title ✨ Maintenance: disable Traefik sticky connection and Socket.IO long polling ✨ Maintenance: disable Traefik sticky connection and Socket.IO long polling 🚨 Dec 15, 2022
@sanderegg
Copy link
Member Author

Please add a rotating_light icon so we remember to test this (and, if you know some, maybe give instructions for checking this works)

Thanks a lot

@mrnicegyu11 : done

Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

Tested 👍

@sanderegg sanderegg force-pushed the maintenance/disable_sticky_connection_and_long_polling branch from bd7501d to 6041a7a Compare December 15, 2022 11:52
@sonarcloud
Copy link

sonarcloud bot commented Dec 15, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sanderegg sanderegg merged commit 332cbb4 into ITISFoundation:master Dec 15, 2022
@sanderegg sanderegg deleted the maintenance/disable_sticky_connection_and_long_polling branch December 15, 2022 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants