From d15385dab87b0142ff5b517dc9ff7223c86ee812 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 13 Mar 2018 11:58:05 -0400 Subject: [PATCH 1/2] Add `Len()` to image store for info endpoint In info, we only need the number of images, but `CountImages` was getting the whole map of images and then grabbing the length from that. This causes a lot of unnecessary CPU usage and memory allocations, which increases with O(n) on the number of images. Signed-off-by: Brian Goff Upstream-commit: f6a7763b6f3256bed9a7352021745189d0ca8dc9 Component: engine --- components/engine/daemon/images/service.go | 2 +- components/engine/image/store.go | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/components/engine/daemon/images/service.go b/components/engine/daemon/images/service.go index 70a8bf4455..4af48959bf 100644 --- a/components/engine/daemon/images/service.go +++ b/components/engine/daemon/images/service.go @@ -77,7 +77,7 @@ type ImageService struct { // CountImages returns the number of images stored by ImageService // called from info.go func (i *ImageService) CountImages() int { - return len(i.imageStore.Map()) + return i.imageStore.Len() } // Children returns the children image.IDs for a parent image. diff --git a/components/engine/image/store.go b/components/engine/image/store.go index a8f8cee5b2..9fd7d7dcf3 100644 --- a/components/engine/image/store.go +++ b/components/engine/image/store.go @@ -27,6 +27,7 @@ type Store interface { Children(id ID) []ID Map() map[ID]*Image Heads() map[ID]*Image + Len() int } // LayerGetReleaser is a minimal interface for getting and releasing images. @@ -336,3 +337,9 @@ func (is *store) imagesMap(all bool) map[ID]*Image { } return images } + +func (is *store) Len() int { + is.RLock() + defer is.RUnlock() + return len(is.images) +} From 2c6fe9c5245715ccbb0ee17388cb88b4995a9044 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 13 Mar 2018 12:21:56 -0400 Subject: [PATCH 2/2] Change containerd monitor ticker to sleep With the ticker this could end up just doing back-to-back checks, which isn't really what we want here. Instead use a sleep to ensure we actually sleep for the desired interval. Signed-off-by: Brian Goff Upstream-commit: 04a0d6b863ed50cfffa79936cf9cdab7a3a9e7df Component: engine --- components/engine/image/store_test.go | 15 +++++++++++++++ components/engine/libcontainerd/remote_daemon.go | 12 ++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/components/engine/image/store_test.go b/components/engine/image/store_test.go index ff40f7ea22..b1300657a0 100644 --- a/components/engine/image/store_test.go +++ b/components/engine/image/store_test.go @@ -1,6 +1,7 @@ package image // import "github.com/docker/docker/image" import ( + "fmt" "runtime" "testing" @@ -171,6 +172,20 @@ func TestGetAndSetLastUpdated(t *testing.T) { assert.Equal(t, updated.IsZero(), false) } +func TestStoreLen(t *testing.T) { + store, cleanup := defaultImageStore(t) + defer cleanup() + + expected := 10 + for i := 0; i < expected; i++ { + _, err := store.Create([]byte(fmt.Sprintf(`{"comment": "abc%d", "rootfs": {"type": "layers"}}`, i))) + assert.NoError(t, err) + } + numImages := store.Len() + assert.Equal(t, expected, numImages) + assert.Equal(t, len(store.Map()), numImages) +} + type mockLayerGetReleaser struct{} func (ls *mockLayerGetReleaser) Get(layer.ChainID) (layer.Layer, error) { diff --git a/components/engine/libcontainerd/remote_daemon.go b/components/engine/libcontainerd/remote_daemon.go index fb3e0bdda8..35ccc0e4a9 100644 --- a/components/engine/libcontainerd/remote_daemon.go +++ b/components/engine/libcontainerd/remote_daemon.go @@ -263,11 +263,15 @@ func (r *remote) startContainerd() error { func (r *remote) monitorConnection(monitor *containerd.Client) { var transientFailureCount = 0 - ticker := time.NewTicker(500 * time.Millisecond) - defer ticker.Stop() - for { - <-ticker.C + select { + case <-r.shutdownContext.Done(): + r.logger.Info("stopping healthcheck following graceful shutdown") + monitor.Close() + return + case <-time.After(500 * time.Millisecond): + } + ctx, cancel := context.WithTimeout(r.shutdownContext, healthCheckTimeout) _, err := monitor.IsServing(ctx) cancel()