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

Add timeouts to sdk calls and subscription set-up #604

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

marcelveldt
Copy link
Contributor

While skimming through user logs I noticed that sometimes a call to the sdk (especially the read attribute one) seems to be hanging forever. I think its good for detection and deadlock-preventing to add timeouts to the sdk calls.

I've set the default to 5 minutes per call. Now with a single executor thread handling all calls to the SDK that seems reasonable. If calls take (even) longer than 5 minutes to complete, something is seriously wrong.

@marcelveldt marcelveldt added the maintenance Code (quality) improvement or small enhancement which not a new feature label Feb 29, 2024
Copy link
Collaborator

@agners agners left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, the SDK should have it's own timeout. When we abort here on Matter Server side, the call will continue anyways... That said, I am not against adding this. It might help to find and analyze such long running calls. We just need to make sure that we also can identify those as this.

matter_server/server/device_controller.py Outdated Show resolved Hide resolved
matter_server/server/device_controller.py Outdated Show resolved Hide resolved
@marcelveldt marcelveldt merged commit 74b0ca3 into main Feb 29, 2024
4 checks passed
@marcelveldt marcelveldt deleted the timeout-subscription-setup branch February 29, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Code (quality) improvement or small enhancement which not a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants