From 012ab6c6e7412563abf0cde02593107bccb2aa72 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Sep 2023 05:21:34 +0000 Subject: [PATCH 01/13] feat: support deletion for memory graph Signed-off-by: Xiaoxuan Wang --- internal/container/set/set.go | 5 + internal/container/set/set_test.go | 8 + internal/graph/deletablememory.go | 144 +++++++++ internal/graph/deletablememory_test.go | 396 +++++++++++++++++++++++++ 4 files changed, 553 insertions(+) create mode 100644 internal/graph/deletablememory.go create mode 100644 internal/graph/deletablememory_test.go diff --git a/internal/container/set/set.go b/internal/container/set/set.go index a084e288..07c96d47 100644 --- a/internal/container/set/set.go +++ b/internal/container/set/set.go @@ -33,3 +33,8 @@ func (s Set[T]) Contains(item T) bool { _, ok := s[item] return ok } + +// Delete deletes an item from the set. +func (s Set[T]) Delete(item T) { + delete(s, item) +} diff --git a/internal/container/set/set_test.go b/internal/container/set/set_test.go index 94f87c7a..12dc6cea 100644 --- a/internal/container/set/set_test.go +++ b/internal/container/set/set_test.go @@ -52,4 +52,12 @@ func TestSet(t *testing.T) { if got, want := len(set), 2; got != want { t.Errorf("len(Set) = %v, want %v", got, want) } + // test deleting a key + set.Delete(key1) + if got, want := set.Contains(key1), false; got != want { + t.Errorf("Set.Contains(%s) = %v, want %v", key1, got, want) + } + if got, want := len(set), 1; got != want { + t.Errorf("len(Set) = %v, want %v", got, want) + } } diff --git a/internal/graph/deletablememory.go b/internal/graph/deletablememory.go new file mode 100644 index 00000000..54aa02a5 --- /dev/null +++ b/internal/graph/deletablememory.go @@ -0,0 +1,144 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package graph + +import ( + "context" + "errors" + "sync" + + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras-go/v2/content" + "oras.land/oras-go/v2/errdef" + "oras.land/oras-go/v2/internal/container/set" + "oras.land/oras-go/v2/internal/descriptor" + "oras.land/oras-go/v2/internal/status" + "oras.land/oras-go/v2/internal/syncutil" +) + +// DeletableMemory is a memory based PredecessorFinder. +type DeletableMemory struct { + nodes map[descriptor.Descriptor]ocispec.Descriptor // nodes saves the map keys of ocispec.Descriptor + predecessors map[descriptor.Descriptor]set.Set[descriptor.Descriptor] + successors map[descriptor.Descriptor]set.Set[descriptor.Descriptor] + lock sync.RWMutex +} + +// NewDeletableMemory creates a new DeletableMemory. +func NewDeletableMemory() *DeletableMemory { + return &DeletableMemory{ + nodes: make(map[descriptor.Descriptor]ocispec.Descriptor), + predecessors: make(map[descriptor.Descriptor]set.Set[descriptor.Descriptor]), + successors: make(map[descriptor.Descriptor]set.Set[descriptor.Descriptor]), + } +} + +// Index indexes predecessors for each direct successor of the given node. +func (m *DeletableMemory) Index(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) error { + _, err := m.index(ctx, fetcher, node) + return err +} + +// Index indexes predecessors for all the successors of the given node. +func (m *DeletableMemory) IndexAll(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) error { + // track content status + tracker := status.NewTracker() + var fn syncutil.GoFunc[ocispec.Descriptor] + fn = func(ctx context.Context, region *syncutil.LimitedRegion, desc ocispec.Descriptor) error { + // skip the node if other go routine is working on it + _, committed := tracker.TryCommit(desc) + if !committed { + return nil + } + successors, err := m.index(ctx, fetcher, desc) + if err != nil { + if errors.Is(err, errdef.ErrNotFound) { + // skip the node if it does not exist + return nil + } + return err + } + if len(successors) > 0 { + // traverse and index successors + return syncutil.Go(ctx, nil, fn, successors...) + } + return nil + } + return syncutil.Go(ctx, nil, fn, node) +} + +// Predecessors returns the nodes directly pointing to the current node. +// Predecessors returns nil without error if the node does not exists in the +// store. +// Like other operations, calling Predecessors() is go-routine safe. However, +// it does not necessarily correspond to any consistent snapshot of the stored +// contents. +func (m *DeletableMemory) Predecessors(_ context.Context, node ocispec.Descriptor) ([]ocispec.Descriptor, error) { + m.lock.RLock() + defer m.lock.RUnlock() + key := descriptor.FromOCI(node) + set, exists := m.predecessors[key] + if !exists { + return nil, nil + } + var res []ocispec.Descriptor + for k := range set { + res = append(res, m.nodes[k]) + } + return res, nil +} + +// Remove removes the node from its predecessors and successors. +func (m *DeletableMemory) Remove(ctx context.Context, node ocispec.Descriptor) error { + nodeKey := descriptor.FromOCI(node) + m.lock.Lock() + defer m.lock.Unlock() + // remove the node from its successors' predecessor list + for successorKey := range m.successors[nodeKey] { + m.predecessors[successorKey].Delete(nodeKey) + } + delete(m.successors, nodeKey) + return nil +} + +// index indexes predecessors for each direct successor of the given node. +func (m *DeletableMemory) index(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) ([]ocispec.Descriptor, error) { + successors, err := content.Successors(ctx, fetcher, node) + if err != nil { + return nil, err + } + m.lock.Lock() + defer m.lock.Unlock() + + // index the node + nodeKey := descriptor.FromOCI(node) + m.nodes[nodeKey] = node + + // index the successors and predecessors + successorSet := set.New[descriptor.Descriptor]() + m.successors[nodeKey] = successorSet + for _, successor := range successors { + successorKey := descriptor.FromOCI(successor) + successorSet.Add(successorKey) + predecessorSet, exists := m.predecessors[successorKey] + if !exists { + predecessorSet = set.New[descriptor.Descriptor]() + m.predecessors[successorKey] = predecessorSet + } + predecessorSet.Add(nodeKey) + } + return successors, nil +} diff --git a/internal/graph/deletablememory_test.go b/internal/graph/deletablememory_test.go new file mode 100644 index 00000000..af14319c --- /dev/null +++ b/internal/graph/deletablememory_test.go @@ -0,0 +1,396 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package graph + +import ( + "bytes" + "context" + "encoding/json" + "io" + "reflect" + "testing" + + "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras-go/v2/internal/cas" + "oras.land/oras-go/v2/internal/descriptor" +) + +/* +A(manifest)-------------------+ + + | | + | | + | | + v | + +B(manifest)-----------------+ | + + | | | + | | | + v v v + +C(layer) D(layer) +*/ +func TestDeletableMemory_IndexAndRemove(t *testing.T) { + testFetcher := cas.NewMemory() + testDeletableMemory := NewDeletableMemory() + ctx := context.Background() + + // generate test content + var blobs [][]byte + var descs []ocispec.Descriptor + appendBlob := func(mediaType string, blob []byte) ocispec.Descriptor { + blobs = append(blobs, blob) + descs = append(descs, ocispec.Descriptor{ + MediaType: mediaType, + Digest: digest.FromBytes(blob), + Size: int64(len(blob)), + }) + return descs[len(descs)-1] + } + generateManifest := func(layers ...ocispec.Descriptor) ocispec.Descriptor { + manifest := ocispec.Manifest{ + Layers: layers, + } + manifestJSON, err := json.Marshal(manifest) + if err != nil { + t.Fatal(err) + } + return appendBlob(ocispec.MediaTypeImageManifest, manifestJSON) + } + descC := appendBlob("layer node C", []byte("Node C is a layer")) // blobs[0], layer "C" + descD := appendBlob("layer node D", []byte("Node D is a layer")) // blobs[1], layer "D" + descB := generateManifest(descs[0:2]...) // blobs[2], manifest "B" + descA := generateManifest(descs[1:3]...) // blobs[3], manifest "A" + testFetcher.Push(ctx, descA, bytes.NewReader(blobs[3])) + testFetcher.Push(ctx, descB, bytes.NewReader(blobs[2])) + testFetcher.Push(ctx, descC, bytes.NewReader(blobs[0])) + testFetcher.Push(ctx, descD, bytes.NewReader(blobs[1])) + + // make sure that testFetcher works + rc, err := testFetcher.Fetch(ctx, descA) + if err != nil { + t.Errorf("testFetcher.Fetch() error = %v", err) + } + got, err := io.ReadAll(rc) + if err != nil { + t.Errorf("testFetcher.Fetch().Read() error = %v", err) + } + err = rc.Close() + if err != nil { + t.Errorf("testFetcher.Fetch().Close() error = %v", err) + } + if !bytes.Equal(got, blobs[3]) { + t.Errorf("testFetcher.Fetch() = %v, want %v", got, blobs[4]) + } + + // index test content into graph.DeletableMemory + testDeletableMemory.Index(ctx, testFetcher, descA) + testDeletableMemory.Index(ctx, testFetcher, descB) + testDeletableMemory.Index(ctx, testFetcher, descC) + testDeletableMemory.Index(ctx, testFetcher, descD) + + // check that the content of testDeletableMemory.predecessors and successors + // are correct + nodeKeyA := descriptor.FromOCI(descA) + nodeKeyB := descriptor.FromOCI(descB) + nodeKeyC := descriptor.FromOCI(descC) + nodeKeyD := descriptor.FromOCI(descD) + + // check the information of node A + predecessorsA := testDeletableMemory.predecessors[nodeKeyA] + successorsA := testDeletableMemory.successors[nodeKeyA] + for range predecessorsA { + t.Errorf("predecessors of %s should be empty", "A") + } + if !successorsA.Contains(nodeKeyB) { + t.Errorf("successors of %s should contain %s", "A", "B") + } + if !successorsA.Contains(nodeKeyD) { + t.Errorf("successors of %s should contain %s", "A", "D") + } + + // check the information of node B + predecessorsB := testDeletableMemory.predecessors[nodeKeyB] + successorsB := testDeletableMemory.successors[nodeKeyB] + if !predecessorsB.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should contain %s", "B", "A") + } + if !successorsB.Contains(nodeKeyC) { + t.Errorf("successors of %s should contain %s", "B", "C") + } + if !successorsB.Contains(nodeKeyD) { + t.Errorf("successors of %s should contain %s", "B", "D") + } + + // check the information of node C + predecessorsC := testDeletableMemory.predecessors[nodeKeyC] + successorsC := testDeletableMemory.successors[nodeKeyC] + if !predecessorsC.Contains(nodeKeyB) { + t.Errorf("predecessors of %s should contain %s", "C", "B") + } + for range successorsC { + t.Errorf("successors of %s should be empty", "C") + } + + // check the information of node D + predecessorsD := testDeletableMemory.predecessors[nodeKeyD] + successorsD := testDeletableMemory.successors[nodeKeyD] + if !predecessorsD.Contains(nodeKeyB) { + t.Errorf("predecessors of %s should contain %s", "D", "B") + } + if !predecessorsD.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should contain %s", "D", "A") + } + for range successorsD { + t.Errorf("successors of %s should be empty", "C") + } + + // remove node B and check the stored information + testDeletableMemory.Remove(ctx, descB) + if predecessorsC.Contains(nodeKeyB) { + t.Errorf("predecessors of %s should not contain %s", "C", "B") + } + if predecessorsD.Contains(nodeKeyB) { + t.Errorf("predecessors of %s should not contain %s", "D", "B") + } + if !successorsA.Contains(nodeKeyB) { + t.Errorf("successors of %s should still contain %s", "A", "B") + } + if _, exists := testDeletableMemory.successors[nodeKeyB]; exists { + t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "B") + } + + // remove node A and check the stored information + testDeletableMemory.Remove(ctx, descA) + if predecessorsD.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should not contain %s", "D", "A") + } + if _, exists := testDeletableMemory.successors[nodeKeyA]; exists { + t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "A") + } +} + +func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { + testFetcher := cas.NewMemory() + testDeletableMemory := NewDeletableMemory() + ctx := context.Background() + + // generate test content + var blobs [][]byte + var descriptors []ocispec.Descriptor + appendBlob := func(mediaType string, blob []byte) ocispec.Descriptor { + blobs = append(blobs, blob) + descriptors = append(descriptors, ocispec.Descriptor{ + MediaType: mediaType, + Digest: digest.FromBytes(blob), + Size: int64(len(blob)), + }) + return descriptors[len(descriptors)-1] + } + generateManifest := func(layers ...ocispec.Descriptor) ocispec.Descriptor { + manifest := ocispec.Manifest{ + Layers: layers, + } + manifestJSON, err := json.Marshal(manifest) + if err != nil { + t.Fatal(err) + } + return appendBlob(ocispec.MediaTypeImageManifest, manifestJSON) + } + generateIndex := func(manifests ...ocispec.Descriptor) ocispec.Descriptor { + index := ocispec.Index{ + Manifests: manifests, + } + indexJSON, err := json.Marshal(index) + if err != nil { + t.Fatal(err) + } + return appendBlob(ocispec.MediaTypeImageIndex, indexJSON) + } + descE := appendBlob("layer node E", []byte("Node E is a layer")) // blobs[0], layer "E" + descF := appendBlob("layer node F", []byte("Node F is a layer")) // blobs[1], layer "F" + descB := generateManifest(descriptors[0:1]...) // blobs[2], manifest "B" + descC := generateManifest(descriptors[0:2]...) // blobs[3], manifest "C" + descD := generateManifest(descriptors[1:2]...) // blobs[4], manifest "D" + descA := generateIndex(descriptors[2:5]...) // blobs[5], index "A" + testFetcher.Push(ctx, descA, bytes.NewReader(blobs[5])) + testFetcher.Push(ctx, descB, bytes.NewReader(blobs[2])) + testFetcher.Push(ctx, descC, bytes.NewReader(blobs[3])) + testFetcher.Push(ctx, descD, bytes.NewReader(blobs[4])) + testFetcher.Push(ctx, descE, bytes.NewReader(blobs[0])) + testFetcher.Push(ctx, descF, bytes.NewReader(blobs[1])) + + // make sure that testFetcher works + rc, err := testFetcher.Fetch(ctx, descA) + if err != nil { + t.Errorf("testFetcher.Fetch() error = %v", err) + } + got, err := io.ReadAll(rc) + if err != nil { + t.Errorf("testFetcher.Fetch().Read() error = %v", err) + } + err = rc.Close() + if err != nil { + t.Errorf("testFetcher.Fetch().Close() error = %v", err) + } + if !bytes.Equal(got, blobs[5]) { + t.Errorf("testFetcher.Fetch() = %v, want %v", got, blobs[4]) + } + + // index node A into graph.DeletableMemory using IndexAll + testDeletableMemory.IndexAll(ctx, testFetcher, descA) + + // check that the content of testDeletableMemory.predecessors and successors + // are correct + nodeKeyA := descriptor.FromOCI(descA) + nodeKeyB := descriptor.FromOCI(descB) + nodeKeyC := descriptor.FromOCI(descC) + nodeKeyD := descriptor.FromOCI(descD) + nodeKeyE := descriptor.FromOCI(descE) + nodeKeyF := descriptor.FromOCI(descF) + + // check the information of node A + predecessorsA := testDeletableMemory.predecessors[nodeKeyA] + successorsA := testDeletableMemory.successors[nodeKeyA] + for range predecessorsA { + t.Errorf("predecessors of %s should be empty", "A") + } + if !successorsA.Contains(nodeKeyB) { + t.Errorf("successors of %s should contain %s", "A", "B") + } + if !successorsA.Contains(nodeKeyC) { + t.Errorf("successors of %s should contain %s", "A", "C") + } + if !successorsA.Contains(nodeKeyD) { + t.Errorf("successors of %s should contain %s", "A", "D") + } + + // check the information of node B + predecessorsB := testDeletableMemory.predecessors[nodeKeyB] + successorsB := testDeletableMemory.successors[nodeKeyB] + if !predecessorsB.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should contain %s", "B", "A") + } + if !successorsB.Contains(nodeKeyE) { + t.Errorf("successors of %s should contain %s", "B", "E") + } + + // check the information of node C + predecessorsC := testDeletableMemory.predecessors[nodeKeyC] + successorsC := testDeletableMemory.successors[nodeKeyC] + if !predecessorsC.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should contain %s", "C", "A") + } + if !successorsC.Contains(nodeKeyE) { + t.Errorf("successors of %s should contain %s", "C", "E") + } + if !successorsC.Contains(nodeKeyF) { + t.Errorf("successors of %s should contain %s", "C", "F") + } + + // check the information of node D + predecessorsD := testDeletableMemory.predecessors[nodeKeyD] + successorsD := testDeletableMemory.successors[nodeKeyD] + if !predecessorsD.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should contain %s", "D", "A") + } + if !successorsD.Contains(nodeKeyF) { + t.Errorf("successors of %s should contain %s", "D", "F") + } + + // check the information of node E + predecessorsE := testDeletableMemory.predecessors[nodeKeyE] + successorsE := testDeletableMemory.successors[nodeKeyE] + if !predecessorsE.Contains(nodeKeyB) { + t.Errorf("predecessors of %s should contain %s", "E", "B") + } + if !predecessorsE.Contains(nodeKeyC) { + t.Errorf("predecessors of %s should contain %s", "E", "C") + } + for range successorsE { + t.Errorf("successors of %s should be empty", "E") + } + + // check the information of node F + predecessorsF := testDeletableMemory.predecessors[nodeKeyF] + successorsF := testDeletableMemory.successors[nodeKeyF] + if !predecessorsF.Contains(nodeKeyC) { + t.Errorf("predecessors of %s should contain %s", "F", "C") + } + if !predecessorsF.Contains(nodeKeyD) { + t.Errorf("predecessors of %s should contain %s", "F", "D") + } + for range successorsF { + t.Errorf("successors of %s should be empty", "E") + } + + // check the Predecessors of node C is node A + predsC, err := testDeletableMemory.Predecessors(ctx, descC) + if err != nil { + t.Errorf("testFetcher.Predecessors() error = %v", err) + } + expectedLength := 1 + if len(predsC) != expectedLength { + t.Errorf("%s should have length %d", "predsC", expectedLength) + } + if !reflect.DeepEqual(predsC[0], descA) { + t.Errorf("incorrect predecessor result") + } + + // check the Predecessors of node F are node C and node D + predsF, err := testDeletableMemory.Predecessors(ctx, descF) + if err != nil { + t.Errorf("testFetcher.Predecessors() error = %v", err) + } + expectedLength = 2 + if len(predsF) != expectedLength { + t.Errorf("%s should have length %d", "predsF", expectedLength) + } + for _, pred := range predsF { + if !reflect.DeepEqual(pred, descC) && !reflect.DeepEqual(pred, descD) { + t.Errorf("incorrect predecessor result") + } + } + + // remove node C and check the stored information + testDeletableMemory.Remove(ctx, descC) + if predecessorsE.Contains(nodeKeyC) { + t.Errorf("predecessors of %s should not contain %s", "E", "C") + } + if predecessorsF.Contains(nodeKeyC) { + t.Errorf("predecessors of %s should not contain %s", "F", "C") + } + if !successorsA.Contains(nodeKeyC) { + t.Errorf("successors of %s should still contain %s", "A", "C") + } + if _, exists := testDeletableMemory.successors[nodeKeyC]; exists { + t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "C") + } + + // remove node A and check the stored information + testDeletableMemory.Remove(ctx, descA) + if predecessorsD.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should not contain %s", "D", "A") + } + if predecessorsB.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should not contain %s", "B", "A") + } + if _, exists := testDeletableMemory.successors[nodeKeyA]; exists { + t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "A") + } +} From e1c667f61cf20b106a54f85917d0c6d26ec6f7d4 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Sep 2023 07:05:21 +0000 Subject: [PATCH 02/13] added benchmark tests Signed-off-by: Xiaoxuan Wang --- internal/graph/benchmark_test.go | 146 +++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 internal/graph/benchmark_test.go diff --git a/internal/graph/benchmark_test.go b/internal/graph/benchmark_test.go new file mode 100644 index 00000000..c6d41e5f --- /dev/null +++ b/internal/graph/benchmark_test.go @@ -0,0 +1,146 @@ +/* +Copyright The ORAS Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package graph + +import ( + "bytes" + "context" + "encoding/json" + "testing" + + "github.com/opencontainers/go-digest" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "oras.land/oras-go/v2/content" + "oras.land/oras-go/v2/internal/cas" +) + +func testSetUp(ctx context.Context) ([]ocispec.Descriptor, content.Fetcher) { + // set up for the test, prepare the test content and + // return the Fetcher + testFetcher := cas.NewMemory() + + var blobs [][]byte + var descriptors []ocispec.Descriptor + appendBlob := func(mediaType string, blob []byte) ocispec.Descriptor { + blobs = append(blobs, blob) + descriptors = append(descriptors, ocispec.Descriptor{ + MediaType: mediaType, + Digest: digest.FromBytes(blob), + Size: int64(len(blob)), + }) + return descriptors[len(descriptors)-1] + } + generateManifest := func(layers ...ocispec.Descriptor) ocispec.Descriptor { + manifest := ocispec.Manifest{ + Layers: layers, + } + manifestJSON, err := json.Marshal(manifest) + if err != nil { + panic(err) + } + return appendBlob(ocispec.MediaTypeImageManifest, manifestJSON) + } + generateIndex := func(manifests ...ocispec.Descriptor) ocispec.Descriptor { + index := ocispec.Index{ + Manifests: manifests, + } + indexJSON, err := json.Marshal(index) + if err != nil { + panic(err) + } + return appendBlob(ocispec.MediaTypeImageIndex, indexJSON) + } + descE := appendBlob("layer node E", []byte("Node E is a layer")) // blobs[0], layer "E" + descF := appendBlob("layer node F", []byte("Node F is a layer")) // blobs[1], layer "F" + descB := generateManifest(descriptors[0:1]...) // blobs[2], manifest "B" + descC := generateManifest(descriptors[0:2]...) // blobs[3], manifest "C" + descD := generateManifest(descriptors[1:2]...) // blobs[4], manifest "D" + descA := generateIndex(descriptors[2:5]...) // blobs[5], index "A" + testFetcher.Push(ctx, descA, bytes.NewReader(blobs[5])) + testFetcher.Push(ctx, descB, bytes.NewReader(blobs[2])) + testFetcher.Push(ctx, descC, bytes.NewReader(blobs[3])) + testFetcher.Push(ctx, descD, bytes.NewReader(blobs[4])) + testFetcher.Push(ctx, descE, bytes.NewReader(blobs[0])) + testFetcher.Push(ctx, descF, bytes.NewReader(blobs[1])) + + return []ocispec.Descriptor{descA, descB, descC, descD, descE, descF}, testFetcher +} + +func BenchmarkMemoryIndex(b *testing.B) { + ctx := context.Background() + descs, testFetcher := testSetUp(ctx) + b.ResetTimer() + for i := 0; i < b.N; i++ { + testMemory := NewMemory() + for _, desc := range descs { + testMemory.Index(ctx, testFetcher, desc) + } + } +} + +func BenchmarkDeletableMemoryIndex(b *testing.B) { + ctx := context.Background() + descs, testFetcher := testSetUp(ctx) + b.ResetTimer() + for i := 0; i < b.N; i++ { + testDeletableMemory := NewDeletableMemory() + for _, desc := range descs { + testDeletableMemory.Index(ctx, testFetcher, desc) + } + } +} + +func BenchmarkMemoryIndexAll(b *testing.B) { + ctx := context.Background() + descs, testFetcher := testSetUp(ctx) + b.ResetTimer() + for i := 0; i < b.N; i++ { + testMemory := NewMemory() + testMemory.IndexAll(ctx, testFetcher, descs[0]) + } +} + +func BenchmarkDeletableMemoryIndexAll(b *testing.B) { + ctx := context.Background() + descs, testFetcher := testSetUp(ctx) + b.ResetTimer() + for i := 0; i < b.N; i++ { + testDeletableMemory := NewDeletableMemory() + testDeletableMemory.IndexAll(ctx, testFetcher, descs[0]) + } +} + +func BenchmarkMemoryPredecessors(b *testing.B) { + ctx := context.Background() + descs, testFetcher := testSetUp(ctx) + testMemory := NewMemory() + testMemory.IndexAll(ctx, testFetcher, descs[0]) + b.ResetTimer() + for i := 0; i < b.N; i++ { + testMemory.Predecessors(ctx, descs[4]) + } +} + +func BenchmarkDeletableMemoryPredecessors(b *testing.B) { + ctx := context.Background() + descs, testFetcher := testSetUp(ctx) + testDeletableMemory := NewDeletableMemory() + testDeletableMemory.IndexAll(ctx, testFetcher, descs[0]) + b.ResetTimer() + for i := 0; i < b.N; i++ { + testDeletableMemory.Predecessors(ctx, descs[4]) + } +} From f98443e7dcbe3a6964be6215e8a968aea11f0549 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 25 Sep 2023 07:09:58 +0000 Subject: [PATCH 03/13] added an ASCII drawing Signed-off-by: Xiaoxuan Wang --- internal/graph/deletablememory_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/graph/deletablememory_test.go b/internal/graph/deletablememory_test.go index af14319c..5587174d 100644 --- a/internal/graph/deletablememory_test.go +++ b/internal/graph/deletablememory_test.go @@ -185,6 +185,22 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { } } +/* ++---------A(index)--------+ +| | | +| | | +| | | +| | | +v v v +B(manifest) C(manifest) D(manifest) +| | | +| | | +| | | +| | | +| | | +| | v ++-->E(layer)<-+------->F(layer) +*/ func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { testFetcher := cas.NewMemory() testDeletableMemory := NewDeletableMemory() From 6f5404797f04a4a8055756de255f700a7394ac0d Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Tue, 26 Sep 2023 19:19:08 +0000 Subject: [PATCH 04/13] resolved comments Signed-off-by: Xiaoxuan Wang --- internal/graph/deletablememory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/graph/deletablememory.go b/internal/graph/deletablememory.go index 54aa02a5..e9662d52 100644 --- a/internal/graph/deletablememory.go +++ b/internal/graph/deletablememory.go @@ -103,9 +103,9 @@ func (m *DeletableMemory) Predecessors(_ context.Context, node ocispec.Descripto // Remove removes the node from its predecessors and successors. func (m *DeletableMemory) Remove(ctx context.Context, node ocispec.Descriptor) error { - nodeKey := descriptor.FromOCI(node) m.lock.Lock() defer m.lock.Unlock() + nodeKey := descriptor.FromOCI(node) // remove the node from its successors' predecessor list for successorKey := range m.successors[nodeKey] { m.predecessors[successorKey].Delete(nodeKey) From 08fde1490ced63996b2539150df0a9eea083d0bb Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Sun, 8 Oct 2023 07:39:44 +0000 Subject: [PATCH 05/13] merged deletable memory into memory Signed-off-by: Xiaoxuan Wang --- internal/graph/benchmark_test.go | 146 ------------------ internal/graph/deletablememory.go | 144 ----------------- internal/graph/memory.go | 96 ++++++------ ...deletablememory_test.go => memory_test.go} | 4 +- 4 files changed, 55 insertions(+), 335 deletions(-) delete mode 100644 internal/graph/benchmark_test.go delete mode 100644 internal/graph/deletablememory.go rename internal/graph/{deletablememory_test.go => memory_test.go} (99%) diff --git a/internal/graph/benchmark_test.go b/internal/graph/benchmark_test.go deleted file mode 100644 index c6d41e5f..00000000 --- a/internal/graph/benchmark_test.go +++ /dev/null @@ -1,146 +0,0 @@ -/* -Copyright The ORAS Authors. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - -http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package graph - -import ( - "bytes" - "context" - "encoding/json" - "testing" - - "github.com/opencontainers/go-digest" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "oras.land/oras-go/v2/content" - "oras.land/oras-go/v2/internal/cas" -) - -func testSetUp(ctx context.Context) ([]ocispec.Descriptor, content.Fetcher) { - // set up for the test, prepare the test content and - // return the Fetcher - testFetcher := cas.NewMemory() - - var blobs [][]byte - var descriptors []ocispec.Descriptor - appendBlob := func(mediaType string, blob []byte) ocispec.Descriptor { - blobs = append(blobs, blob) - descriptors = append(descriptors, ocispec.Descriptor{ - MediaType: mediaType, - Digest: digest.FromBytes(blob), - Size: int64(len(blob)), - }) - return descriptors[len(descriptors)-1] - } - generateManifest := func(layers ...ocispec.Descriptor) ocispec.Descriptor { - manifest := ocispec.Manifest{ - Layers: layers, - } - manifestJSON, err := json.Marshal(manifest) - if err != nil { - panic(err) - } - return appendBlob(ocispec.MediaTypeImageManifest, manifestJSON) - } - generateIndex := func(manifests ...ocispec.Descriptor) ocispec.Descriptor { - index := ocispec.Index{ - Manifests: manifests, - } - indexJSON, err := json.Marshal(index) - if err != nil { - panic(err) - } - return appendBlob(ocispec.MediaTypeImageIndex, indexJSON) - } - descE := appendBlob("layer node E", []byte("Node E is a layer")) // blobs[0], layer "E" - descF := appendBlob("layer node F", []byte("Node F is a layer")) // blobs[1], layer "F" - descB := generateManifest(descriptors[0:1]...) // blobs[2], manifest "B" - descC := generateManifest(descriptors[0:2]...) // blobs[3], manifest "C" - descD := generateManifest(descriptors[1:2]...) // blobs[4], manifest "D" - descA := generateIndex(descriptors[2:5]...) // blobs[5], index "A" - testFetcher.Push(ctx, descA, bytes.NewReader(blobs[5])) - testFetcher.Push(ctx, descB, bytes.NewReader(blobs[2])) - testFetcher.Push(ctx, descC, bytes.NewReader(blobs[3])) - testFetcher.Push(ctx, descD, bytes.NewReader(blobs[4])) - testFetcher.Push(ctx, descE, bytes.NewReader(blobs[0])) - testFetcher.Push(ctx, descF, bytes.NewReader(blobs[1])) - - return []ocispec.Descriptor{descA, descB, descC, descD, descE, descF}, testFetcher -} - -func BenchmarkMemoryIndex(b *testing.B) { - ctx := context.Background() - descs, testFetcher := testSetUp(ctx) - b.ResetTimer() - for i := 0; i < b.N; i++ { - testMemory := NewMemory() - for _, desc := range descs { - testMemory.Index(ctx, testFetcher, desc) - } - } -} - -func BenchmarkDeletableMemoryIndex(b *testing.B) { - ctx := context.Background() - descs, testFetcher := testSetUp(ctx) - b.ResetTimer() - for i := 0; i < b.N; i++ { - testDeletableMemory := NewDeletableMemory() - for _, desc := range descs { - testDeletableMemory.Index(ctx, testFetcher, desc) - } - } -} - -func BenchmarkMemoryIndexAll(b *testing.B) { - ctx := context.Background() - descs, testFetcher := testSetUp(ctx) - b.ResetTimer() - for i := 0; i < b.N; i++ { - testMemory := NewMemory() - testMemory.IndexAll(ctx, testFetcher, descs[0]) - } -} - -func BenchmarkDeletableMemoryIndexAll(b *testing.B) { - ctx := context.Background() - descs, testFetcher := testSetUp(ctx) - b.ResetTimer() - for i := 0; i < b.N; i++ { - testDeletableMemory := NewDeletableMemory() - testDeletableMemory.IndexAll(ctx, testFetcher, descs[0]) - } -} - -func BenchmarkMemoryPredecessors(b *testing.B) { - ctx := context.Background() - descs, testFetcher := testSetUp(ctx) - testMemory := NewMemory() - testMemory.IndexAll(ctx, testFetcher, descs[0]) - b.ResetTimer() - for i := 0; i < b.N; i++ { - testMemory.Predecessors(ctx, descs[4]) - } -} - -func BenchmarkDeletableMemoryPredecessors(b *testing.B) { - ctx := context.Background() - descs, testFetcher := testSetUp(ctx) - testDeletableMemory := NewDeletableMemory() - testDeletableMemory.IndexAll(ctx, testFetcher, descs[0]) - b.ResetTimer() - for i := 0; i < b.N; i++ { - testDeletableMemory.Predecessors(ctx, descs[4]) - } -} diff --git a/internal/graph/deletablememory.go b/internal/graph/deletablememory.go deleted file mode 100644 index e9662d52..00000000 --- a/internal/graph/deletablememory.go +++ /dev/null @@ -1,144 +0,0 @@ -/* -Copyright The ORAS Authors. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - -http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package graph - -import ( - "context" - "errors" - "sync" - - ocispec "github.com/opencontainers/image-spec/specs-go/v1" - "oras.land/oras-go/v2/content" - "oras.land/oras-go/v2/errdef" - "oras.land/oras-go/v2/internal/container/set" - "oras.land/oras-go/v2/internal/descriptor" - "oras.land/oras-go/v2/internal/status" - "oras.land/oras-go/v2/internal/syncutil" -) - -// DeletableMemory is a memory based PredecessorFinder. -type DeletableMemory struct { - nodes map[descriptor.Descriptor]ocispec.Descriptor // nodes saves the map keys of ocispec.Descriptor - predecessors map[descriptor.Descriptor]set.Set[descriptor.Descriptor] - successors map[descriptor.Descriptor]set.Set[descriptor.Descriptor] - lock sync.RWMutex -} - -// NewDeletableMemory creates a new DeletableMemory. -func NewDeletableMemory() *DeletableMemory { - return &DeletableMemory{ - nodes: make(map[descriptor.Descriptor]ocispec.Descriptor), - predecessors: make(map[descriptor.Descriptor]set.Set[descriptor.Descriptor]), - successors: make(map[descriptor.Descriptor]set.Set[descriptor.Descriptor]), - } -} - -// Index indexes predecessors for each direct successor of the given node. -func (m *DeletableMemory) Index(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) error { - _, err := m.index(ctx, fetcher, node) - return err -} - -// Index indexes predecessors for all the successors of the given node. -func (m *DeletableMemory) IndexAll(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) error { - // track content status - tracker := status.NewTracker() - var fn syncutil.GoFunc[ocispec.Descriptor] - fn = func(ctx context.Context, region *syncutil.LimitedRegion, desc ocispec.Descriptor) error { - // skip the node if other go routine is working on it - _, committed := tracker.TryCommit(desc) - if !committed { - return nil - } - successors, err := m.index(ctx, fetcher, desc) - if err != nil { - if errors.Is(err, errdef.ErrNotFound) { - // skip the node if it does not exist - return nil - } - return err - } - if len(successors) > 0 { - // traverse and index successors - return syncutil.Go(ctx, nil, fn, successors...) - } - return nil - } - return syncutil.Go(ctx, nil, fn, node) -} - -// Predecessors returns the nodes directly pointing to the current node. -// Predecessors returns nil without error if the node does not exists in the -// store. -// Like other operations, calling Predecessors() is go-routine safe. However, -// it does not necessarily correspond to any consistent snapshot of the stored -// contents. -func (m *DeletableMemory) Predecessors(_ context.Context, node ocispec.Descriptor) ([]ocispec.Descriptor, error) { - m.lock.RLock() - defer m.lock.RUnlock() - key := descriptor.FromOCI(node) - set, exists := m.predecessors[key] - if !exists { - return nil, nil - } - var res []ocispec.Descriptor - for k := range set { - res = append(res, m.nodes[k]) - } - return res, nil -} - -// Remove removes the node from its predecessors and successors. -func (m *DeletableMemory) Remove(ctx context.Context, node ocispec.Descriptor) error { - m.lock.Lock() - defer m.lock.Unlock() - nodeKey := descriptor.FromOCI(node) - // remove the node from its successors' predecessor list - for successorKey := range m.successors[nodeKey] { - m.predecessors[successorKey].Delete(nodeKey) - } - delete(m.successors, nodeKey) - return nil -} - -// index indexes predecessors for each direct successor of the given node. -func (m *DeletableMemory) index(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) ([]ocispec.Descriptor, error) { - successors, err := content.Successors(ctx, fetcher, node) - if err != nil { - return nil, err - } - m.lock.Lock() - defer m.lock.Unlock() - - // index the node - nodeKey := descriptor.FromOCI(node) - m.nodes[nodeKey] = node - - // index the successors and predecessors - successorSet := set.New[descriptor.Descriptor]() - m.successors[nodeKey] = successorSet - for _, successor := range successors { - successorKey := descriptor.FromOCI(successor) - successorSet.Add(successorKey) - predecessorSet, exists := m.predecessors[successorKey] - if !exists { - predecessorSet = set.New[descriptor.Descriptor]() - m.predecessors[successorKey] = predecessorSet - } - predecessorSet.Add(nodeKey) - } - return successors, nil -} diff --git a/internal/graph/memory.go b/internal/graph/memory.go index 0aa25aee..c74b8c47 100644 --- a/internal/graph/memory.go +++ b/internal/graph/memory.go @@ -23,6 +23,7 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" "oras.land/oras-go/v2/content" "oras.land/oras-go/v2/errdef" + "oras.land/oras-go/v2/internal/container/set" "oras.land/oras-go/v2/internal/descriptor" "oras.land/oras-go/v2/internal/status" "oras.land/oras-go/v2/internal/syncutil" @@ -30,35 +31,31 @@ import ( // Memory is a memory based PredecessorFinder. type Memory struct { - predecessors sync.Map // map[descriptor.Descriptor]map[descriptor.Descriptor]ocispec.Descriptor - indexed sync.Map // map[descriptor.Descriptor]any + nodes map[descriptor.Descriptor]ocispec.Descriptor // nodes saves the map keys of ocispec.Descriptor + predecessors map[descriptor.Descriptor]set.Set[descriptor.Descriptor] + successors map[descriptor.Descriptor]set.Set[descriptor.Descriptor] + lock sync.RWMutex } // NewMemory creates a new memory PredecessorFinder. func NewMemory() *Memory { - return &Memory{} + return &Memory{ + nodes: make(map[descriptor.Descriptor]ocispec.Descriptor), + predecessors: make(map[descriptor.Descriptor]set.Set[descriptor.Descriptor]), + successors: make(map[descriptor.Descriptor]set.Set[descriptor.Descriptor]), + } } // Index indexes predecessors for each direct successor of the given node. -// There is no data consistency issue as long as deletion is not implemented -// for the underlying storage. func (m *Memory) Index(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) error { - successors, err := content.Successors(ctx, fetcher, node) - if err != nil { - return err - } - - m.index(ctx, node, successors) - return nil + _, err := m.index(ctx, fetcher, node) + return err } // Index indexes predecessors for all the successors of the given node. -// There is no data consistency issue as long as deletion is not implemented -// for the underlying storage. func (m *Memory) IndexAll(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) error { // track content status tracker := status.NewTracker() - var fn syncutil.GoFunc[ocispec.Descriptor] fn = func(ctx context.Context, region *syncutil.LimitedRegion, desc ocispec.Descriptor) error { // skip the node if other go routine is working on it @@ -66,15 +63,7 @@ func (m *Memory) IndexAll(ctx context.Context, fetcher content.Fetcher, node oci if !committed { return nil } - - // skip the node if it has been indexed - key := descriptor.FromOCI(desc) - _, exists := m.indexed.Load(key) - if exists { - return nil - } - - successors, err := content.Successors(ctx, fetcher, desc) + successors, err := m.index(ctx, fetcher, desc) if err != nil { if errors.Is(err, errdef.ErrNotFound) { // skip the node if it does not exist @@ -82,9 +71,6 @@ func (m *Memory) IndexAll(ctx context.Context, fetcher content.Fetcher, node oci } return err } - m.index(ctx, desc, successors) - m.indexed.Store(key, nil) - if len(successors) > 0 { // traverse and index successors return syncutil.Go(ctx, nil, fn, successors...) @@ -101,34 +87,58 @@ func (m *Memory) IndexAll(ctx context.Context, fetcher content.Fetcher, node oci // it does not necessarily correspond to any consistent snapshot of the stored // contents. func (m *Memory) Predecessors(_ context.Context, node ocispec.Descriptor) ([]ocispec.Descriptor, error) { + m.lock.RLock() + defer m.lock.RUnlock() key := descriptor.FromOCI(node) - value, exists := m.predecessors.Load(key) + set, exists := m.predecessors[key] if !exists { return nil, nil } - predecessors := value.(*sync.Map) - var res []ocispec.Descriptor - predecessors.Range(func(key, value interface{}) bool { - res = append(res, value.(ocispec.Descriptor)) - return true - }) + for k := range set { + res = append(res, m.nodes[k]) + } return res, nil } +// Remove removes the node from its predecessors and successors. +func (m *Memory) Remove(ctx context.Context, node ocispec.Descriptor) error { + m.lock.Lock() + defer m.lock.Unlock() + nodeKey := descriptor.FromOCI(node) + // remove the node from its successors' predecessor list + for successorKey := range m.successors[nodeKey] { + m.predecessors[successorKey].Delete(nodeKey) + } + delete(m.successors, nodeKey) + return nil +} + // index indexes predecessors for each direct successor of the given node. -// There is no data consistency issue as long as deletion is not implemented -// for the underlying storage. -func (m *Memory) index(ctx context.Context, node ocispec.Descriptor, successors []ocispec.Descriptor) { - if len(successors) == 0 { - return +func (m *Memory) index(ctx context.Context, fetcher content.Fetcher, node ocispec.Descriptor) ([]ocispec.Descriptor, error) { + successors, err := content.Successors(ctx, fetcher, node) + if err != nil { + return nil, err } + m.lock.Lock() + defer m.lock.Unlock() - predecessorKey := descriptor.FromOCI(node) + // index the node + nodeKey := descriptor.FromOCI(node) + m.nodes[nodeKey] = node + + // index the successors and predecessors + successorSet := set.New[descriptor.Descriptor]() + m.successors[nodeKey] = successorSet for _, successor := range successors { successorKey := descriptor.FromOCI(successor) - value, _ := m.predecessors.LoadOrStore(successorKey, &sync.Map{}) - predecessors := value.(*sync.Map) - predecessors.Store(predecessorKey, node) + successorSet.Add(successorKey) + predecessorSet, exists := m.predecessors[successorKey] + if !exists { + predecessorSet = set.New[descriptor.Descriptor]() + m.predecessors[successorKey] = predecessorSet + } + predecessorSet.Add(nodeKey) } + return successors, nil } diff --git a/internal/graph/deletablememory_test.go b/internal/graph/memory_test.go similarity index 99% rename from internal/graph/deletablememory_test.go rename to internal/graph/memory_test.go index 5587174d..d6352e18 100644 --- a/internal/graph/deletablememory_test.go +++ b/internal/graph/memory_test.go @@ -47,7 +47,7 @@ C(layer) D(layer) */ func TestDeletableMemory_IndexAndRemove(t *testing.T) { testFetcher := cas.NewMemory() - testDeletableMemory := NewDeletableMemory() + testDeletableMemory := NewMemory() ctx := context.Background() // generate test content @@ -203,7 +203,7 @@ B(manifest) C(manifest) D(manifest) */ func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { testFetcher := cas.NewMemory() - testDeletableMemory := NewDeletableMemory() + testDeletableMemory := NewMemory() ctx := context.Background() // generate test content From b7a10d48abdb685300cc1dc1b2512e2a99ab0ae2 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Wed, 11 Oct 2023 10:59:00 +0000 Subject: [PATCH 06/13] resolved some comments and update the 1st test Signed-off-by: Xiaoxuan Wang --- internal/graph/memory.go | 2 + internal/graph/memory_test.go | 147 ++++++++++++++++++++-------------- 2 files changed, 88 insertions(+), 61 deletions(-) diff --git a/internal/graph/memory.go b/internal/graph/memory.go index c74b8c47..83779183 100644 --- a/internal/graph/memory.go +++ b/internal/graph/memory.go @@ -89,6 +89,7 @@ func (m *Memory) IndexAll(ctx context.Context, fetcher content.Fetcher, node oci func (m *Memory) Predecessors(_ context.Context, node ocispec.Descriptor) ([]ocispec.Descriptor, error) { m.lock.RLock() defer m.lock.RUnlock() + key := descriptor.FromOCI(node) set, exists := m.predecessors[key] if !exists { @@ -105,6 +106,7 @@ func (m *Memory) Predecessors(_ context.Context, node ocispec.Descriptor) ([]oci func (m *Memory) Remove(ctx context.Context, node ocispec.Descriptor) error { m.lock.Lock() defer m.lock.Unlock() + nodeKey := descriptor.FromOCI(node) // remove the node from its successors' predecessor list for successorKey := range m.successors[nodeKey] { diff --git a/internal/graph/memory_test.go b/internal/graph/memory_test.go index d6352e18..ead9ad77 100644 --- a/internal/graph/memory_test.go +++ b/internal/graph/memory_test.go @@ -29,25 +29,23 @@ import ( "oras.land/oras-go/v2/internal/descriptor" ) -/* -A(manifest)-------------------+ - - | | - | | - | | - v | - -B(manifest)-----------------+ | - - | | | - | | | - v v v - -C(layer) D(layer) -*/ +// A(manifest)-------------------+ +// +// | | +// | | +// | | +// v | +// +// B(manifest)-----------------+ | +// +// | | | +// | | | +// v v v +// +// C(layer) D(layer) func TestDeletableMemory_IndexAndRemove(t *testing.T) { testFetcher := cas.NewMemory() - testDeletableMemory := NewMemory() + testMemory := NewMemory() ctx := context.Background() // generate test content @@ -64,6 +62,7 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { } generateManifest := func(layers ...ocispec.Descriptor) ocispec.Descriptor { manifest := ocispec.Manifest{ + Config: ocispec.Descriptor{MediaType: "test config"}, Layers: layers, } manifestJSON, err := json.Marshal(manifest) @@ -76,10 +75,12 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { descD := appendBlob("layer node D", []byte("Node D is a layer")) // blobs[1], layer "D" descB := generateManifest(descs[0:2]...) // blobs[2], manifest "B" descA := generateManifest(descs[1:3]...) // blobs[3], manifest "A" - testFetcher.Push(ctx, descA, bytes.NewReader(blobs[3])) - testFetcher.Push(ctx, descB, bytes.NewReader(blobs[2])) - testFetcher.Push(ctx, descC, bytes.NewReader(blobs[0])) - testFetcher.Push(ctx, descD, bytes.NewReader(blobs[1])) + + // prepare the content in the fetcher, so that it can be used to test Index. + testContents := []ocispec.Descriptor{descC, descD, descB, descA} + for i := 0; i < len(blobs); i++ { + testFetcher.Push(ctx, testContents[i], bytes.NewReader(blobs[i])) + } // make sure that testFetcher works rc, err := testFetcher.Fetch(ctx, descA) @@ -98,70 +99,97 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { t.Errorf("testFetcher.Fetch() = %v, want %v", got, blobs[4]) } - // index test content into graph.DeletableMemory - testDeletableMemory.Index(ctx, testFetcher, descA) - testDeletableMemory.Index(ctx, testFetcher, descB) - testDeletableMemory.Index(ctx, testFetcher, descC) - testDeletableMemory.Index(ctx, testFetcher, descD) - - // check that the content of testDeletableMemory.predecessors and successors - // are correct nodeKeyA := descriptor.FromOCI(descA) nodeKeyB := descriptor.FromOCI(descB) nodeKeyC := descriptor.FromOCI(descC) nodeKeyD := descriptor.FromOCI(descD) - // check the information of node A - predecessorsA := testDeletableMemory.predecessors[nodeKeyA] - successorsA := testDeletableMemory.successors[nodeKeyA] - for range predecessorsA { - t.Errorf("predecessors of %s should be empty", "A") + // index and check the information of node D + testMemory.Index(ctx, testFetcher, descD) + successorsD := testMemory.successors[nodeKeyD] + for range successorsD { + t.Errorf("successors of %s should be empty", "C") } + // there should be no entry of D in testMemory.predecessors yet. + _, exist := testMemory.predecessors[nodeKeyD] + if exist { + t.Errorf("predecessor entry of %s should not exist yet", "D") + } + + // index and check the information of node C + testMemory.Index(ctx, testFetcher, descC) + successorsC := testMemory.successors[nodeKeyC] + for range successorsC { + t.Errorf("successors of %s should be empty", "C") + } + // there should be no entry of C in testMemory.predecessors yet. + _, exist = testMemory.predecessors[nodeKeyC] + if exist { + t.Errorf("predecessor entry of %s should not exist yet", "C") + } + + // index and check the information of node A + testMemory.Index(ctx, testFetcher, descA) + successorsA := testMemory.successors[nodeKeyA] if !successorsA.Contains(nodeKeyB) { t.Errorf("successors of %s should contain %s", "A", "B") } if !successorsA.Contains(nodeKeyD) { t.Errorf("successors of %s should contain %s", "A", "D") } - - // check the information of node B - predecessorsB := testDeletableMemory.predecessors[nodeKeyB] - successorsB := testDeletableMemory.successors[nodeKeyB] - if !predecessorsB.Contains(nodeKeyA) { - t.Errorf("predecessors of %s should contain %s", "B", "A") + // there should be no entry of A in testMemory.predecessors. + _, exist = testMemory.predecessors[nodeKeyA] + if exist { + t.Errorf("predecessor entry of %s should not exist", "A") + } + // there should be an entry of D in testMemory.predecessors + // and it should contain A. + predecessorsD, exist := testMemory.predecessors[nodeKeyD] + if !exist { + t.Errorf("predecessor entry of %s should exist now", "D") + } + if !predecessorsD.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should contain %s", "D", "A") } + + // index and check the information of node B + testMemory.Index(ctx, testFetcher, descB) + successorsB := testMemory.successors[nodeKeyB] if !successorsB.Contains(nodeKeyC) { t.Errorf("successors of %s should contain %s", "B", "C") } if !successorsB.Contains(nodeKeyD) { t.Errorf("successors of %s should contain %s", "B", "D") } - - // check the information of node C - predecessorsC := testDeletableMemory.predecessors[nodeKeyC] - successorsC := testDeletableMemory.successors[nodeKeyC] + // there should be an entry of B in testMemory.predecessors + // and it should contain A. + predecessorsB, exist := testMemory.predecessors[nodeKeyB] + if !exist { + t.Errorf("predecessor entry of %s should exist already", "B") + } + if !predecessorsB.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should contain %s", "B", "A") + } + // there should be an entry of C in testMemory.predecessors + // and it should contain B. + predecessorsC, exist := testMemory.predecessors[nodeKeyC] + if !exist { + t.Errorf("predecessor entry of %s should exist now", "C") + } if !predecessorsC.Contains(nodeKeyB) { t.Errorf("predecessors of %s should contain %s", "C", "B") } - for range successorsC { - t.Errorf("successors of %s should be empty", "C") - } - // check the information of node D - predecessorsD := testDeletableMemory.predecessors[nodeKeyD] - successorsD := testDeletableMemory.successors[nodeKeyD] + // predecessors of D should have been updated now if !predecessorsD.Contains(nodeKeyB) { t.Errorf("predecessors of %s should contain %s", "D", "B") } if !predecessorsD.Contains(nodeKeyA) { t.Errorf("predecessors of %s should contain %s", "D", "A") } - for range successorsD { - t.Errorf("successors of %s should be empty", "C") - } // remove node B and check the stored information - testDeletableMemory.Remove(ctx, descB) + testMemory.Remove(ctx, descB) if predecessorsC.Contains(nodeKeyB) { t.Errorf("predecessors of %s should not contain %s", "C", "B") } @@ -171,16 +199,16 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { if !successorsA.Contains(nodeKeyB) { t.Errorf("successors of %s should still contain %s", "A", "B") } - if _, exists := testDeletableMemory.successors[nodeKeyB]; exists { + if _, exists := testMemory.successors[nodeKeyB]; exists { t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "B") } // remove node A and check the stored information - testDeletableMemory.Remove(ctx, descA) + testMemory.Remove(ctx, descA) if predecessorsD.Contains(nodeKeyA) { t.Errorf("predecessors of %s should not contain %s", "D", "A") } - if _, exists := testDeletableMemory.successors[nodeKeyA]; exists { + if _, exists := testMemory.successors[nodeKeyA]; exists { t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "A") } } @@ -192,10 +220,7 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { | | | | | | v v v -B(manifest) C(manifest) D(manifest) -| | | -| | | -| | | +B(manifest) C(manifest) D (manifest) | | | | | | | | v From e32a36c03aed6fe554ef95ac8573ff3f9ab49d50 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Thu, 12 Oct 2023 09:55:36 +0000 Subject: [PATCH 07/13] cleaned up remove test and updated remove behavior Signed-off-by: Xiaoxuan Wang --- internal/graph/memory.go | 11 +++- internal/graph/memory_test.go | 95 ++++++++++++++++++++++++----------- 2 files changed, 76 insertions(+), 30 deletions(-) diff --git a/internal/graph/memory.go b/internal/graph/memory.go index 83779183..b7cf388e 100644 --- a/internal/graph/memory.go +++ b/internal/graph/memory.go @@ -110,7 +110,13 @@ func (m *Memory) Remove(ctx context.Context, node ocispec.Descriptor) error { nodeKey := descriptor.FromOCI(node) // remove the node from its successors' predecessor list for successorKey := range m.successors[nodeKey] { - m.predecessors[successorKey].Delete(nodeKey) + predecessorEntry := m.predecessors[successorKey] + predecessorEntry.Delete(nodeKey) + // remove the successorKey entry from predecessors, if it's an empty set + // i.e. if all its predecessors are already deleted + if len(predecessorEntry) == 0 { + delete(m.predecessors, successorKey) + } } delete(m.successors, nodeKey) return nil @@ -129,7 +135,8 @@ func (m *Memory) index(ctx context.Context, fetcher content.Fetcher, node ocispe nodeKey := descriptor.FromOCI(node) m.nodes[nodeKey] = node - // index the successors and predecessors + // for each successor, put it into the node's successors list, and + // put node into the succeesor's predecessors list successorSet := set.New[descriptor.Descriptor]() m.successors[nodeKey] = successorSet for _, successor := range successors { diff --git a/internal/graph/memory_test.go b/internal/graph/memory_test.go index ead9ad77..d36358b2 100644 --- a/internal/graph/memory_test.go +++ b/internal/graph/memory_test.go @@ -106,19 +106,31 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { // index and check the information of node D testMemory.Index(ctx, testFetcher, descD) - successorsD := testMemory.successors[nodeKeyD] + successorsD, exist := testMemory.successors[nodeKeyD] + if !exist { + t.Errorf("successor entry of %s should exist", "D") + } + if successorsD == nil { + t.Errorf("successors of %s should not be nil", "D") + } for range successorsD { t.Errorf("successors of %s should be empty", "C") } // there should be no entry of D in testMemory.predecessors yet. - _, exist := testMemory.predecessors[nodeKeyD] + _, exist = testMemory.predecessors[nodeKeyD] if exist { t.Errorf("predecessor entry of %s should not exist yet", "D") } // index and check the information of node C testMemory.Index(ctx, testFetcher, descC) - successorsC := testMemory.successors[nodeKeyC] + successorsC, exist := testMemory.successors[nodeKeyC] + if !exist { + t.Errorf("successor entry of %s should exist", "C") + } + if successorsC == nil { + t.Errorf("successors of %s should not be nil", "C") + } for range successorsC { t.Errorf("successors of %s should be empty", "C") } @@ -142,15 +154,27 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { if exist { t.Errorf("predecessor entry of %s should not exist", "A") } - // there should be an entry of D in testMemory.predecessors - // and it should contain A. + // there should be an entry of D in testMemory.predecessors by now + // and it should contain A but not B. predecessorsD, exist := testMemory.predecessors[nodeKeyD] if !exist { - t.Errorf("predecessor entry of %s should exist now", "D") + t.Errorf("predecessor entry of %s should exist by now", "D") } if !predecessorsD.Contains(nodeKeyA) { t.Errorf("predecessors of %s should contain %s", "D", "A") } + if predecessorsD.Contains(nodeKeyB) { + t.Errorf("predecessors of %s should not contain %s yet", "D", "B") + } + // there should be an entry of B in testMemory.predecessors now + // and it should contain A. + predecessorsB, exist := testMemory.predecessors[nodeKeyB] + if !exist { + t.Errorf("predecessor entry of %s should exist by now", "B") + } + if !predecessorsB.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should contain %s", "B", "A") + } // index and check the information of node B testMemory.Index(ctx, testFetcher, descB) @@ -161,26 +185,16 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { if !successorsB.Contains(nodeKeyD) { t.Errorf("successors of %s should contain %s", "B", "D") } - // there should be an entry of B in testMemory.predecessors - // and it should contain A. - predecessorsB, exist := testMemory.predecessors[nodeKeyB] - if !exist { - t.Errorf("predecessor entry of %s should exist already", "B") - } - if !predecessorsB.Contains(nodeKeyA) { - t.Errorf("predecessors of %s should contain %s", "B", "A") - } - // there should be an entry of C in testMemory.predecessors + // there should be an entry of C in testMemory.predecessors by now // and it should contain B. predecessorsC, exist := testMemory.predecessors[nodeKeyC] if !exist { - t.Errorf("predecessor entry of %s should exist now", "C") + t.Errorf("predecessor entry of %s should exist by now", "C") } if !predecessorsC.Contains(nodeKeyB) { t.Errorf("predecessors of %s should contain %s", "C", "B") } - - // predecessors of D should have been updated now + // predecessors of D should have been updated now. if !predecessorsD.Contains(nodeKeyB) { t.Errorf("predecessors of %s should contain %s", "D", "B") } @@ -190,27 +204,52 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { // remove node B and check the stored information testMemory.Remove(ctx, descB) - if predecessorsC.Contains(nodeKeyB) { - t.Errorf("predecessors of %s should not contain %s", "C", "B") + // verify B' predecessors info: B's entry in testMemory.predecessors should + // still exist, since its predecessor A still exists + predecessorsB, exists := testMemory.predecessors[nodeKeyB] + if !exists { + t.Errorf("testDeletableMemory.predecessors should still contain the entry of %v", "B") } - if predecessorsD.Contains(nodeKeyB) { - t.Errorf("predecessors of %s should not contain %s", "D", "B") + if !predecessorsB.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should still contain %s", "B", "A") + } + // verify B' successors info: B's entry in testMemory.successors should no + // longer exist + if _, exists := testMemory.successors[nodeKeyB]; exists { + t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "B") } + // verify B' predecessors' successors info: B should still exist in A's + // successors if !successorsA.Contains(nodeKeyB) { t.Errorf("successors of %s should still contain %s", "A", "B") } - if _, exists := testMemory.successors[nodeKeyB]; exists { - t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "B") + // verify B' successors' predecessors info: C's entry in testMemory.predecessors + // should no longer exist, since C's only predecessor B is already deleted. + // B should no longer exist in D's predecessors. + if _, exists = testMemory.predecessors[nodeKeyC]; exists { + t.Errorf("predecessor entry of %s should no longer exist by now, since all its predecessors have been deleted", "C") + } + if predecessorsD.Contains(nodeKeyB) { + t.Errorf("predecessors of %s should not contain %s", "D", "B") } // remove node A and check the stored information testMemory.Remove(ctx, descA) - if predecessorsD.Contains(nodeKeyA) { - t.Errorf("predecessors of %s should not contain %s", "D", "A") - } + // verify A' successors info: A's entry in testMemory.successors should no + // longer exist if _, exists := testMemory.successors[nodeKeyA]; exists { t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "A") } + // verify A' successors' predecessors info: D's entry in testMemory.predecessors + // should no longer exist, since all predecessors of D are already deleted. + // B's entry in testMemory.predecessors should no longer exist, since B's only + // predecessor A is already deleted. + if _, exists = testMemory.predecessors[nodeKeyD]; exists { + t.Errorf("predecessor entry of %s should no longer exist by now, since all its predecessors have been deleted", "D") + } + if _, exists = testMemory.predecessors[nodeKeyC]; exists { + t.Errorf("predecessor entry of %s should no longer exist by now, since all its predecessors have been deleted", "C") + } } /* From 02948ffc65b3095c8c2128a4218d9e761d3345bf Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Thu, 12 Oct 2023 12:55:42 +0000 Subject: [PATCH 08/13] done refining the 1st test Signed-off-by: Xiaoxuan Wang --- internal/graph/memory.go | 1 + internal/graph/memory_test.go | 144 ++++++++++++++++++++++------------ 2 files changed, 97 insertions(+), 48 deletions(-) diff --git a/internal/graph/memory.go b/internal/graph/memory.go index b7cf388e..e3228890 100644 --- a/internal/graph/memory.go +++ b/internal/graph/memory.go @@ -119,6 +119,7 @@ func (m *Memory) Remove(ctx context.Context, node ocispec.Descriptor) error { } } delete(m.successors, nodeKey) + delete(m.nodes, nodeKey) return nil } diff --git a/internal/graph/memory_test.go b/internal/graph/memory_test.go index d36358b2..432f48ba 100644 --- a/internal/graph/memory_test.go +++ b/internal/graph/memory_test.go @@ -76,7 +76,7 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { descB := generateManifest(descs[0:2]...) // blobs[2], manifest "B" descA := generateManifest(descs[1:3]...) // blobs[3], manifest "A" - // prepare the content in the fetcher, so that it can be used to test Index. + // prepare the content in the fetcher, so that it can be used to test Index testContents := []ocispec.Descriptor{descC, descD, descB, descA} for i := 0; i < len(blobs); i++ { testFetcher.Push(ctx, testContents[i], bytes.NewReader(blobs[i])) @@ -106,58 +106,76 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { // index and check the information of node D testMemory.Index(ctx, testFetcher, descD) - successorsD, exist := testMemory.successors[nodeKeyD] - if !exist { + // 1. verify its existence in testMemory.nodes + if _, exists := testMemory.nodes[nodeKeyD]; !exists { + t.Errorf("nodes entry of %s should exist", "D") + } + // 2. verify that the entry of D exists in testMemory.successors and it's empty + successorsD, exists := testMemory.successors[nodeKeyD] + if !exists { t.Errorf("successor entry of %s should exist", "D") } if successorsD == nil { - t.Errorf("successors of %s should not be nil", "D") + t.Errorf("successors of %s should be an empty set, not nil", "D") } - for range successorsD { - t.Errorf("successors of %s should be empty", "C") + if len(successorsD) != 0 { + t.Errorf("successors of %s should be empty", "D") } - // there should be no entry of D in testMemory.predecessors yet. - _, exist = testMemory.predecessors[nodeKeyD] - if exist { + // 3. there should be no entry of D in testMemory.predecessors yet + _, exists = testMemory.predecessors[nodeKeyD] + if exists { t.Errorf("predecessor entry of %s should not exist yet", "D") } // index and check the information of node C testMemory.Index(ctx, testFetcher, descC) - successorsC, exist := testMemory.successors[nodeKeyC] - if !exist { + // 1. verify its existence in memory.nodes + if _, exists := testMemory.nodes[nodeKeyC]; !exists { + t.Errorf("nodes entry of %s should exist", "C") + } + // 2. verify that the entry of C exists in testMemory.successors and it's empty + successorsC, exists := testMemory.successors[nodeKeyC] + if !exists { t.Errorf("successor entry of %s should exist", "C") } if successorsC == nil { - t.Errorf("successors of %s should not be nil", "C") + t.Errorf("successors of %s should be an empty set, not nil", "C") } - for range successorsC { + if len(successorsC) != 0 { t.Errorf("successors of %s should be empty", "C") } - // there should be no entry of C in testMemory.predecessors yet. - _, exist = testMemory.predecessors[nodeKeyC] - if exist { + // 3. there should be no entry of C in testMemory.predecessors yet + _, exists = testMemory.predecessors[nodeKeyC] + if exists { t.Errorf("predecessor entry of %s should not exist yet", "C") } // index and check the information of node A testMemory.Index(ctx, testFetcher, descA) - successorsA := testMemory.successors[nodeKeyA] + // 1. verify its existence in testMemory.nodes + if _, exists := testMemory.nodes[nodeKeyA]; !exists { + t.Errorf("nodes entry of %s should exist", "A") + } + // 2. verify that the entry of A exists in testMemory.successors and it contains + // node B and node D + successorsA, exists := testMemory.successors[nodeKeyA] + if !exists { + t.Errorf("successor entry of %s should exist", "A") + } + if successorsA == nil { + t.Errorf("successors of %s should be a set, not nil", "A") + } if !successorsA.Contains(nodeKeyB) { t.Errorf("successors of %s should contain %s", "A", "B") } if !successorsA.Contains(nodeKeyD) { t.Errorf("successors of %s should contain %s", "A", "D") } - // there should be no entry of A in testMemory.predecessors. - _, exist = testMemory.predecessors[nodeKeyA] - if exist { - t.Errorf("predecessor entry of %s should not exist", "A") - } - // there should be an entry of D in testMemory.predecessors by now - // and it should contain A but not B. - predecessorsD, exist := testMemory.predecessors[nodeKeyD] - if !exist { + // 3. verify that node A exists in the predecessors lists of its successors. + // there should be an entry of D in testMemory.predecessors by now and it + // should contain A but not B + predecessorsD, exists := testMemory.predecessors[nodeKeyD] + if !exists { t.Errorf("predecessor entry of %s should exist by now", "D") } if !predecessorsD.Contains(nodeKeyA) { @@ -167,34 +185,52 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { t.Errorf("predecessors of %s should not contain %s yet", "D", "B") } // there should be an entry of B in testMemory.predecessors now - // and it should contain A. - predecessorsB, exist := testMemory.predecessors[nodeKeyB] - if !exist { + // and it should contain A + predecessorsB, exists := testMemory.predecessors[nodeKeyB] + if !exists { t.Errorf("predecessor entry of %s should exist by now", "B") } if !predecessorsB.Contains(nodeKeyA) { t.Errorf("predecessors of %s should contain %s", "B", "A") } + // 4. there should be no entry of A in testMemory.predecessors + _, exists = testMemory.predecessors[nodeKeyA] + if exists { + t.Errorf("predecessor entry of %s should not exist", "A") + } // index and check the information of node B testMemory.Index(ctx, testFetcher, descB) - successorsB := testMemory.successors[nodeKeyB] + // 1. verify its existence in testMemory.nodes + if _, exists := testMemory.nodes[nodeKeyB]; !exists { + t.Errorf("nodes entry of %s should exist", "B") + } + // 2. verify that the entry of B exists in testMemory.successors and it contains + // node C and node D + successorsB, exists := testMemory.successors[nodeKeyB] + if !exists { + t.Errorf("successor entry of %s should exist", "B") + } + if successorsB == nil { + t.Errorf("successors of %s should be a set, not nil", "B") + } if !successorsB.Contains(nodeKeyC) { t.Errorf("successors of %s should contain %s", "B", "C") } if !successorsB.Contains(nodeKeyD) { t.Errorf("successors of %s should contain %s", "B", "D") } + // 3. verify that node B exists in the predecessors lists of its successors. // there should be an entry of C in testMemory.predecessors by now - // and it should contain B. - predecessorsC, exist := testMemory.predecessors[nodeKeyC] - if !exist { + // and it should contain B + predecessorsC, exists := testMemory.predecessors[nodeKeyC] + if !exists { t.Errorf("predecessor entry of %s should exist by now", "C") } if !predecessorsC.Contains(nodeKeyB) { t.Errorf("predecessors of %s should contain %s", "C", "B") } - // predecessors of D should have been updated now. + // predecessors of D should have been updated now to have node A and B if !predecessorsD.Contains(nodeKeyB) { t.Errorf("predecessors of %s should contain %s", "D", "B") } @@ -204,51 +240,63 @@ func TestDeletableMemory_IndexAndRemove(t *testing.T) { // remove node B and check the stored information testMemory.Remove(ctx, descB) - // verify B' predecessors info: B's entry in testMemory.predecessors should + // 1. verify that node B no longer exists in testMemory.nodes + if _, exists := testMemory.nodes[nodeKeyB]; exists { + t.Errorf("nodes entry of %s should no longer exist", "B") + } + // 2. verify B' predecessors info: B's entry in testMemory.predecessors should // still exist, since its predecessor A still exists - predecessorsB, exists := testMemory.predecessors[nodeKeyB] + predecessorsB, exists = testMemory.predecessors[nodeKeyB] if !exists { t.Errorf("testDeletableMemory.predecessors should still contain the entry of %v", "B") } if !predecessorsB.Contains(nodeKeyA) { t.Errorf("predecessors of %s should still contain %s", "B", "A") } - // verify B' successors info: B's entry in testMemory.successors should no + // 3. verify B' successors info: B's entry in testMemory.successors should no // longer exist if _, exists := testMemory.successors[nodeKeyB]; exists { t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "B") } - // verify B' predecessors' successors info: B should still exist in A's + // 4. verify B' predecessors' successors info: B should still exist in A's // successors if !successorsA.Contains(nodeKeyB) { t.Errorf("successors of %s should still contain %s", "A", "B") } - // verify B' successors' predecessors info: C's entry in testMemory.predecessors - // should no longer exist, since C's only predecessor B is already deleted. - // B should no longer exist in D's predecessors. + // 5. verify B' successors' predecessors info: C's entry in testMemory.predecessors + // should no longer exist, since C's only predecessor B is already deleted if _, exists = testMemory.predecessors[nodeKeyC]; exists { t.Errorf("predecessor entry of %s should no longer exist by now, since all its predecessors have been deleted", "C") } + // B should no longer exist in D's predecessors if predecessorsD.Contains(nodeKeyB) { t.Errorf("predecessors of %s should not contain %s", "D", "B") } + // but A still exists in D's predecessors + if !predecessorsD.Contains(nodeKeyA) { + t.Errorf("predecessors of %s should still contain %s", "D", "A") + } // remove node A and check the stored information testMemory.Remove(ctx, descA) - // verify A' successors info: A's entry in testMemory.successors should no + // 1. verify that node A no longer exists in testMemory.nodes + if _, exists := testMemory.nodes[nodeKeyA]; exists { + t.Errorf("nodes entry of %s should no longer exist", "A") + } + // 2. verify A' successors info: A's entry in testMemory.successors should no // longer exist if _, exists := testMemory.successors[nodeKeyA]; exists { t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "A") } - // verify A' successors' predecessors info: D's entry in testMemory.predecessors - // should no longer exist, since all predecessors of D are already deleted. - // B's entry in testMemory.predecessors should no longer exist, since B's only - // predecessor A is already deleted. + // 3. verify A' successors' predecessors info: D's entry in testMemory.predecessors + // should no longer exist, since all predecessors of D are already deleted if _, exists = testMemory.predecessors[nodeKeyD]; exists { t.Errorf("predecessor entry of %s should no longer exist by now, since all its predecessors have been deleted", "D") } - if _, exists = testMemory.predecessors[nodeKeyC]; exists { - t.Errorf("predecessor entry of %s should no longer exist by now, since all its predecessors have been deleted", "C") + // B's entry in testMemory.predecessors should no longer exist, since B's only + // predecessor A is already deleted + if _, exists = testMemory.predecessors[nodeKeyB]; exists { + t.Errorf("predecessor entry of %s should no longer exist by now, since all its predecessors have been deleted", "B") } } From 3f4848d7b13493380f565a3a8869a513867cb97a Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Thu, 12 Oct 2023 13:29:44 +0000 Subject: [PATCH 09/13] refined the second test Signed-off-by: Xiaoxuan Wang --- internal/graph/memory_test.go | 125 ++++++++++++++++++++++++---------- 1 file changed, 88 insertions(+), 37 deletions(-) diff --git a/internal/graph/memory_test.go b/internal/graph/memory_test.go index 432f48ba..b2b4eb44 100644 --- a/internal/graph/memory_test.go +++ b/internal/graph/memory_test.go @@ -43,7 +43,7 @@ import ( // v v v // // C(layer) D(layer) -func TestDeletableMemory_IndexAndRemove(t *testing.T) { +func TestMemory_IndexAndRemove(t *testing.T) { testFetcher := cas.NewMemory() testMemory := NewMemory() ctx := context.Background() @@ -313,9 +313,9 @@ B(manifest) C(manifest) D (manifest) | | v +-->E(layer)<-+------->F(layer) */ -func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { +func TestMemory_IndexAllAndPredecessors(t *testing.T) { testFetcher := cas.NewMemory() - testDeletableMemory := NewMemory() + testMemory := NewMemory() ctx := context.Background() // generate test content @@ -356,12 +356,12 @@ func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { descC := generateManifest(descriptors[0:2]...) // blobs[3], manifest "C" descD := generateManifest(descriptors[1:2]...) // blobs[4], manifest "D" descA := generateIndex(descriptors[2:5]...) // blobs[5], index "A" - testFetcher.Push(ctx, descA, bytes.NewReader(blobs[5])) - testFetcher.Push(ctx, descB, bytes.NewReader(blobs[2])) - testFetcher.Push(ctx, descC, bytes.NewReader(blobs[3])) - testFetcher.Push(ctx, descD, bytes.NewReader(blobs[4])) - testFetcher.Push(ctx, descE, bytes.NewReader(blobs[0])) - testFetcher.Push(ctx, descF, bytes.NewReader(blobs[1])) + + // prepare the content in the fetcher, so that it can be used to test IndexAll + testContents := []ocispec.Descriptor{descE, descF, descB, descC, descD, descA} + for i := 0; i < len(blobs); i++ { + testFetcher.Push(ctx, testContents[i], bytes.NewReader(blobs[i])) + } // make sure that testFetcher works rc, err := testFetcher.Fetch(ctx, descA) @@ -380,11 +380,6 @@ func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { t.Errorf("testFetcher.Fetch() = %v, want %v", got, blobs[4]) } - // index node A into graph.DeletableMemory using IndexAll - testDeletableMemory.IndexAll(ctx, testFetcher, descA) - - // check that the content of testDeletableMemory.predecessors and successors - // are correct nodeKeyA := descriptor.FromOCI(descA) nodeKeyB := descriptor.FromOCI(descB) nodeKeyC := descriptor.FromOCI(descC) @@ -392,11 +387,22 @@ func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { nodeKeyE := descriptor.FromOCI(descE) nodeKeyF := descriptor.FromOCI(descF) + // index node A into testMemory using IndexAll + testMemory.IndexAll(ctx, testFetcher, descA) + // check the information of node A - predecessorsA := testDeletableMemory.predecessors[nodeKeyA] - successorsA := testDeletableMemory.successors[nodeKeyA] - for range predecessorsA { - t.Errorf("predecessors of %s should be empty", "A") + // 1. verify that node A exists in testMemory.nodes + if _, exists := testMemory.nodes[nodeKeyA]; !exists { + t.Errorf("nodes entry of %s should exist", "A") + } + // 2. verify that there is no entry of A in predecessors + if _, exists := testMemory.predecessors[nodeKeyA]; exists { + t.Errorf("there should be no entry of %s in predecessors", "A") + } + // 3. verify that A has successors B, C, D + successorsA, exists := testMemory.successors[nodeKeyA] + if !exists { + t.Errorf("there should be an entry of %s in successors", "A") } if !successorsA.Contains(nodeKeyB) { t.Errorf("successors of %s should contain %s", "A", "B") @@ -409,21 +415,33 @@ func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { } // check the information of node B - predecessorsB := testDeletableMemory.predecessors[nodeKeyB] - successorsB := testDeletableMemory.successors[nodeKeyB] + // 1. verify that node B exists in testMemory.nodes + if _, exists := testMemory.nodes[nodeKeyB]; !exists { + t.Errorf("nodes entry of %s should exist", "B") + } + // 2. verify that B has node A in its predecessors + predecessorsB := testMemory.predecessors[nodeKeyB] if !predecessorsB.Contains(nodeKeyA) { t.Errorf("predecessors of %s should contain %s", "B", "A") } + // 3. verify that B has node E in its successors + successorsB := testMemory.successors[nodeKeyB] if !successorsB.Contains(nodeKeyE) { t.Errorf("successors of %s should contain %s", "B", "E") } // check the information of node C - predecessorsC := testDeletableMemory.predecessors[nodeKeyC] - successorsC := testDeletableMemory.successors[nodeKeyC] + // 1. verify that node C exists in testMemory.nodes + if _, exists := testMemory.nodes[nodeKeyC]; !exists { + t.Errorf("nodes entry of %s should exist", "C") + } + // 2. verify that C has node A in its predecessors + predecessorsC := testMemory.predecessors[nodeKeyC] if !predecessorsC.Contains(nodeKeyA) { t.Errorf("predecessors of %s should contain %s", "C", "A") } + // 3. verify that C has node E and F in its successors + successorsC := testMemory.successors[nodeKeyC] if !successorsC.Contains(nodeKeyE) { t.Errorf("successors of %s should contain %s", "C", "E") } @@ -432,43 +450,73 @@ func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { } // check the information of node D - predecessorsD := testDeletableMemory.predecessors[nodeKeyD] - successorsD := testDeletableMemory.successors[nodeKeyD] + // 1. verify that node D exists in testMemory.nodes + if _, exists := testMemory.nodes[nodeKeyD]; !exists { + t.Errorf("nodes entry of %s should exist", "D") + } + // 2. verify that D has node A in its predecessors + predecessorsD := testMemory.predecessors[nodeKeyD] if !predecessorsD.Contains(nodeKeyA) { t.Errorf("predecessors of %s should contain %s", "D", "A") } + // 3. verify that D has node F in its successors + successorsD := testMemory.successors[nodeKeyD] if !successorsD.Contains(nodeKeyF) { t.Errorf("successors of %s should contain %s", "D", "F") } // check the information of node E - predecessorsE := testDeletableMemory.predecessors[nodeKeyE] - successorsE := testDeletableMemory.successors[nodeKeyE] + // 1. verify that node E exists in testMemory.nodes + if _, exists := testMemory.nodes[nodeKeyE]; !exists { + t.Errorf("nodes entry of %s should exist", "E") + } + // 2. verify that E has node B and C in its predecessors + predecessorsE := testMemory.predecessors[nodeKeyE] if !predecessorsE.Contains(nodeKeyB) { t.Errorf("predecessors of %s should contain %s", "E", "B") } if !predecessorsE.Contains(nodeKeyC) { t.Errorf("predecessors of %s should contain %s", "E", "C") } - for range successorsE { + // 3. verify that E has an entry in successors and it's empty + successorsE, exists := testMemory.successors[nodeKeyE] + if !exists { + t.Errorf("entry %s should exist in testMemory.successors", "E") + } + if successorsE == nil { + t.Errorf("successors of %s should be an empty set, not nil", "E") + } + if len(successorsE) != 0 { t.Errorf("successors of %s should be empty", "E") } // check the information of node F - predecessorsF := testDeletableMemory.predecessors[nodeKeyF] - successorsF := testDeletableMemory.successors[nodeKeyF] + // 1. verify that node F exists in testMemory.nodes + if _, exists := testMemory.nodes[nodeKeyF]; !exists { + t.Errorf("nodes entry of %s should exist", "F") + } + // 2. verify that F has node C and D in its predecessors + predecessorsF := testMemory.predecessors[nodeKeyF] if !predecessorsF.Contains(nodeKeyC) { t.Errorf("predecessors of %s should contain %s", "F", "C") } if !predecessorsF.Contains(nodeKeyD) { t.Errorf("predecessors of %s should contain %s", "F", "D") } - for range successorsF { - t.Errorf("successors of %s should be empty", "E") + // 3. verify that F has an entry in successors and it's empty + successorsF, exists := testMemory.successors[nodeKeyF] + if !exists { + t.Errorf("entry %s should exist in testMemory.successors", "F") + } + if successorsF == nil { + t.Errorf("successors of %s should be an empty set, not nil", "F") + } + if len(successorsF) != 0 { + t.Errorf("successors of %s should be empty", "F") } // check the Predecessors of node C is node A - predsC, err := testDeletableMemory.Predecessors(ctx, descC) + predsC, err := testMemory.Predecessors(ctx, descC) if err != nil { t.Errorf("testFetcher.Predecessors() error = %v", err) } @@ -481,7 +529,7 @@ func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { } // check the Predecessors of node F are node C and node D - predsF, err := testDeletableMemory.Predecessors(ctx, descF) + predsF, err := testMemory.Predecessors(ctx, descF) if err != nil { t.Errorf("testFetcher.Predecessors() error = %v", err) } @@ -496,7 +544,7 @@ func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { } // remove node C and check the stored information - testDeletableMemory.Remove(ctx, descC) + testMemory.Remove(ctx, descC) if predecessorsE.Contains(nodeKeyC) { t.Errorf("predecessors of %s should not contain %s", "E", "C") } @@ -506,19 +554,22 @@ func TestDeletableMemory_IndexAllAndPredecessors(t *testing.T) { if !successorsA.Contains(nodeKeyC) { t.Errorf("successors of %s should still contain %s", "A", "C") } - if _, exists := testDeletableMemory.successors[nodeKeyC]; exists { + if _, exists := testMemory.successors[nodeKeyC]; exists { t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "C") } // remove node A and check the stored information - testDeletableMemory.Remove(ctx, descA) + testMemory.Remove(ctx, descA) if predecessorsD.Contains(nodeKeyA) { t.Errorf("predecessors of %s should not contain %s", "D", "A") } if predecessorsB.Contains(nodeKeyA) { t.Errorf("predecessors of %s should not contain %s", "B", "A") } - if _, exists := testDeletableMemory.successors[nodeKeyA]; exists { + if _, exists := testMemory.predecessors[nodeKeyC]; exists { + t.Errorf("entry %s in predecessors should no longer exists", "C") + } + if _, exists := testMemory.successors[nodeKeyA]; exists { t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "A") } } From 66b17369cc6dee404221930a027884face377293 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Thu, 12 Oct 2023 13:56:19 +0000 Subject: [PATCH 10/13] refined doc of Remove Signed-off-by: Xiaoxuan Wang --- internal/graph/memory.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/graph/memory.go b/internal/graph/memory.go index e3228890..3978436c 100644 --- a/internal/graph/memory.go +++ b/internal/graph/memory.go @@ -112,8 +112,11 @@ func (m *Memory) Remove(ctx context.Context, node ocispec.Descriptor) error { for successorKey := range m.successors[nodeKey] { predecessorEntry := m.predecessors[successorKey] predecessorEntry.Delete(nodeKey) - // remove the successorKey entry from predecessors, if it's an empty set - // i.e. if all its predecessors are already deleted + // remove the successorKey entry from m. predecessors, if it's an empty set. + // To it another way, if all its predecessors are already deleted, then we + // delete the entry in m.predecessors. In any of its predecessors still exists, + // the entry is NOT deleted. The presence of the predecessors entry depends on + // the status of all predecessors of the node. if len(predecessorEntry) == 0 { delete(m.predecessors, successorKey) } From 8c0d22acfd9639838c93a39e31333e0b691fa10a Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Thu, 12 Oct 2023 14:34:45 +0000 Subject: [PATCH 11/13] refined the drawing! Signed-off-by: Xiaoxuan Wang --- internal/graph/memory_test.go | 65 ++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/internal/graph/memory_test.go b/internal/graph/memory_test.go index b2b4eb44..51d2f450 100644 --- a/internal/graph/memory_test.go +++ b/internal/graph/memory_test.go @@ -29,20 +29,25 @@ import ( "oras.land/oras-go/v2/internal/descriptor" ) -// A(manifest)-------------------+ -// -// | | -// | | -// | | -// v | -// -// B(manifest)-----------------+ | -// -// | | | -// | | | -// v v v -// -// C(layer) D(layer) +// +------------------------------+ +// | | +// | +-----------+ | +// | |A(manifest)| | +// | +-----+-----+ | +// | | | +// | +------------+ | +// | | | | +// | v v | +// | +-----+-----+ +---+----+ | +// | |B(manifest)| |C(layer)| | +// | +-----+-----+ +--------+ | +// | | | +// | v | +// | +---+----+ | +// | |D(layer)| | +// | +--------+ | +// | | +// |------------------------------+ func TestMemory_IndexAndRemove(t *testing.T) { testFetcher := cas.NewMemory() testMemory := NewMemory() @@ -300,19 +305,25 @@ func TestMemory_IndexAndRemove(t *testing.T) { } } -/* -+---------A(index)--------+ -| | | -| | | -| | | -| | | -v v v -B(manifest) C(manifest) D (manifest) -| | | -| | | -| | v -+-->E(layer)<-+------->F(layer) -*/ +// +-----------------------------------------------+ +// | | +// | +--------+ | +// | |A(index)| | +// | +---+----+ | +// | | | +// | -+--------------+--------------+- | +// | | | | | +// | +-----v-----+ +-----v-----+ +-----v-----+ | +// | |B(manifest)| |C(manifest)| |D(manifest)| | +// | +--------+--+ ++---------++ +--+--------+ | +// | | | | | | +// | | | | | | +// | v v v v | +// | ++------++ ++------++ | +// | |E(layer)| |F(layer)| | +// | +--------+ +--------+ | +// | | +// +-----------------------------------------------+ func TestMemory_IndexAllAndPredecessors(t *testing.T) { testFetcher := cas.NewMemory() testMemory := NewMemory() From f68a3a3bea22f2f1ba227685cf5a8393f678a598 Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Fri, 13 Oct 2023 06:46:02 +0000 Subject: [PATCH 12/13] refined the predecessor test Signed-off-by: Xiaoxuan Wang --- internal/graph/memory_test.go | 44 ++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/internal/graph/memory_test.go b/internal/graph/memory_test.go index 51d2f450..f9f9e89d 100644 --- a/internal/graph/memory_test.go +++ b/internal/graph/memory_test.go @@ -343,6 +343,7 @@ func TestMemory_IndexAllAndPredecessors(t *testing.T) { } generateManifest := func(layers ...ocispec.Descriptor) ocispec.Descriptor { manifest := ocispec.Manifest{ + Config: ocispec.Descriptor{MediaType: "test config"}, Layers: layers, } manifestJSON, err := json.Marshal(manifest) @@ -526,7 +527,7 @@ func TestMemory_IndexAllAndPredecessors(t *testing.T) { t.Errorf("successors of %s should be empty", "F") } - // check the Predecessors of node C is node A + // check that the Predecessors of node C is node A predsC, err := testMemory.Predecessors(ctx, descC) if err != nil { t.Errorf("testFetcher.Predecessors() error = %v", err) @@ -539,7 +540,7 @@ func TestMemory_IndexAllAndPredecessors(t *testing.T) { t.Errorf("incorrect predecessor result") } - // check the Predecessors of node F are node C and node D + // check that the Predecessors of node F are node C and node D predsF, err := testMemory.Predecessors(ctx, descF) if err != nil { t.Errorf("testFetcher.Predecessors() error = %v", err) @@ -550,7 +551,7 @@ func TestMemory_IndexAllAndPredecessors(t *testing.T) { } for _, pred := range predsF { if !reflect.DeepEqual(pred, descC) && !reflect.DeepEqual(pred, descD) { - t.Errorf("incorrect predecessor result") + t.Errorf("incorrect predecessor results") } } @@ -566,21 +567,46 @@ func TestMemory_IndexAllAndPredecessors(t *testing.T) { t.Errorf("successors of %s should still contain %s", "A", "C") } if _, exists := testMemory.successors[nodeKeyC]; exists { - t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "C") + t.Errorf("testMemory.successors should not contain the entry of %v", "C") + } + if _, exists := testMemory.predecessors[nodeKeyC]; !exists { + t.Errorf("entry %s in predecessors should still exists since it still has at least one predecessor node present", "C") } // remove node A and check the stored information testMemory.Remove(ctx, descA) - if predecessorsD.Contains(nodeKeyA) { - t.Errorf("predecessors of %s should not contain %s", "D", "A") - } - if predecessorsB.Contains(nodeKeyA) { - t.Errorf("predecessors of %s should not contain %s", "B", "A") + if _, exists := testMemory.predecessors[nodeKeyB]; exists { + t.Errorf("entry %s in predecessors should no longer exists", "B") } if _, exists := testMemory.predecessors[nodeKeyC]; exists { t.Errorf("entry %s in predecessors should no longer exists", "C") } + if _, exists := testMemory.predecessors[nodeKeyD]; exists { + t.Errorf("entry %s in predecessors should no longer exists", "D") + } if _, exists := testMemory.successors[nodeKeyA]; exists { t.Errorf("testDeletableMemory.successors should not contain the entry of %v", "A") } + + // check that the Predecessors of node D is empty + predsD, err := testMemory.Predecessors(ctx, descD) + if err != nil { + t.Errorf("testFetcher.Predecessors() error = %v", err) + } + if predsD != nil { + t.Errorf("%s should be nil", "predsD") + } + + // check that the Predecessors of node E is node B + predsE, err := testMemory.Predecessors(ctx, descE) + if err != nil { + t.Errorf("testFetcher.Predecessors() error = %v", err) + } + expectedLength = 1 + if len(predsE) != expectedLength { + t.Errorf("%s should have length %d", "predsE", expectedLength) + } + if !reflect.DeepEqual(predsE[0], descB) { + t.Errorf("incorrect predecessor result") + } } From 012930a8ac28a05859c26a73eb854619b7dbe15c Mon Sep 17 00:00:00 2001 From: Xiaoxuan Wang Date: Mon, 16 Oct 2023 04:42:29 +0000 Subject: [PATCH 13/13] rephrased the doc Signed-off-by: Xiaoxuan Wang --- internal/graph/memory.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/internal/graph/memory.go b/internal/graph/memory.go index 3978436c..bbb57556 100644 --- a/internal/graph/memory.go +++ b/internal/graph/memory.go @@ -82,10 +82,9 @@ func (m *Memory) IndexAll(ctx context.Context, fetcher content.Fetcher, node oci // Predecessors returns the nodes directly pointing to the current node. // Predecessors returns nil without error if the node does not exists in the -// store. -// Like other operations, calling Predecessors() is go-routine safe. However, -// it does not necessarily correspond to any consistent snapshot of the stored -// contents. +// store. Like other operations, calling Predecessors() is go-routine safe. +// However, it does not necessarily correspond to any consistent snapshot of +// the stored contents. func (m *Memory) Predecessors(_ context.Context, node ocispec.Descriptor) ([]ocispec.Descriptor, error) { m.lock.RLock() defer m.lock.RUnlock() @@ -112,11 +111,9 @@ func (m *Memory) Remove(ctx context.Context, node ocispec.Descriptor) error { for successorKey := range m.successors[nodeKey] { predecessorEntry := m.predecessors[successorKey] predecessorEntry.Delete(nodeKey) - // remove the successorKey entry from m. predecessors, if it's an empty set. - // To it another way, if all its predecessors are already deleted, then we - // delete the entry in m.predecessors. In any of its predecessors still exists, - // the entry is NOT deleted. The presence of the predecessors entry depends on - // the status of all predecessors of the node. + + // if none of the predecessors of the node still exists, we remove the + // predecessors entry. Otherwise, we do not remove the entry. if len(predecessorEntry) == 0 { delete(m.predecessors, successorKey) }