Fix myq increasing number of network connections

[ehendrix23]

Description:

Updated pymyq version to 1.2.0. The aiohttp client session is now setup within the pymyq library instead with tested socket options to prevent continuous increase in sockets on certain platforms.

Related issue (if applicable): fixes #9193

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.

If the code communicates with devices, web services, or third-party tools:

• New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.

[awarecan]

Can you elaborate what changes in your session fixed the leak socket issue? enable_cleanup_closed=True?

If so, maybe we should change aiohttp_client to default turn on this option.

[ehendrix23]

So this is what I have found out so far. This is experience and testing with aiohttp for 2 components that were having issues on certain deployments of HassIO.

Harmony component (through library aioharmony) is using websockets from aiohttp. People were having regular disconnects and then re-connections failing. Sometimes also with errors like "Network unreachable" etc...
In that one a solution was to set the connection to be socket.AF_INET and verify_ssl=False. This is based on some info I found for aiohttp with others running into similar issues:
aio-libs/aiohttp#2522
aio-libs/aiohttp#3171

The whole conversation related to aioharmony is here: https://community.home-assistant.io/t/harmony-hub-problems-after-update/98460

Next issue with myq. Somewhat similar but they would just see the number of connections keep on increasing even although it's supposed to keep the connection open and using pooling (all managed through aiohttp).
Hence I started off with same settings. Issue was still there only now happening every 30 seconds or so instead of every 10 seconds (every 10 seconds a request is sent).
I then set limit_per_host to 5 and added enable_cleanup_closed=True.
After that change is was reported back to overall it only had 1 connection open, with a maximum of 2 connections.
That is still below the limit_per_host of 5.

What I currently do now know however is what if it is only enable_cleanup_closed=True and family is not set. I know with the issue on websockets I had to set it to resolve that issue.

[awarecan]

I believe enable_cleanup_closed=True is the key, can you try to only set it?

It seems happened in particular website, which didn't handle the SSL connection close gracefully.

[pvizeli]

You can create a new websession with same connector pool with the helper. I would prefere that one because it need less resource and home assistant is care that this session will be closed: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/helpers/aiohttp_client.py#L49

[ehendrix23]

You can create a new websession with same connector pool with the helper. I would prefere that one because it need less resource and home assistant is care that this session will be closed: https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/helpers/aiohttp_client.py#L49

It's not about creating new sessions. It's about aiohttp "leaking" connections. Same connection pool is being used but and idea is same connection is used all the time. Yet on certain platforms they were seeing a constant increase in number of connections.
Was using the HA aiohttp client before to create the web session.

[ehendrix23]

Just checking

Can you elaborate what changes in your session fixed the leak socket issue? enable_cleanup_closed=True?

If so, maybe we should change aiohttp_client to default turn on this option.
They confirmed this indeed helps.

[ehendrix23]

Not sure what needs to be done further to get this approved. I see a number of new checks sitting there though.

[codecov]

Codecov Report

Merging #22432 into dev will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##              dev   #22432   +/-   ##
=======================================
  Coverage   93.83%   93.83%           
=======================================
  Files         448      448           
  Lines       36574    36574           
=======================================
  Hits        34321    34321           
  Misses       2253     2253

Impacted Files	Coverage Δ
homeassistant/helpers/aiohttp_client.py	95.34% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49a2f5a...49daa00. Read the comment docs.

[awarecan]

I think it has been confirmed that enable_cleanup_closed=True works, can you change the pull request to change aiohttp_client add this option instead of maintenance your own session.

[ehendrix23]

I think it has been confirmed that enable_cleanup_closed=True works, can you change the pull request to change aiohttp_client add this option instead of maintenance your own session.

Changed it back to use aiohttp helper client from HA and updated aiohttp helper client to include this option.

[awarecan]

@pvizeli

Per aiohttp document

enable_cleanup_closed (bool) – some SSL servers do not properly complete SSL shutdown process, in that case asyncio leaks ssl connections. If this parameter is set to True, aiohttp additionally aborts underlining transport after 2 seconds. It is off by default.

However add this option will apply to all clients, would you mind take a look into it?

[balloob]

I think that it's fine.