diff --git a/content/oci/deletableOci.go b/content/oci/deletableOci.go index d50a2f54..d6641ea5 100644 --- a/content/oci/deletableOci.go +++ b/content/oci/deletableOci.go @@ -132,7 +132,7 @@ func (ds *DeletableStore) Delete(ctx context.Context, target ocispec.Descriptor) ds.tagResolver.Delete(reference) } } - if err := ds.graph.RemoveFromIndex(ctx, target); err != nil { + if err := ds.graph.Remove(ctx, target); err != nil { return err } if ds.AutoSaveIndex { diff --git a/internal/graph/memoryWithDelete.go b/internal/graph/memoryWithDelete.go index b4413bb2..911ae7d3 100644 --- a/internal/graph/memoryWithDelete.go +++ b/internal/graph/memoryWithDelete.go @@ -30,9 +30,10 @@ import ( // MemoryWithDelete is a MemoryWithDelete based PredecessorFinder. type MemoryWithDelete struct { + indexed sync.Map // map[descriptor.Descriptor]any predecessors sync.Map // map[descriptor.Descriptor]map[descriptor.Descriptor]ocispec.Descriptor successors sync.Map // map[descriptor.Descriptor]map[descriptor.Descriptor]ocispec.Descriptor - indexed sync.Map // map[descriptor.Descriptor]any + lock sync.Mutex } // NewMemoryWithDelete creates a new MemoryWithDelete PredecessorFinder. @@ -117,8 +118,10 @@ func (m *MemoryWithDelete) Predecessors(_ context.Context, node ocispec.Descript return res, nil } -// RemoveFromIndex removes the node from its predecessors and successors. -func (m *MemoryWithDelete) RemoveFromIndex(ctx context.Context, node ocispec.Descriptor) error { +// Remove removes the node from its predecessors and successors. +func (m *MemoryWithDelete) 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 value, _ := m.successors.Load(nodeKey) @@ -129,7 +132,7 @@ func (m *MemoryWithDelete) RemoveFromIndex(ctx context.Context, node ocispec.Des predecessors.Delete(nodeKey) return true }) - m.removeFromMemoryWithDelete(ctx, node) + m.removeEntriesFromMaps(ctx, node) return nil } @@ -137,7 +140,9 @@ func (m *MemoryWithDelete) RemoveFromIndex(ctx context.Context, node ocispec.Des // There is no data consistency issue as long as deletion is not implemented // for the underlying storage. func (m *MemoryWithDelete) index(ctx context.Context, node ocispec.Descriptor, successors []ocispec.Descriptor) { - m.indexIntoMemoryWithDelete(ctx, node) + m.lock.Lock() + defer m.lock.Unlock() + m.createEntriesInMaps(ctx, node) if len(successors) == 0 { return } @@ -155,14 +160,13 @@ func (m *MemoryWithDelete) index(ctx context.Context, node ocispec.Descriptor, s } } -func (m *MemoryWithDelete) indexIntoMemoryWithDelete(ctx context.Context, node ocispec.Descriptor) { +func (m *MemoryWithDelete) createEntriesInMaps(ctx context.Context, node ocispec.Descriptor) { key := descriptor.FromOCI(node) m.predecessors.LoadOrStore(key, &sync.Map{}) m.successors.LoadOrStore(key, &sync.Map{}) - m.indexed.LoadOrStore(key, &sync.Map{}) } -func (m *MemoryWithDelete) removeFromMemoryWithDelete(ctx context.Context, node ocispec.Descriptor) { +func (m *MemoryWithDelete) removeEntriesFromMaps(ctx context.Context, node ocispec.Descriptor) { key := descriptor.FromOCI(node) m.predecessors.Delete(key) m.successors.Delete(key) diff --git a/internal/graph/memoryWithDelete_test.go b/internal/graph/memoryWithDelete_test.go index 1097f746..5195c58c 100644 --- a/internal/graph/memoryWithDelete_test.go +++ b/internal/graph/memoryWithDelete_test.go @@ -35,11 +35,13 @@ var ( DKey = descriptor.FromOCI(D) ) -// 条件： -// 1. 只要node被index（作为函数的第二个输入），node就在predecessors，successors，indexed中有entry -// 2. 只要node被Removed from index，node就在predecessors，successors，indexed中都没有entry +// 当前条件（后续可能改动）： +// 1. 只要node被index（作为函数的第二个输入），node就在predecessors，successors中都有entry +// 2. 只要node被Removed from index，node就在predecessors，successors中都没有entry // 3. 只有node被index（作为函数的第二个输入），successors中才有其entry // 4. successors中的内容与node中的内容一致，只要entry还在，其内容就不会改变 +// 5. 变量indexed的作用和行为和原始版本一样，没有加以变动 +// 6. 删除一个node的时候，indexed里面的entry也会删除。 // GC情况： // predecessors中可能有空的map，如下图中B被删除后，C还在predecessors中有entry但内容为空 @@ -51,14 +53,14 @@ A--->B--->C | | +---------+ */ -func TestMemoryWithDelete_index(t *testing.T) { +func TestMemoryWithDelete_indexAndDelete(t *testing.T) { ctx := context.Background() testMemoryWithDelete := NewMemoryWithDelete() // test 1: index "A -> B" testMemoryWithDelete.index(ctx, A, []ocispec.Descriptor{B}) - // check the MemoryWithDelete: A exists in testMemoryWithDelete.predecessors, successors and indexed, + // check the MemoryWithDelete: A exists in testMemoryWithDelete.predecessors and successors // B ONLY exists in predecessors _, exists := testMemoryWithDelete.predecessors.Load(AKey) if !exists { @@ -68,10 +70,6 @@ func TestMemoryWithDelete_index(t *testing.T) { if !exists { t.Errorf("could not find the entry of %v in successors", A) } - _, exists = testMemoryWithDelete.indexed.Load(AKey) - if !exists { - t.Errorf("could not find the entry of %v in indexed", A) - } _, exists = testMemoryWithDelete.predecessors.Load(BKey) if !exists { t.Errorf("could not find the entry of %v in predecessors", B) @@ -80,10 +78,6 @@ func TestMemoryWithDelete_index(t *testing.T) { if exists { t.Errorf("%v should not exist in successors", B) } - _, exists = testMemoryWithDelete.indexed.Load(BKey) - if exists { - t.Errorf("%v should not exist in indexed", B) - } // predecessors[B] contains A, successors[A] contains B value, _ := testMemoryWithDelete.predecessors.Load(BKey) @@ -112,10 +106,6 @@ func TestMemoryWithDelete_index(t *testing.T) { if !exists { t.Errorf("could not find the entry of %v in successors", B) } - _, exists = testMemoryWithDelete.indexed.Load(BKey) - if !exists { - t.Errorf("could not find the entry of %v in indexed", B) - } _, exists = testMemoryWithDelete.predecessors.Load(CKey) if !exists { t.Errorf("could not find the entry of %v in predecessors", C) @@ -124,10 +114,6 @@ func TestMemoryWithDelete_index(t *testing.T) { if exists { t.Errorf("%v should not exist in successors", C) } - _, exists = testMemoryWithDelete.indexed.Load(CKey) - if exists { - t.Errorf("%v should not exist in indexed", C) - } _, exists = testMemoryWithDelete.predecessors.Load(DKey) if !exists { t.Errorf("could not find the entry of %v in predecessors", D) @@ -136,10 +122,6 @@ func TestMemoryWithDelete_index(t *testing.T) { if exists { t.Errorf("%v should not exist in successors", D) } - _, exists = testMemoryWithDelete.indexed.Load(DKey) - if exists { - t.Errorf("%v should not exist in indexed", D) - } // predecessors[C] contains B, predecessors[D] contains B, // successors[B] contains C and D value, _ = testMemoryWithDelete.predecessors.Load(CKey) @@ -191,26 +173,18 @@ func TestMemoryWithDelete_index(t *testing.T) { } // check the MemoryWithDelete: C and D have not been indexed, so C, D should not - // exist in indexed and successors + // exist in successors _, exists = testMemoryWithDelete.successors.Load(CKey) if exists { t.Errorf("%v should not exist in successors", C) } - _, exists = testMemoryWithDelete.indexed.Load(CKey) - if exists { - t.Errorf("%v should not exist in indexed", C) - } _, exists = testMemoryWithDelete.successors.Load(DKey) if exists { t.Errorf("%v should not exist in successors", D) } - _, exists = testMemoryWithDelete.indexed.Load(DKey) - if exists { - t.Errorf("%v should not exist in indexed", D) - } // test 4: delete B - err := testMemoryWithDelete.RemoveFromIndex(ctx, B) + err := testMemoryWithDelete.Remove(ctx, B) if err != nil { t.Errorf("got error when removing %v from index: %v", B, err) } @@ -224,10 +198,6 @@ func TestMemoryWithDelete_index(t *testing.T) { if exists { t.Errorf("%v should not exist in successors", B) } - _, exists = testMemoryWithDelete.indexed.Load(BKey) - if exists { - t.Errorf("%v should not exist in indexed", B) - } // B should STILL exist in successors[A] value, _ = testMemoryWithDelete.successors.Load(AKey)