[serve] Rolling updates for redeployments

[edoakes]

Why are these changes needed?

Performs a rolling update when re-deploying a backend. Currently, the batch size of the update is hardcoded to floor(20%) of the target number of replicas. We can make this configurable in the future.

This follows the same protocol as k8s:

• Old replicas are turned down before new ones are started.
• At most batch_size old replicas will be stopped and new replicas will be started at once.

Related issue number

Closes #14805

Checks

• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

[simon-mo]

It's going to take sometime to review this, ETA tuesday noon.

[edoakes]

@simon-mo this the core logic to review FYI

[architkulkarni]

I looked at the core logic and I had a couple of concerns, but I might be misunderstanding or misremembering something. I think it's close though! Overall more comments would be helpful.

[edoakes]

@simon-mo @architkulkarni I updated the tests to be easier to read and added two new test cases. I also addressed the issue @architkulkarni pointed out about target_replicas being below the current pending_replicas & added some more comments explaining the logic.

Please take another look and let me know what's still confusing / if you see any issues in the logic.

[simon-mo]

in this version, seems like we are not counting number of new version replicas that are starting? namely if the following is > 0, we are breaking the invariant the max number of replicas in transition.

replicas.count(
            version=target_version, states=[ReplicaState.STARTING, ReplicaState.SHOULD_START])

[architkulkarni]

I think this part might be okay--the expression you've written is contributes to pending_replicas.

Concretely, if target_replicas = current_replicas = 10 replicas, and it consists of 8 old RUNNING and 2 new STARTING, then we have

pending_replicas = 10 - 8 - 0 = 2

so max_to_stop is 2-2 = 0 and nothing gets stopped. (Until the new_version replicas move from STARTING to RUNNING, as desired.)

[architkulkarni]

Logic and new tests look good! The comments are helpful. I think the core logic would still be hard to understand for someone new to the code but I can't really think of a way to make it much clearer. One idea might be to move some of the algebra into the algorithm itself--specifically, actually pop all the replicas we can stop for "free" (namely, old SHOULD START and old STARTING) at the beginning, so we never have to account for them in the rest of the function. In any case, that can be done in a future PR (if at all), and the tests you wrote will make it much easier.

[edoakes]

@architkulkarni I totally agree, I'm not very happy with the readability/understandability, but I couldn't find a better way to express it. I tried splitting out the "rolling update" and "regular scaling" logic into separate codepaths, but it ended up being both hard to make correct and added some redundancy. Hopefully we can find a way to clean this up in the near future.