From cc955ae73c8aa234e0c41bdb65f111be79b32b90 Mon Sep 17 00:00:00 2001 From: Roman Strashkin Date: Fri, 19 Jun 2015 18:01:39 +0300 Subject: [PATCH] added ability to iterate over all indexes and use index.Iterate() instead of ReadDir() to walk over the graph Signed-off-by: Roman Strashkin --- daemon/daemon.go | 5 +--- daemon/image_delete.go | 5 +--- daemon/info.go | 2 +- graph/graph.go | 42 +++++++++++-------------------- graph/graph_test.go | 26 +++++++------------ graph/list.go | 7 ++---- graph/load.go | 5 +--- pkg/truncindex/truncindex.go | 10 ++++++++ pkg/truncindex/truncindex_test.go | 23 +++++++++++++++++ 9 files changed, 63 insertions(+), 62 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 4310b3528c..8f3b272389 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -884,10 +884,7 @@ func (daemon *Daemon) ContainerGraph() *graphdb.Database { func (daemon *Daemon) ImageGetCached(imgID string, config *runconfig.Config) (*graph.Image, error) { // Retrieve all images - images, err := daemon.Graph().Map() - if err != nil { - return nil, err - } + images := daemon.Graph().Map() // Store the tree in a map of map (map[parentId][childId]) imageMap := make(map[string]map[string]struct{}) diff --git a/daemon/image_delete.go b/daemon/image_delete.go index 05d013191c..5ee9248e1e 100644 --- a/daemon/image_delete.go +++ b/daemon/image_delete.go @@ -55,10 +55,7 @@ func (daemon *Daemon) imgDeleteHelper(name string, list *[]types.ImageDelete, fi tag = "" } - byParents, err := daemon.Graph().ByParent() - if err != nil { - return err - } + byParents := daemon.Graph().ByParent() repos := daemon.Repositories().ByID()[img.ID] diff --git a/daemon/info.go b/daemon/info.go index 4c73377b64..240ac20af8 100644 --- a/daemon/info.go +++ b/daemon/info.go @@ -17,7 +17,7 @@ import ( ) func (daemon *Daemon) SystemInfo() (*types.Info, error) { - images, _ := daemon.Graph().Map() + images := daemon.Graph().Map() var imgcount int if images == nil { imgcount = 0 diff --git a/graph/graph.go b/graph/graph.go index c77de3b963..fb66118537 100644 --- a/graph/graph.go +++ b/graph/graph.go @@ -327,42 +327,33 @@ func (graph *Graph) Delete(name string) error { } // Map returns a list of all images in the graph, addressable by ID. -func (graph *Graph) Map() (map[string]*Image, error) { +func (graph *Graph) Map() map[string]*Image { images := make(map[string]*Image) - err := graph.walkAll(func(image *Image) { + graph.walkAll(func(image *Image) { images[image.ID] = image }) - if err != nil { - return nil, err - } - return images, nil + return images } // walkAll iterates over each image in the graph, and passes it to a handler. // The walking order is undetermined. -func (graph *Graph) walkAll(handler func(*Image)) error { - files, err := ioutil.ReadDir(graph.root) - if err != nil { - return err - } - for _, st := range files { - if img, err := graph.Get(st.Name()); err != nil { - // Skip image - continue +func (graph *Graph) walkAll(handler func(*Image)) { + graph.idIndex.Iterate(func(id string) { + if img, err := graph.Get(id); err != nil { + return } else if handler != nil { handler(img) } - } - return nil + }) } // ByParent returns a lookup table of images by their parent. // If an image of id ID has 3 children images, then the value for key ID // will be a list of 3 images. // If an image has no children, it will not have an entry in the table. -func (graph *Graph) ByParent() (map[string][]*Image, error) { +func (graph *Graph) ByParent() map[string][]*Image { byParent := make(map[string][]*Image) - err := graph.walkAll(func(img *Image) { + graph.walkAll(func(img *Image) { parent, err := graph.Get(img.Parent) if err != nil { return @@ -373,25 +364,22 @@ func (graph *Graph) ByParent() (map[string][]*Image, error) { byParent[parent.ID] = []*Image{img} } }) - return byParent, err + return byParent } // Heads returns all heads in the graph, keyed by id. // A head is an image which is not the parent of another image in the graph. -func (graph *Graph) Heads() (map[string]*Image, error) { +func (graph *Graph) Heads() map[string]*Image { heads := make(map[string]*Image) - byParent, err := graph.ByParent() - if err != nil { - return nil, err - } - err = graph.walkAll(func(image *Image) { + byParent := graph.ByParent() + graph.walkAll(func(image *Image) { // If it's not in the byParent lookup table, then // it's not a parent -> so it's a head! if _, exists := byParent[image.ID]; !exists { heads[image.ID] = image } }) - return heads, err + return heads } func (graph *Graph) imageRoot(id string) string { diff --git a/graph/graph_test.go b/graph/graph_test.go index 946ac0652c..208c9e81cd 100644 --- a/graph/graph_test.go +++ b/graph/graph_test.go @@ -56,9 +56,8 @@ func TestInit(t *testing.T) { t.Fatal(err) } // Map() should be empty - if l, err := graph.Map(); err != nil { - t.Fatal(err) - } else if len(l) != 0 { + l := graph.Map() + if len(l) != 0 { t.Fatalf("len(Map()) should return %d, not %d", 0, len(l)) } } @@ -110,10 +109,8 @@ func TestGraphCreate(t *testing.T) { if img.DockerVersion != dockerversion.VERSION { t.Fatalf("Wrong docker_version: should be '%s', not '%s'", dockerversion.VERSION, img.DockerVersion) } - images, err := graph.Map() - if err != nil { - t.Fatal(err) - } else if l := len(images); l != 1 { + images := graph.Map() + if l := len(images); l != 1 { t.Fatalf("Wrong number of images. Should be %d, not %d", 1, l) } if images[img.ID] == nil { @@ -137,9 +134,8 @@ func TestRegister(t *testing.T) { if err != nil { t.Fatal(err) } - if images, err := graph.Map(); err != nil { - t.Fatal(err) - } else if l := len(images); l != 1 { + images := graph.Map() + if l := len(images); l != 1 { t.Fatalf("Wrong number of images. Should be %d, not %d", 1, l) } if resultImg, err := graph.Get(image.ID); err != nil { @@ -254,10 +250,7 @@ func TestByParent(t *testing.T) { _ = graph.Register(childImage1, archive2) _ = graph.Register(childImage2, archive3) - byParent, err := graph.ByParent() - if err != nil { - t.Fatal(err) - } + byParent := graph.ByParent() numChildren := len(byParent[parentImage.ID]) if numChildren != 2 { t.Fatalf("Expected 2 children, found %d", numChildren) @@ -277,9 +270,8 @@ func createTestImage(graph *Graph, t *testing.T) *Image { } func assertNImages(graph *Graph, t *testing.T, n int) { - if images, err := graph.Map(); err != nil { - t.Fatal(err) - } else if actualN := len(images); actualN != n { + images := graph.Map() + if actualN := len(images); actualN != n { t.Fatalf("Expected %d images, found %d", n, actualN) } } diff --git a/graph/list.go b/graph/list.go index 7c6027b4bf..46ff7bb8a6 100644 --- a/graph/list.go +++ b/graph/list.go @@ -58,12 +58,9 @@ func (s *TagStore) Images(config *ImagesConfig) ([]*types.Image, error) { _, filtLabel = imageFilters["label"] if config.All && filtTagged { - allImages, err = s.graph.Map() + allImages = s.graph.Map() } else { - allImages, err = s.graph.Heads() - } - if err != nil { - return nil, err + allImages = s.graph.Heads() } lookup := make(map[string]*types.Image) diff --git a/graph/load.go b/graph/load.go index 3a4a2746fc..5ca7b52f21 100644 --- a/graph/load.go +++ b/graph/load.go @@ -31,10 +31,7 @@ func (s *TagStore) Load(inTar io.ReadCloser, outStream io.Writer) error { if err := os.Mkdir(repoDir, os.ModeDir); err != nil { return err } - images, err := s.graph.Map() - if err != nil { - return err - } + images := s.graph.Map() excludes := make([]string, len(images)) i := 0 for k := range images { diff --git a/pkg/truncindex/truncindex.go b/pkg/truncindex/truncindex.go index 9aae5c0d08..8d8bee0c9b 100644 --- a/pkg/truncindex/truncindex.go +++ b/pkg/truncindex/truncindex.go @@ -108,3 +108,13 @@ func (idx *TruncIndex) Get(s string) (string, error) { } return "", fmt.Errorf("no such id: %s", s) } + +// Iterates over all stored IDs, and passes each of them to the given handler +func (idx *TruncIndex) Iterate(handler func(id string)) { + idx.RLock() + defer idx.RUnlock() + idx.trie.Visit(func(prefix patricia.Prefix, item patricia.Item) error { + handler(string(prefix)) + return nil + }) +} diff --git a/pkg/truncindex/truncindex_test.go b/pkg/truncindex/truncindex_test.go index 7253f6a4c4..cc7bc01d4c 100644 --- a/pkg/truncindex/truncindex_test.go +++ b/pkg/truncindex/truncindex_test.go @@ -96,6 +96,29 @@ func TestTruncIndex(t *testing.T) { assertIndexGet(t, index, id[:7], id, false) assertIndexGet(t, index, id[:15], id, false) assertIndexGet(t, index, id, id, false) + + assertIndexIterate(t) +} + +func assertIndexIterate(t *testing.T) { + ids := []string{ + "19b36c2c326ccc11e726eee6ee78a0baf166ef96", + "28b36c2c326ccc11e726eee6ee78a0baf166ef96", + "37b36c2c326ccc11e726eee6ee78a0baf166ef96", + "46b36c2c326ccc11e726eee6ee78a0baf166ef96", + } + + index := NewTruncIndex(ids) + + index.Iterate(func(targetId string) { + for _, id := range ids { + if targetId == id { + return + } + } + + t.Fatalf("An unknown ID '%s'", targetId) + }) } func assertIndexGet(t *testing.T, index *TruncIndex, input, expectedResult string, expectError bool) {