Skip to content

[DO NOT MERGE] repro TestRayServiceIncrementalUpgradeWithLocust RPS<=400 flakiness - #5098

Open
justinyeh1995 wants to merge 19 commits into
ray-project:masterfrom
justinyeh1995:flaky/tune-serveConfigV2-for-e2e-test
Open

[DO NOT MERGE] repro TestRayServiceIncrementalUpgradeWithLocust RPS<=400 flakiness#5098
justinyeh1995 wants to merge 19 commits into
ray-project:masterfrom
justinyeh1995:flaky/tune-serveConfigV2-for-e2e-test

Conversation

@justinyeh1995

@justinyeh1995 justinyeh1995 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Why are these changes needed?

Reproducing RPS <= 400 flakiness on buildkite with 25 distinct jobs on TestRayServiceIncrementalUpgradeWithLocust and TestRayServiceIncrementalUpgradeRollbackMatrixWithLocust to observe the fail-before cases.

some ci failing cases related to this issue.

https://buildkite.com/ray-project/ray-ecosystem-ci-kuberay-ci/builds/16269/list?sid=019fc713-155e-4397-965e-1f48c754c305&tab=output -> shows failures happened in TestRayServiceIncrementalUpgradeWithLocust:BlueGreen、AggressiveGradual、ConservativeGradual

https://buildkite.com/ray-project/ray-ecosystem-ci-kuberay-ci/builds/16394/list?jid=019fd75f-afcb-4932-981c-e6ea5d32bbe0&tab=output -> shoes failures happened in TestRayServiceIncrementalUpgradeRollbackMatrixWithLocust:ThirdSpec、EarlyRollback、FastRollback

Related issue number

related to #4782

Fail Before Repro

commit fc20caa and the two builds above showed this flakiness. so it is worth the fix.

notice that max_ongoing_request is ignored (misplaced, Serve default applied)


Checkpoint 1: no changes introduced. Ran it 25 times.

fc20caa 1/25 fails due to the RPS <= 400 issue

a4fd846 0/25 fails due to the RPS issue, but three jobs failed due to the clean up issue mention below. #5098 (comment)


Checkpoint 2:

Fixed the canonical form, max_ongoing_requests is not part of the autoscalingConfig.

  • Tune target_on_going to 10 and max_ongoing_target to 20.

    • the rationale is we have a set 10 concurrent requests all the time, why don't we allow it on one replicas to reduce the latency and bump max_ongoing_target to way more than 10 so we reduce queue time.
  • 376d838 6 out of 25 failed due to the RPS issue (not sure why but the observation is the truth)
    1 out of 25 failed due to clean up issue.


Checkpoint 3:


Checkpoint 4.

Keep the canonical format change, but revert the max_ongoing_requests back to the original value, 6.

f6a9042 1/25 failed with the RPS check.
b74f79a 0/25 failed due to the RPS check. 2/25 failed due to the flakiness due to the issue mention below.

Results: 1/50 failed to meet the RPS >= 400 requirement. (not a regression from the current set up but fix the canonical format)


Suggestion.

After conducting the experiment, I would suggest max_ongoing_requests stays at the intended 6 since there is no regression and it fits the orignal intent more. The fix will fix the canonical format issue, and use the value, 6.

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
Signed-off-by: Justin Yeh <justinyeh1995@gmail.com>
…ceIncrementalUpgradeRollbackMatrixWithLocust

Signed-off-by: Justin Yeh <justinyeh1995@gmail.com>
Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
Signed-off-by: Justin Yeh <justinyeh1995@gmail.com>
Comment thread ray-operator/test/e2eincrementalupgrade/constant.go Outdated
Signed-off-by: Justin Yeh <justinyeh1995@gmail.com>
Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
@justinyeh1995

justinyeh1995 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

