ITISFoundation / osparc-simcore

🐼 osparc-simcore simulation framework
https://osparc.io
MIT License
44 stars 26 forks source link

✨ Limit inflight requests to the api-server #6007

Open bisgaard-itis opened 3 days ago

bisgaard-itis commented 3 days ago

What do these changes do?

Related issue/s

How to test

run local deployment with API_SERVER_INFLIGHTREQ_AMOUNT=1 in .env. Then run

seq 10 | parallel -j 10 "curl -X 'GET'   'http://<ip address>.nip.io:<api-server-port>/v0/solvers'   -H 'accept: application/json'   -H 'Authorization: Basic <auth>' -v > output{#}.txt"

with <api-server-port>=the port exposing the api-server through traefik and <auth>=valid authorization. Observe the output*.txt files and see that only a single request received a 2xx return code and the others received a 429.

Dev-ops checklist

codecov[bot] commented 3 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.9%. Comparing base (cafbf96) to head (0922c6b). Report is 310 commits behind head on master.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/6007/graphs/tree.svg?width=650&height=150&src=pr&token=h1rOE8q7ic&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation)](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/6007?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation) ```diff @@ Coverage Diff @@ ## master #6007 +/- ## ========================================= + Coverage 84.5% 87.9% +3.3% ========================================= Files 10 1419 +1409 Lines 214 58212 +57998 Branches 25 1395 +1370 ========================================= + Hits 181 51169 +50988 - Misses 23 6750 +6727 - Partials 10 293 +283 ``` | [Flag](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/6007/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation) | Coverage Δ | | |---|---|---| | [integrationtests](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/6007/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation) | `64.7% <ø> (?)` | | | [unittests](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/6007/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation) | `85.9% <ø> (+1.3%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation#carryforward-flags-in-the-pull-request-comment) to find out more. [see 1384 files with indirect coverage changes](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/6007/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation)
YuryHrytsuk commented 3 days ago

Thanks!

This limit will only take place for requests coming via public network. For the internal (within docker swarm cluster) communication it will not apply.

bisgaard-itis commented 3 days ago

Q: are you adding a sleep+retry in the client for this code?

there is already the retry strategy in place with exponential backoff. That should also be triggered by a 429

bisgaard-itis commented 3 days ago

very nice! Q: what is your strategy to determine this value (e.g. why did you start in 25 simultaneous requests?)

When running Werner's "load test" I see in our grafana dashboard that the api-server is at times handling around 50 requests simultaniously. That seems to add quite a performance overhead, so 25 is chosen to try and avoid that heavy load. Not sure this is optimal of course. So we will have to tune it as we go

bisgaard-itis commented 8 hours ago

@YuryHrytsuk and @mrnicegyu11 can I get you guys (codeowners) to accept this one?