[serve] Better behavior when deployment constructor fails

[edoakes]

Right now the controller will basically spin trying to continually start and stop replicas. We should do something like an exponential backoff and mark a deployment as failed if the constructor never succeeds.

[edoakes]

@simjay here's another one if you're feeling very ambitious :)

[simjay]

Took awhile to understand how the controller's control loop works! I implemented a quick solution for this. Wanted to verify if it makes sense before I finish it. Here are changes I propose.

• Add backend_health (bool) field in BackendInfo class, which is in BackendState._backend_metadata
• Implement exponential_retry(func, retry=RETEY_COUNT) method in utils.py. This method will simply perform an exponential retry of an input method
• In BackendState.update method, in each iteration for a backend, we check BackendInfo's backend_health is true. If true, perform an exponential retry. If false, ignore.
• If exponential retry fails in the above step, flag backend_health as false.

Let me know if this looks okay!

[edoakes]

@simjay this seems pretty good, but is the intention to just retry forever? This would mean that Deployment.deploy() would just hang forever. I'm thinking that it would make the most sense to fail the deployment after a number of retries. This would cause the deploy call to error and the deployment to be deleted.

[edoakes]

The way I would implement this is to have a failure counter instead of just a boolean flag and do exponential backoff on starting the replicas until we hit some retry limit (at which point we fail the deployment).

[simjay]

@edoakes Oh maybe my wordings were a bit confusing. I am actually not suggesting an infinite retry.

The exponential_retry(func, retry=RETEY_COUNT) method I am suggesting will only retry RETRY_COUNT[int] times (which can be an user input argument with default value being a static env value). If RETRY_COUNT times of retry failed, then we would set backend_health flag as false, so that the backend's control loop will not continually try to deploy the failed deployment.

In summary, this will be a life cycle of an errornous deployment.

1. User deploy the deployment
2. backend's control loop invokes BackendState's update method.
3. BackendState checks if the deployment(or backend)'s backend_health field is True (it will be True here, as it just got created)
4. BackendState calls exponential_retry for the deployment, with RETRY_COUNT amount of times (which will all fail).
5. Since exponential_retry failed, we set backend_health of the deployment(or backend) as False.
6. (in future) Coming back to the control loop
7. BackendState checks if the deployment(or backend)'s backend_health field is True (it will be False here)
8. We do not try to deploy, since it is failed deployment

Let me know if this makes sense!

[simjay]

BTW, I am assuming that we still want to keep the deployment process as an asynchronous process!

[fehtemam]

I searched really hard but this open issue here is the only thing I could find that based on my limited understanding I think it might be related to my problem. I cut most of my code to keep it as close as I can to the example from the documentation (from here: https://docs.ray.io/en/master/serve/http-servehandle.html#serve-fastapi-http)

My example:

from fastapi import FastAPI
from pydantic import BaseModel
import ray
from ray import serve
import uvicorn

serve.start()

app = FastAPI()

class input(BaseModel):
    x1: str
    x2: str

@serve.deployment(route_prefix="/my_tests")
@serve.ingress(app)
class MyFastAPIDeployment:
    @app.post("/")
    def root(request: input):
        return {"message": f"{request} done!"}

MyFastAPIDeployment.deploy()

if __name__ == "__main__":
    uvicorn.run("api:app", host="127.0.0.1", port=8000, reload=False)

This creates an infinite loop with Serve printing these messages:
Adding 1 replicas to backend 'MyFastAPIDeployment'.
Starting HTTP proxy with name 'TrXIAU:SERVE_CONTROLLER_ACTOR:SERVE_PROXY_ACTOR-node:xxx.xx.xx.xx-0'
Updating deployment 'MyFastAPIDeployment'.

Now if I change the type of the request from the custom type defined in the class to anything else (e.g., str, dict, list, ...) then starting the server will be fine. So something like this works:

@serve.deployment(route_prefix="/my_tests")
@serve.ingress(app)
class MyFastAPIDeployment:
    @app.post("/")
    def root(request: str):
        return {"message": f"{request} done!"}

My issue is probably related to not using the class properly since my understanding of how to set this up is limited. But I just wanted to say that if the process fails I think it shouldn't get stuck in an infinite loop. It should just give an error.

[edoakes]

@fehtemam looks like your question was answered on Slack, but reposting here for future searchers:

You don't need to call uvicorn.run to run the FastAPI app yourself--that's taken care of by Ray Serve. So here is a version of your script which should work (you'll need to call ray start --head before running, and call ray stop later when you want to shut it down):

import ray
from ray import serve
from fastapi import FastAPI
from pydantic import BaseModel

ray.init(address="auto", namespace="serve")
serve.start(detached=True)

app = FastAPI()

class input(BaseModel):
	x1: str

def my_func(x1):
	x = x1 + x1

@app.post("/run_test")
def root(request: input):
	my_func(request.x1)
	return {"message": f"{request.x1} is done."}

@serve.deployment(route_prefix="/run_test")
@serve.ingress(app)
class Test:
   pass 

Test.deploy()