cc/ @thockin @vmarmol

LGTM, I assume that directory means something to influx

GCE e2e build/test passed for commit b6cf1134ed7e3b948e8207f5f76812edb423295c.

• Build Log
• Test Artifacts
• Internal Jenkins Results

LGTM

LGTM
@thockin I don't see this documented anywhere in the InfluxDB documentation, but /opt/influxdb/shared appears to be the default InfluxDB database storage directory. See link, for example.

I am fine with this change, but I want Dawn to confirm it is working properly and dumping state to this volume and reduces the memory footprint

Actually it looks like we use a custom InfluxDB config to change the storage dir to /data/
@dchen1107 is looking in to it

Ok, I think this pr is ready to have another review now. /opt/influxdb/shared is the default directory of influxdb for configuration and storage. But our heapster version influxdb's storage is moved to /data/db:

https://github.com/GoogleCloudPlatform/heapster/blob/master/influxdb/config.toml#L88

I manually tested it by restarting influxdb container on the node, and check monitoring-grafana page, and stats before killing are exist.

GCE e2e build/test passed for commit ff17623.

• Build Log
• Test Artifacts
• Internal Jenkins Results

LGTM

LGTM

Someone should slap the magic label on it so I remember to merge it in the morning. Or I could do it now.

done

On Tue, Jun 30, 2015 at 10:50 PM, Zach Loafman notifications@github.com
wrote:

Someone should slap the magic label on it so I remember to merge it in the
morning. Or I could do it now.

—
Reply to this email directly or view it on GitHub
#10528 (comment)
.

Ok, I still observed memory leakage of influxdb. After some research, it is known issue for influxdb 0.8 release which is what we are using here https://github.com/GoogleCloudPlatform/heapster/blob/master/influxdb/Dockerfile#L5.

The issue filed against influxdb and latest update can be found at: influxdata/influxdb#1228 (comment)

But this pr and upcoming one adding resource limit for addons can help with the situation even with memory leakage since most of stats are persistent to the local host, and restarting such docker container will pick up preexisting stats.

cc/ @thockin and @bgrant0607 for TL's lgtm. Thanks!

@thockin thanks! you are beating me :-)

LGTM

I just wanted to mention that this change probably led to another issue: #13508.