there is a second, unrelated flaky failure in https://buildkite.com/ray-project/ray-ecosystem-ci-kuberay-ci/builds/16452/list?jid=019fdc92-39a1-4bbd-8b37-56c1182ddbd6&tab=output where Locust exits with code 2 during its SIGINT-triggered shutdown.

[2026-08-07T14:40:14Z]           File "/home/ray/anaconda3/lib/python3.10/site-packages/locust/stats.py", line 588, in get_response_time_percentile
--
[2026-08-07T14:40:14Z]             return calculate_response_time_percentile(self.response_times, self.num_requests, percent)
[2026-08-07T14:40:14Z]           File "/home/ray/anaconda3/lib/python3.10/site-packages/locust/stats.py", line 145, in calculate_response_time_percentile
[2026-08-07T14:40:14Z]             num_of_request = int(num_requests * percent)
[2026-08-07T14:40:14Z]         KeyboardInterrupt
...
[2026-08-07T14:41:19Z]     rayservice_incremental_upgrade_test.go:671: Locust load test failed: command terminated with exit code 

Depending on timing, the resulting KeyboardInterrupt may be raised while the CSV stats writer greenlet is executing. Locust then reports the expected shutdown interruption as an unhandled greenlet exception.

Go test
   └─ pkill -SIGINT -f "locust --headless" (async OS signal, 2 independent targets)
         |--- master (own SIGINT, own gevent scheduling)
         |--- worker (own SIGINT, own gevent scheduling)

Each process gets its own SIGINT. Inside each, CPython can only raise it as KeyboardInterrupt into whichever greenlet is actively executing bytecode at that instant. In our case, the master is the one that returns exit code 2.

An important clarification is that this happens specifically in the master process, not the worker. pkill -SIGINT -f "locust --headless" matches both the master and worker processes independently, each gets its own SIGINT and races against its own gevent scheduling separately. But locust_runner.py only spawns the master via subprocess.Popen and only reads its exit code via proc.returncode; the worker's subprocess (spawned inside a separate Ray actor) is never waited on or checked at all. So while both processes are equally exposed to this race, only the master's outcome is ever observed. This tells us the exit code 2 we see is the master's own, not something relayed or propagated from the worker.

The test's cleanup

defer func() {
LogWithTimestamp(test.T(), "Stopping Locust load test")
_, _, _ = ExecPodCmdWithError(
test,
locustHeadPod,
common.RayHeadContainer,
[]string{"pkill", "-SIGINT", "-f", "locust --headless"},
)
LogWithTimestamp(test.T(), "Waiting for Locust load test goroutine to finish")
if err := eg.Wait(); err != nil && !test.T().Failed() {
test.T().Errorf("Locust load test failed: %v", err)
}
}()

then treats that nonzero exit as a hard test failure even when every real assertion in the test had already passed by that point.

A likely fix is to

(a) A likely fix is to trust assert num_failures == 0 instead of sys.exit(proc.returncode), since that's the real correctness signal, and asserted before shutdown even begins. it is also unaffected by which process or which greenlet the race happens to hit.

Not fixing this here since it's unrelated to serveConfigV2 tuning. Will test it in another follow-up. this still requires further investigations.

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
Comment thread ray-operator/test/e2eincrementalupgrade/support.go
Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
@justinyeh1995

Copy link
Copy Markdown
Contributor Author

This is a follow up issue on the second greenlet/SIGINT race issue. #5109

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
…SThreshold to 350.0

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 1dbea77. Configure here.

const (
// The lower bound of the RPS for the Locust to reach the steady state.
locustWarmupRPSThreshold = 400.0
locustWarmupRPSThreshold = 350.0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Warmup RPS gate weakened

Medium Severity

locustWarmupRPSThreshold is lowered from 400 to 350, so warmup succeeds whenever RPS stays in the 350–399 range. That is the only RPS gate in this suite, so the RPS<=400 failures this PR aims to reproduce or guard against no longer fail the test.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1dbea77. Configure here.

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>
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.

1 participant