jasonacox / pypowerwall

Python API for Tesla Powerwall and Solar Power Data
MIT License
120 stars 21 forks source link

Extra connection created? #89

Closed KevM closed 1 month ago

KevM commented 2 months ago

I have my pypowerwall.env connection/cache settings like this:

PW_CACHE_EXPIRE=60
PW_POOL_MAXSIZE=1

I am seeing logs that look like this showing a lot of discarded connections. Why is it creating extra connections when there "should only be one"? Could this be a problem?

04/24/2024 09:54:00 AM [urllib3.connectionpool] [WARNING] Connection pool is full, discarding connection: 192.168.7.87. Connection pool size: 1
04/24/2024 09:57:00 AM [urllib3.connectionpool] [WARNING] Connection pool is full, discarding connection: 192.168.7.87. Connection pool size: 1
04/24/2024 09:59:00 AM [urllib3.connectionpool] [WARNING] Connection pool is full, discarding connection: 192.168.7.87. Connection pool size: 1
04/24/2024 10:01:00 AM [urllib3.connectionpool] [WARNING] Connection pool is full, discarding connection: 192.168.7.87. Connection pool size: 1
04/24/2024 10:01:39 AM [proxy] [INFO] pyPowerwall Proxy Stopped
 CANCEL 
04/24/2024 10:01:56 AM [proxy] [INFO] pyPowerwall [0.8.3] Proxy Server [t54] - HTTP Port 8675
04/24/2024 10:01:56 AM [proxy] [INFO] pyPowerwall Proxy Started
04/24/2024 10:01:56 AM [proxy] [INFO] pyPowerwall Proxy Server - Local Mode
04/24/2024 10:01:56 AM [proxy] [INFO] Connected to Energy Gateway 192.168.7.87 (Harrison)
04/24/2024 10:02:00 AM [urllib3.connectionpool] [WARNING] Connection pool is full, discarding connection: 192.168.7.87. Connection pool size: 1
04/24/2024 10:04:00 AM [urllib3.connectionpool] [WARNING] Connection pool is full, discarding connection: 192.168.7.87. Connection pool size: 1
04/24/2024 10:06:00 AM [urllib3.connectionpool] [WARNING] Connection pool is full, discarding connection: 192.168.7.87. Connection pool size: 1
04/24/2024 10:11:00 AM [urllib3.connectionpool] [WARNING] Connection pool is full, discarding connection: 192.168.7.87. Connection pool size: 1
04/24/2024 10:17:00 AM [urllib3.connectionpool] [WARNING] Connection pool is full, discarding connection: 192.168.7.87. Connection pool size: 1
mcbirse commented 2 months ago

Hi @KevM - setting PW_POOL_MAXSIZE=1 limits the connection pool size for HTTP persistent connections / re-use to 1. The thinking there "should only be one" connection is a misunderstanding of how this works...

It's due to the fact the pyPowerwall proxy is a multi-threaded HTTP server. For every API request sent by telegraf, a new connection could be opened to the gateway, and telegraf sends multiple requests simultaneously so you will end up with multiple connections due to the multi-threaded nature of the application.

Limiting the pool size to one just means the pool size won't be big enough to serve all the API requests, and you will see pool full warnings, connections discarded and re-opened to serve the request. Setting PW_POOL_MAXSIZE=0 will disable connection pooling and persistent connections, which means a new request to the gateway is opened/closed for every single API request from telegraf (slower, probably adds additional load to the gateway, and guaranteed to not see those warnings if it bothers you).

Persistent connections with the use of a connection pool was implemented to reduce load on the gateway. At least then generally, only e.g. 15x connections would be opened to the gateway. If there were more than 15 simultaneous API requests you will still see pool full warnings though.

There is background on this in issue #20 where I did an analysis of the connection use and benefit of the persistent connections / pool, implemented in PR #21

I'm using the default pool size of 15 and never see any warnings or have performance / drop out issues at all (note - direct ethernet connection to the gateway helps a lot too).

If you really only wanted a single connection from pyPowerwall being used for all API requests to the gateway, then we would need to implement a similar connection use setup that was done for the Cloud mode. Because pyPowerwall is multi-threaded, then this would involve adding locking around API requests to the proxy, so only a single thread would send requests to the gateway - refer https://github.com/jasonacox/pypowerwall/pull/59#issuecomment-1871712069

It's not a bad idea to implement this to be honest... especially if we start seeing issues with the gateway not responding well to having multiple HTTP sessions opened simultaneously (although, I have not seen issues with this so far). It was necessary with the Cloud mode setup due to the cloud limiting connections and often responding with HTTPError: 429 Client Error: Too Many Requests.

jasonacox commented 2 months ago

Excellent synopsis @mcbirse... 🙏

KevM commented 1 month ago

I've moved away from using PyPowerwall because of this issue. After using a raw connection to the gateway I do not run into any login reconnect issues.

That said I also started using the FleetAPI directly for my needs. The historic precision is limited to 5 minute intervals but this is good enough for my needs. 🤷🏼

jasonacox commented 1 month ago

Thanks for the follow up @KevM .