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

Server Timeout Unit is Minutes in MMS, but docstring says Seconds #99

Open
kastman opened this issue Feb 4, 2022 · 0 comments
Open

Comments

@kastman
Copy link

kastman commented Feb 4, 2022

Describe the bug
The model server timeout ("used for model server's backend workers before they are deemed unresponsive and rebooted") currently set in with env vars using SAGEMAKER_MODEL_SERVER_TIMEOUT is listed in seconds in the property method description docstring...

def model_server_timeout(self):  # type: () -> int
    """int: Timeout, in **seconds**, used for model server's backend workers before
    they are deemed unresponsive and rebooted.
    """
    return self._model_server_timeout

...but the actual unit used downstream in multi-model server worker manager is minutes, not seconds.

// TODO: Change this to configurable param
ModelWorkerResponse reply = replies.poll(responseTimeout, TimeUnit.MINUTES);

Because of this, the default timeout of 20 in inference toolkit is actually a 20 minute timeout, not a 20 second timeout.

It seems odd that the unit is minutes, and because this is a parsed as an int in inference-toolkit argparse it does only give a resolution of whole minutes (instead of say, .33 minutes for a 20s equivalent timeout), so should I report this downstream in multi-model-server? If you don't want to change it, we should at least fix the docstring in inference-toolkit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant