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

🚨🐛 Mitigate dy-service file upload bug: Increase traefik timeouts #6395

Merged

Conversation

mrnicegyu11
Copy link
Member

@mrnicegyu11 mrnicegyu11 commented Sep 19, 2024

What do these changes do?

The traefik timeout defaults changed between minor versions 2.10 and 2.11: https://doc.traefik.io/traefik/migration/v2/#v211

This PR also adds the increased timeouts of 6h that have been set in the ops-traefik to the simcore traefik, as dy-services such as S4L do fileupload using a singular POST requests that goes via client --> ops-traefik -- > simcore-traefik -- > caddy --> nginx. When the REST requests' long-running conenction exceeds 60s, traefik >2.10by default severs the connection. Nginx indicates this client disconnect by responding the unique status code 499.

Related issue/s

How to test

Dev-ops checklist

@mrnicegyu11 mrnicegyu11 added bug buggy, it does not work as expected High Priority a totally crucial bug/feature to be fixed asap labels Sep 19, 2024
@mrnicegyu11 mrnicegyu11 added this to the MartinKippenberger milestone Sep 19, 2024
@mrnicegyu11 mrnicegyu11 self-assigned this Sep 19, 2024
@mrnicegyu11 mrnicegyu11 marked this pull request as ready for review September 19, 2024 08:41
Copy link

1 similar comment
Copy link

Copy link
Contributor

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

Thanks

Copy link

codecov bot commented Sep 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.9%. Comparing base (cafbf96) to head (b0cda02).
Report is 559 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6395      +/-   ##
=========================================
+ Coverage    84.5%   87.9%    +3.3%     
=========================================
  Files          10    1421    +1411     
  Lines         214   58710   +58496     
  Branches       25    2070    +2045     
=========================================
+ Hits          181   51641   +51460     
- Misses         23    6748    +6725     
- Partials       10     321     +311     
Flag Coverage Δ
integrationtests 64.6% <ø> (?)
unittests 85.7% <ø> (+1.2%) ⬆️

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

see 1370 files with indirect coverage changes

@mrnicegyu11 mrnicegyu11 enabled auto-merge (squash) September 19, 2024 08:58
@mrnicegyu11 mrnicegyu11 merged commit f90ca82 into ITISFoundation:master Sep 19, 2024
56 of 59 checks passed
@mrnicegyu11
Copy link
Member Author

Forcemerged as told by @matusdrobuliak66

@mguidon
Copy link
Member

mguidon commented Sep 19, 2024

I guess it would be wise to write these findings down for the next time this gets upgraded.

mrnicegyu11 added a commit to mrnicegyu11/osparc-simcore that referenced this pull request Oct 2, 2024
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Oct 30, 2024
57 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug buggy, it does not work as expected High Priority a totally crucial bug/feature to be fixed asap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants