jasonacox / pypowerwall

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

Memory leak in pypowerwall proxy #13

Closed jasonacox closed 2 years ago

jasonacox commented 2 years ago

From @youzer-name https://github.com/jasonacox/Powerwall-Dashboard/discussions/25#discussion-4052636

I use Grafana and InfluxDB to monitor my Pi4's vital stats. Recently I noticed that my memory used was steadily climbing. When I checked the Task Manager in the Pi4 GUI I found that Python3 was using a lot of memory in the "VM-Size" column. After rebooting the Pi the total memory use dropped down a lot.

I've been watching the memory usage since my last reboot about 6 days ago. While pypowerwall isn't the only thing using more memory, restarting my pypowerwall container immediately dropped the total memory usage by about 11%. The memory use by Python3 dropped from over 400MB to < 20MB. Based on the numbers I saw before the reboot, I expect that if I had let it go longer, the Python3 memory utilization would have just continued to climb.

Is this normal or could there be a memory leak in pypowerwall or Python3?

This image shows the last 7 days. Towards the left you can see the before/after reboot. At the very right edge you can see the restart of the pypowerwall docker container.

image

Originally posted by @youzer-name in https://github.com/jasonacox/Powerwall-Dashboard/discussions/25

jasonacox commented 2 years ago

It seems very compelling that there is a leak. It should grow as it buffers API calls but that should level off and not continue to grow as your graph shows. The pypowerwall proxy uses ThreadingHTTPServer. I did some quick research and believe there are conditions with python 3.7+ with ThreadingHTTPServer that can cause this memory leak. I have put in a recommended fix and I'm running some tests before releasing.

youzer-name commented 2 years ago

Pypowerwall version:

{"version": "22.9.1 bce98cad", "vint": 220901}

python --version = 2.7.16

python3 --version = 3.7.3

Per your reply in the Powerwall-Dashboard area, I'm trying out the t10 version. I'll check back on it later today, but it might take a few days to notice if there is anything going on with the memory use.

jasonacox commented 2 years ago

For others following, as @youzer-name mentions, If you want to test the patch - Edit powerwall.yml and change this line:

# from
    pypowerwall:
        image: jasonacox/pypowerwall:latest

# to
    pypowerwall:
        image: jasonacox/pypowerwall:0.4.0t10

Delete and pull down the t10 proxy:

docker stop pypowerwall

# delete pypowerwall
docker rm pypowerwall
docker images | grep pypowerwall | awk '{print $3}' | xargs docker rmi -f

# rebuild
docker-compose -f powerwall.yml up -d
jasonacox commented 2 years ago

Not 100% conclusive yet, but memory footprint is staying fairly stead with the patch (more than it was before).

image

Using a cron with this to push into InfluxDB/Grafana:

#!/bin/bash

# Push memory usage of pypowerwall to influxdb
PID=`ps aux | grep server.py | grep -v grep | awk '{print $2}'`
MEM=`sudo pmap ${PID} | tail -1 | awk '{print $2}' | cut -f1 -dK`

# Send
curl -i -XPOST 'http://localhost:8086/write?db=powerwall' --data-binary "pypowerwall mem=${MEM}"
youzer-name commented 2 years ago

Likewise. I've been running the new version for about 8 hours and it looks good.

I adjusted the Y axis on the chart below to make the trends more visible. The dip in the middle is a container restart using the old pypowerwall version. The smaller dip to the right on 5/5 at about 0800 is the installation of the new version. It stayed really flat until noon, jumped a little and then was really flat again after noon. One of my cron backup jobs runs at noon, so that may explain that increase. I also see something going on each night at midnight which is when my other cron backups run. It looks like there's a dip and then a jump in memory use around midnight each night.

image

jasonacox commented 2 years ago

Nice! Thanks @youzer-name. So far, stability seems to be good with the patch (no side effects) at least from my observations. Let me know if you see anything odd. If it hold good for another ~4 hours, I'll merge this change and push out t10 as the latest for new installs/upgrades for everyone.

youzer-name commented 2 years ago

I haven't seen any issues. I have alerts set up that notify me if the Powerwall data drops out for more than a minute and they haven't gone off, so the whole system seems to be stable. (I was originally connected via WiFi and had lots of dropouts so needed to keep a close eye on the data stream... I'm now using Ethernet but the alerts are still set up).

Here is the last 7 days memory use. The vertical line is when I installed the new version of pyPowerwall.

image

jasonacox commented 2 years ago

Nice! Thanks @youzer-name ! It's clear that the update made a difference. Thanks for reporting this and helping get it fixed!

My 24-hr graph showing the natural but small spikes and memory collections as different APIs are processed over time:

image

Like you, I had to switch from WiFi to hardwire ethernet. Even with strong WiFi signal reported by the Powerwall, it would see 10-20m dropouts all the time. Now it is solid.

Thanks again!

jasonacox commented 2 years ago

To others using https://github.com/jasonacox/Powerwall-Dashboard and finding this who need to apply the fix, you can use the upgrade.sh script or follow these manual steps:

Ensure your powerwall.yml has this (pulling latest):

    pypowerwall:
        image: jasonacox/pypowerwall:latest

Delete the old and pull the upgraded pypowerwall proxy container:

docker stop pypowerwall

# delete pypowerwall
docker rm pypowerwall
docker images | grep pypowerwall | awk '{print $3}' | xargs docker rmi -f

# rebuild
docker-compose -f powerwall.yml up -d
jasonacox commented 2 years ago

pyPowerwall Proxy now also provide a memory stat in the /stats call:

"mem": 37120

I'm seeing fairly consistent memory usage an no leaks. Closing this out for now, but will re-open if there are any reports of memory leak issues.