Fix gcplogs memory/connection leak #41513
Conversation
|
Actually, this doesn't compile; |
|
Ack. I'm figuring out the build/test process right now (first time working on this project). This method should exist according to the docs, though our vendored version may be behind. |
|
I recalled I had a PR that updates the dependencies to a more recent version (may have to do a follow-up after that, because it stalled some time, so is now quite behind on current latest); I just rebased it to see if the CI issues on that one are resolved; #39838 |
|
Should be working now (got my local environment setup and verified the fix) I had mixed up the internals (gcplogs creates a NewClient and then a Logger, discarding the former, while that's the object that requires closing) |
|
Thanks! Could you perhaps squash the two commits? |
The cloud logging client should be closed when the log driver is closed. Otherwise dockerd will keep a gRPC connection to the logging endpoint open indefinitely. This results in a slow leak of tcp sockets (1) and memory (~200Kb) any time that a container using `--log-driver=gcplogs` is terminates. Signed-off-by: Patrick Haas <patrickhaas@google.com>
|
Gladly. Squashed (and force pushed), hope everything looks ok still :) |
|
@cpuguy83 ptal |
The cloud logging client should be closed when the log driver is closed. Otherwise dockerd will keep a gRPC connection to the logging endpoint open indefinitely.
This results in a slow leak of tcp sockets (1) and memory (~200Kb) any time that a container using
--log-driver=gcplogsis terminates.fixes #41512
Testing
Tested locally in docker dev environment,
docker run --rm --log-driver=gcplogs hello-worldno longer grows the connection count of dockerd.- Description for the changelog Fix gcplogs memory/connection leak