python: retry ingress POST past transient pipeline unreachability - #6851
Conversation
mihaibudiu
left a comment
There was a problem hiding this comment.
Is this the only test that may have this problem?
No. Good catch. I'll follow up with a more comprehensive version. |
…le race start_pipeline()/resume_pipeline()/start_pipeline_as_paused() (wait=True) only wait for the control plane's deployment_status to report "running"; the pod itself can still be briefly unreachable through its k8s service afterward (the control plane's own readiness probe is a point-in-time check, not re-verified before the next caller's request). Calls through the SDK client already retry transient 503s, but tests/platform/helper.py's raw REST helpers are deliberately no-retry (other tests rely on that to see raw status codes), so they're exposed whenever they're the first pod-facing call after a start/resume. Add wait_for_pipeline_reachable(), which polls /stats until it stops returning 503 PipelineInteractionUnreachable, and call it at each of the 25 sites across 7 test files where that pattern occurs. test_pipeline_error.py (the one that actually flaked in CI) now uses the shared helper instead of its original bespoke retry loop. Signed-off-by: Ben Pfaff <blp@feldera.com>
95126a4 to
5d950b9
Compare
|
This new version is intended to catch all the cases where the tests started or resumed a pipeline and then immediately did a raw request that doesn't itself retry on a 503. Take another look @mihaibudiu ? |
mihaibudiu
left a comment
There was a problem hiding this comment.
Is there a dynamic check we could add to catch this if it's introduced in new tests?
"You may have forgotten to call 'wait_for_pipeline_reachable'"?
|
|
||
| - Calls made through the `feldera.Pipeline`/`FelderaClient` SDK (e.g. | ||
| `pipeline.input_json`, `pipeline.query`) already retry transient 503s | ||
| themselves, so the race resolves on its own. |
There was a problem hiding this comment.
no need to mention the race here
One approach I considered was to just do it every time inside the functions like start_pipeline_as_paused(). The downside, if there could be said to be one, is that we wouldn't get as much testing of the ability of the usual functions that do know to retry 503 failures. Any thoughts? |
|
I was thinking to have a tiny state machine someplace which said "I have checked on startup for 503". And if you check for another error while not in this state you get a failure. But this is clearly an improvement, and maybe people will just copy-paste this from now on. |
Right after start() reports "running", the pipeline pod can briefly be unreachable through the k8s service (control plane confirms readiness once, but nothing re-checks it before the SDK's next request). This raced with cause_error_and_wait_for_stopped's initial ingest POST, which uses the no-retry http_request test helper, and flaked test_pipeline_error_dismissal in CI with a 503 PipelineInteractionUnreachable.
This failure was observed in the CI run
https://github.com/feldera/cloud/actions/runs/31617894273/job/94194277123