Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow WorkersManager.THRESHOLD adjustment #2629

Closed
1 task done
notzippy opened this issue Dec 16, 2022 · 1 comment
Closed
1 task done

Allow WorkersManager.THRESHOLD adjustment #2629

notzippy opened this issue Dec 16, 2022 · 1 comment

Comments

@notzippy
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe.

During startup, my app functioned fine on my machine. Failed to start on a colleague with the following error

 [ERROR] Not all workers are ack. Shutting down.
 [ERROR] Experienced exception while trying to serve

Tracing this down to the THRESHOLD value which is a kill-all action. The threshold value which at last release was 50, but I see is now set to 300 is not configurable, and it should be IMO.

I modified my app startup to do

    from sanic.worker  import manager
    manager.WorkerManager.THRESHOLD = 500

Obviously, this is a very fragile solution at best

Describe the solution you'd like

Have the Threshold value configurable. I see in #2610 that someone already modified the value from 50 to 300. But in this scenario, a more configurable solution would make sense.

Additional context

No response

@ahopkins
Copy link
Member

That pattern of monkey patch is the way to handle. There are some other low level values like that so that it's possible to change without really being a config value and overloading the namespace. In v22.12 it will be highly unlikely to need to touch it since exceptions on startup will kill the main process immediately without needing the deadlock check. 30s should be plenty except when it's not and we'll have documentation to that effect and an error pointing devs to it. The solution will be the exact snippet you provided above. Closing this as its no longer an issue on main and will not be on released version in another week or so.

@ahopkins ahopkins closed this as not planned Won't fix, can't repro, duplicate, stale Dec 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants