added lock, need to change sync map to regular map

Signed-off-by: Xiaoxuan Wang <[email protected]>
oras-project · Sep 7, 2023 · 8e946ab · 8e946ab
1 parent 7731e12
commit 8e946ab
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 48 deletions.
diff --git 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
@@ -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,15 +132,17 @@ 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
 }
 
 // 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 *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
@@ -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)