From 0ce48eae9d1cd9d2554905c1ad3df84f3e06d50b Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Fri, 31 Mar 2023 16:42:17 -0700 Subject: [PATCH] `runsc metric-server`: Do not attempt to load sandboxes that ended abruptly. Prior to this CL, if a `runsc` sandbox was `kill -9`'d, its state file remains in `Running` state and the metric server would repeatedly attempt to load it. This wastes CPU and generates log spam. With this change, this case is now treated the same as a "the state file is not loadable" case, i.e. it will not attempt to reload this sandbox until its state file is updated on-disk. PiperOrigin-RevId: 521041222 --- runsc/cmd/metric_server.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/runsc/cmd/metric_server.go b/runsc/cmd/metric_server.go index 955a9cb1e..8a254b855 100644 --- a/runsc/cmd/metric_server.go +++ b/runsc/cmd/metric_server.go @@ -441,6 +441,7 @@ func (m *MetricServer) refreshSandboxesLocked() { log.Warningf("Cannot load state file for sandbox %q: %v", sid, err) continue } + // This is redundant with one of the checks performed below in servedSandbox.load(), but this // avoids log spam for the non-error case of sandboxes that didn't request instrumentation. sandboxMetricAddr := strings.ReplaceAll(cont.Sandbox.MetricServerAddress, "%RUNTIME_ROOT%", m.rootDir) @@ -448,6 +449,16 @@ func (m *MetricServer) refreshSandboxesLocked() { m.lastStateFileStat[sid] = stat continue } + + // This case can be hit when there is a leftover state file for a sandbox that was `kill -9`'d + // without an opportunity for it to clean up its state file. This results in a valid state file + // but the sandbox PID is gone. We don't want to continuously load this sandbox's state file. + if cont.Status == container.Running && !cont.Sandbox.IsRunning() { + log.Warningf("Sandbox %q has state file in state Running, yet it isn't actually running. Ignoring it.", sid) + m.lastStateFileStat[sid] = stat + continue + } + m.numSandboxes++ served := &servedSandbox{ rootContainerID: sid,