core: improve worker lifetime #9439
Conversation
bougue-pe
added
area:core
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #9439 +/- ##
==========================================
- Coverage 37.84% 37.83% -0.01%
==========================================
Files 990 990
Lines 90915 90920 +5
Branches 1176 1176
==========================================
- Hits 34404 34401 -3
- Misses 56057 56065 +8
Partials 454 454
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
bougue-pe
force-pushed
the
peb/core/improve_worker_lifetime
branch
from
58671c6
to
3bc9bad
Compare
core/src/main/kotlin/fr/sncf/osrd/api/api_v2/pathfinding/PathfindingBlocksEndpointV2.kt
Outdated
Show resolved
Hide resolved
core/src/main/kotlin/fr/sncf/osrd/api/api_v2/standalone_sim/SimulationEndpoint.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
LGTM (not tested though)
Though I feel a little uncomfortable about the isCacheable flag in ErrorType. Apparently it was there but not set for any error, now we set it for InfraSoftLoadingError. But then most of the logic is made by checking for InfraSoftLoadingError specifically.
Maybe we could just remove the flag? Or always use it instead of checking for this specific error.
I'm OK to remove the flag (I won't have the time to do it before 4/11/2024).
|
bougue-pe
force-pushed
the
peb/core/improve_worker_lifetime
branch
3 times, most recently
from
a243a35
to
da81c3d
Compare
This led to cases where the worker would hang forever when throwing exception. Signed-off-by: Pierre-Etienne Bougué <bougue.pe@proton.me>
The worker is able to process requests if infra load: * ... went fine, happy path (just process request) * ... failed with a perennial error (expected behavior is reject requests on the same version of the same infra) If infra load failed with a temporary error (and after possible retries), better just stop and let orchestrator decide to retry or not. No retry implemented so far. Signed-off-by: Pierre-Etienne Bougué <bougue.pe@proton.me>
So far any recoverable error is "cacheable/serializable" for simulation or pathfinding. Signed-off-by: Pierre-Etienne Bougué <bougue.pe@proton.me>
make sure handle() is called only when not recoverable Signed-off-by: Pierre-Etienne Bougué <bougue.pe@proton.me>
bougue-pe
force-pushed
the
peb/core/improve_worker_lifetime
branch
from
954a328
to
401947e
Compare
🔍 review the commits separately, ignore whitespace on first commit.
fixes #9388
Done:
Details:
The worker is able to process requests if infra load:
If infra load failed with a temporary error (and after possible retries), better just stop and let orchestrator decide to retry or not.
No retry implemented so far (easier to test the behavior... and easier to code 😇).
Hand-tested on the cases I could think of and reproduce, more test is welcome.