Add Fake TPU e2e Autoscaling Test Cases #2279
Conversation
| test.Expect(GetGroupPods(test, rayCluster, "tpu-group")).To(HaveLen(4)) | ||
|
|
||
| // Terminating one TPU detached actor will result in the Ray node becoming idle, causing Ray to scale down the entire multi-host | ||
| // worker group. A new multi-host worker group will then be scaled back up since the remaining detached actors are running. |
There was a problem hiding this comment.
This behavior seems a bit unexpected to me. What's the reason we expect a scale down and a scale up again in this scenario?
There was a problem hiding this comment.
I might just be mis-understanding this comment. Should there be an assertion for this part?
A new multi-host worker group will then be scaled back up since the remaining detached actors are running.
There was a problem hiding this comment.
Detached actors keep running when the Ray node they're scheduled on is scaled down, so the autoscaler sees the request for TPUs and scales back up a multi-host worker group to meet the unmet demand. In a regular scenario (i..e non-detached actors), the actors would be terminated along with their respective nodes when the replica scales down.
There was a problem hiding this comment.
I can add an assertion that checks that the pod list length becomes 0 before becoming 4 again
There was a problem hiding this comment.
Ah I see, I missed the behavior specific to detached actors.
There was a problem hiding this comment.
I can add an assertion that checks that the pod list length becomes 0 before becoming 4 again
sgtm!
There was a problem hiding this comment.
I ended up removing this section in 0c6bb58, because getting the node to become idle requires setting the timeout to 5+ minutes which I'd imagine would slow down the presubmit too much. The behavior to scale down a multi-host replica is still tested by deleting the detached actors.
|
cc: @kevin85421 |
|
I plan to include this PR in v1.3.0 instead. |
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com> Fix tests for updated format Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
ryanaoleary
force-pushed
the
tpu-fake-test
branch
from
e95f608 to
fd552da
Compare
|
Related Issue: |
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
|
I'm moving the multi-host test case to a separate PR based on offline discussion. For the V2 autoscaler only I'm seeing issues with maxReplicas being enforced (the V1 test case passes but V2 fails to ever delete the Pod), I'll open an issue with reproduction steps to see if I can root cause. |
Why are these changes needed?
This PR adds a fake TPU test case, similar to the existing fake GPU test case for autoscaling, that uses detached actors to verify that single-host autoscaling behaves as expected. The behaviors tested included:
resources: {"TPU": 4}will scale up a Ray TPU workerreplicas * numOfHostsEdit: Removed test behavior for idle nodes being scaled down, since KubeRay TPU Pods are un-schedulable due to required nodeSelectors.
Related issue number
Checks