[RayService][e2e] Fix misplaced max_ongoing_requests in incremental-upgrade serveConfigV2

[justinyeh1995]

Why are these changes needed?

The max_ongoing_requests used in highRPSServeConfigV2 is misplaced.

kuberay/ray-operator/test/e2eincrementalupgrade/constant.go

Line 117 in 55ffdeb

max_ongoing_requests: 6

As Ray Serve Autoscaling points out, it is not part of the autoscaling config. Therefore, the current setup uses a default value of 5 instead of the proclaimed value 6.

https://github.com/ray-project/ray/blob/328e6b7646facad6dc92e37b8db2b478b2089a87/python/ray/serve/_private/constants.py#L283

Also, there is a 1/50 (2×25 runs) chances the RPS >= 400 check would timeout but comparing with the original setup it is not a regression, which also has a chances of failing 1 out of 50 times under 2 runs.

Related issue number

related to #4782

Labels

• If this PR has user-facing changes that require documentation updates at release time, I have added the doc-updates-required label.
• If this PR contains breaking changes, I have added the breaking-change label.

Checks

• I've made sure the tests are passing.
• Testing Strategy
  • Unit tests
  • Manual tests

Manual test instructions

Manual test is recorded in #5098, the change does not introduce a regression.

[justinyeh1995]

cc @win5923 @machichima to decide if this counts as fixing #4782. Thank you.

[win5923]

Thanks! I think we can merge this for now, but should still follow up on the 1/50 failure and figure out what's causing it.
I can also take a look later.

[machichima]

Actually I'm thinking if we could decrease the required RPS from 400 to something like 350. In the PR adding this test, there's a few tuning on this value

ffd38b6

[justinyeh1995]

Thanks! I think we can merge this for now, but should still follow up on the 1/50 failure and figure out what's causing it. I can also take a look later.

Will do. Thanks.

Actually I'm thinking if we could decrease the required RPS from 400 to something like 350. In the PR adding this test, there's a few tuning on this value

ffd38b6

If there is no particular reason to use 400, I think we can turn it down to 350, and run the test again to verify the results. Would love to get more context from @JiangJiaWei1103 if you get the time. Thanks.

[justinyeh1995]

Had an discussion with @machichima, who proposed looking into whether the locust container has undergone cpu throttling. The intuition here is that the client could possibly not able to send enough requests.

[JiangJiaWei1103]

Sorry for the delay. I'm currently busy with relocation. Will take a look tmr. Thx!

[JiangJiaWei1103]

Thanks for working on this! I agree with merging this fix first and lowering the threshold to 350.

In the long term, relying on hardcoded absolute RPS assertions on shared CI runners is flaky due to issues like noisy neighbors on shared host VMs, varying runner hardware specifications across runs, and local resource contention.

We can discuss adopting a more robust benchmark strategy in the original issue (e.g., relative performance comparison against baseline).