lazcad / homeassistant

Home Assistant Development
249 stars 83 forks source link

Sync socket API usage breaks MainThread on long requests #38

Open PaulAnnekov opened 7 years ago

PaulAnnekov commented 7 years ago

Looks like this component is doing its work synchronous in the MainThread. That's unacceptable. Each socket request blocks main HA thread = blocks all the core. If your gateway or anybody between HA and a gateway (e.g. router) will respond too long (I think more than 1 sec), thread will be blocked for this time. E.g. my router likes to make socket timeouts (>10 secs) at least 1 time a day, thread is blocked for 10 seconds, it looks like this:

...
home_assistant_1  | 17-03-29 20:40:47 DEBUG (MainThread) [custom_components.xiaomi] >> b'{ "cmd":"read","sid":"128d001123ca9a"}'
home_assistant_1  | 17-03-29 20:40:49 ERROR (Thread-7) [homeassistant.components.light.yeelight] Unable to update bulb status: Bulb closed the connection.
home_assistant_1  | 17-03-29 20:40:57 ERROR (MainThread) [custom_components.xiaomi] Cannot connect to Gateway
home_assistant_1  | 17-03-29 20:40:57 ERROR (MainThread) [custom_components.xiaomi] No data in response from hub None
home_assistant_1  | 17-03-29 20:40:57 ERROR (MainThread) [homeassistant.core] Timer got out of sync. Resetting
...

These calls should be rewritten with asyncore or it should use the same API as other components use (check this and everything below to understand how it should be implemented).

DeviantEng commented 7 years ago

Yup, I'm also having this issue. I'm finding that this xiaomi component is now causing CPU spikes on a system that is normally pretty calm. Unfortunately I've had to remove this component for the time being. :(

EDIT: This is my CPU usage (over the last hour). The drop is when I disabled the Xioami component and restarted hass. http://imgur.com/a/w4TkN

Danielhiversen commented 7 years ago

I have fixed this in my fork: https://github.com/Danielhiversen/homeassistant You are welcome to test that.

PaulAnnekov commented 7 years ago

@Danielhiversen great. Switched to your repo, will check. First impression: finally, socket requests are in separate Thread :+1: .

vsternbach commented 7 years ago

@Danielhiversen Hi, is there any special reason your fork isn't merged into master?

Danielhiversen commented 7 years ago

Ask @ravetam. I opened a pull request in February, but @ravetam seems to ignore it.

vsternbach commented 7 years ago

That's what I thought, don't you want maybe to setup this repository as a separate repository and not a fork, so that people can open issues and pull requests directly to your repository?