From 29b346deabd1d8cc20bf0a13026aea26221112bc Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 30 Jan 2023 15:43:03 +0100 Subject: [PATCH] container rm: save once for exec removal and state change Do not save the container each for changing the state and for removing running exec sessions. Saving the container is expensive and avoiding the redundant save makes `container rm` 1.2 times faster on my workstation. [NO NEW TESTS NEEDED] Signed-off-by: Valentin Rothberg --- libpod/container_exec.go | 8 -------- libpod/runtime_ctr.go | 24 ++++++++++++++---------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/libpod/container_exec.go b/libpod/container_exec.go index 7896d1932d..bda6588eb3 100644 --- a/libpod/container_exec.go +++ b/libpod/container_exec.go @@ -1072,14 +1072,6 @@ func (c *Container) removeAllExecSessions() error { } c.state.ExecSessions = nil c.state.LegacyExecSessions = nil - if err := c.save(); err != nil { - if !errors.Is(err, define.ErrCtrRemoved) { - if lastErr != nil { - logrus.Errorf("Stopping container %s exec sessions: %v", c.ID(), lastErr) - } - lastErr = err - } - } return lastErr } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 33e31c63e3..572cabd0ee 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -776,16 +776,6 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo cleanupErr = fmt.Errorf("cleaning up container %s: %w", c.ID(), err) } - // Set ContainerStateRemoving - c.state.State = define.ContainerStateRemoving - - if err := c.save(); err != nil { - if cleanupErr != nil { - logrus.Errorf(err.Error()) - } - return fmt.Errorf("unable to set container %s removing state in database: %w", c.ID(), err) - } - // Remove all active exec sessions // removing the exec sessions might temporarily unlock the container's lock. Using it // after setting the state to ContainerStateRemoving will prevent that the container is @@ -798,6 +788,20 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo } } + // Set ContainerStateRemoving as an intermediate state (we may get + // killed at any time) and save the container. + c.state.State = define.ContainerStateRemoving + + if err := c.save(); err != nil { + if !errors.Is(err, define.ErrCtrRemoved) { + if cleanupErr == nil { + cleanupErr = err + } else { + logrus.Errorf("Saving container: %v", err) + } + } + } + // Stop the container's storage if err := c.teardownStorage(); err != nil { if cleanupErr == nil {