Skip to content

Commit

Permalink
changed predecessors to regular map
Browse files Browse the repository at this point in the history 
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
  • Loading branch information
@wangxiaoxuan273
wangxiaoxuan273 committed Sep 7, 2023
1 parent 49cfe2b commit e7f7d34
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 50 deletions.
33 changes: 16 additions & 17 deletions internal/graph/memoryWithDelete.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,16 @@ import (
// MemoryWithDelete is a MemoryWithDelete based PredecessorFinder.
type MemoryWithDelete struct {
indexed sync.Map // map[descriptor.Descriptor]any, this variable is only used by IndexAll
predecessors sync.Map // map[descriptor.Descriptor]map[descriptor.Descriptor]ocispec.Descriptor
predecessors map[descriptor.Descriptor]map[descriptor.Descriptor]ocispec.Descriptor
successors map[descriptor.Descriptor]map[descriptor.Descriptor]ocispec.Descriptor
lock sync.Mutex
}

// NewMemoryWithDelete creates a new MemoryWithDelete PredecessorFinder.
func NewMemoryWithDelete() *MemoryWithDelete {
return &MemoryWithDelete{
successors: make(map[descriptor.Descriptor]map[descriptor.Descriptor]ocispec.Descriptor),
predecessors: make(map[descriptor.Descriptor]map[descriptor.Descriptor]ocispec.Descriptor),
successors: make(map[descriptor.Descriptor]map[descriptor.Descriptor]ocispec.Descriptor),
}
}

Expand Down Expand Up @@ -106,17 +107,14 @@ func (m *MemoryWithDelete) IndexAll(ctx context.Context, fetcher content.Fetcher
// contents.
func (m *MemoryWithDelete) Predecessors(_ context.Context, node ocispec.Descriptor) ([]ocispec.Descriptor, error) {
key := descriptor.FromOCI(node)
value, exists := m.predecessors.Load(key)
_, 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 _, v := range m.predecessors[key] {
res = append(res, v)
}
return res, nil
}

Expand All @@ -126,10 +124,8 @@ func (m *MemoryWithDelete) Remove(ctx context.Context, node ocispec.Descriptor)
defer m.lock.Unlock()
nodeKey := descriptor.FromOCI(node)
// remove the node from its successors' predecessor list
for successorKey, _ := range m.successors[nodeKey] {
value, _ := m.predecessors.Load(successorKey)
predecessors := value.(*sync.Map)
predecessors.Delete(nodeKey)
for successorKey := range m.successors[nodeKey] {
delete(m.predecessors[successorKey], nodeKey)
}
m.removeEntriesFromMaps(ctx, node)
return nil
Expand All @@ -149,17 +145,20 @@ func (m *MemoryWithDelete) index(ctx context.Context, node ocispec.Descriptor, s
for _, successor := range successors {
successorKey := descriptor.FromOCI(successor)
// store in m.predecessors, MemoryWithDelete.predecessors[successorKey].Store(node)
pred, _ := m.predecessors.LoadOrStore(successorKey, &sync.Map{})
predecessorsMap := pred.(*sync.Map)
predecessorsMap.Store(predecessorKey, node)
if _, exists := m.predecessors[successorKey]; !exists {
m.predecessors[successorKey] = make(map[descriptor.Descriptor]ocispec.Descriptor)
}
m.predecessors[successorKey][predecessorKey] = node
// store in m.successors, MemoryWithDelete.successors[predecessorKey].Store(successor)
m.successors[predecessorKey][successorKey] = successor
}
}

func (m *MemoryWithDelete) createEntriesInMaps(ctx context.Context, node ocispec.Descriptor) {
key := descriptor.FromOCI(node)
m.predecessors.LoadOrStore(key, &sync.Map{})
if _, hasEntry := m.predecessors[key]; !hasEntry {
m.predecessors[key] = make(map[descriptor.Descriptor]ocispec.Descriptor)
}
if _, hasEntry := m.successors[key]; !hasEntry {
m.successors[key] = make(map[descriptor.Descriptor]ocispec.Descriptor)
}
Expand Down
86 changes: 53 additions & 33 deletions internal/graph/memoryWithDelete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package graph

import (
"context"
"sync"
"testing"

ocispec "github.com/opencontainers/image-spec/specs-go/v1"
Expand All @@ -35,17 +34,27 @@ var (
DKey = descriptor.FromOCI(D)
)

// TODO:
// IndexAll和Delete，会不会出现race condition？
// IndexAll和Index同时调用，会不会出现race condition？

// 当前条件（后续可能改动）：
// 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也会删除。
// * 变量indexed的作用和行为和原始版本一样，没有加以变动
// * 只要node被index（作为函数的第二个输入），node就在predecessors，successors中都有entry
// * 只有node被index（作为函数的第二个输入），successors中才有其entry
// * successors中的内容与node中的内容一致，只要entry还在，其内容就不会改变
// * 删除一个node的时候，indexed和successors里面的entry也会删除。predecessors中的entry不会被删除

// GC情况：
// predecessors中可能有空的map，如下图中B被删除后，C还在predecessors中有entry但内容为空
/*
test 1: index "A -> B"
test 2: index "B -> C, B -> D"
test 3: index "A -> D"
test 4: delete B
test 5: delete A
A--->B--->C
| |
| +--->D
Expand All @@ -62,15 +71,15 @@ func TestMemoryWithDelete_indexAndDelete(t *testing.T) {

// check the memory: A exists in predecessors and successors
// B ONLY exists in predecessors
_, exists := testMemoryWithDelete.predecessors.Load(AKey)
_, exists := testMemoryWithDelete.predecessors[AKey]
if !exists {
t.Errorf("could not find the entry of %v in predecessors", A)
}
_, exists = testMemoryWithDelete.successors[AKey]
if !exists {
t.Errorf("could not find the entry of %v in successors", A)
}
_, exists = testMemoryWithDelete.predecessors.Load(BKey)
_, exists = testMemoryWithDelete.predecessors[BKey]
if !exists {
t.Errorf("could not find the entry of %v in predecessors", B)
}
Expand All @@ -80,9 +89,7 @@ func TestMemoryWithDelete_indexAndDelete(t *testing.T) {
}

// predecessors[B] contains A, successors[A] contains B
value, _ := testMemoryWithDelete.predecessors.Load(BKey)
BPreds := value.(*sync.Map)
_, exists = BPreds.Load(AKey)
_, exists = testMemoryWithDelete.predecessors[BKey][AKey]
if !exists {
t.Errorf("could not find %v in predecessors of %v", A, B)
}
Expand All @@ -96,23 +103,23 @@ func TestMemoryWithDelete_indexAndDelete(t *testing.T) {

// check the memory: B exists in predecessors and successors,
// C, D ONLY exists in predecessors
_, exists = testMemoryWithDelete.predecessors.Load(BKey)
_, exists = testMemoryWithDelete.predecessors[BKey]
if !exists {
t.Errorf("could not find the entry of %v in predecessors", B)
}
_, exists = testMemoryWithDelete.successors[BKey]
if !exists {
t.Errorf("could not find the entry of %v in successors", B)
}
_, exists = testMemoryWithDelete.predecessors.Load(CKey)
_, exists = testMemoryWithDelete.predecessors[CKey]
if !exists {
t.Errorf("could not find the entry of %v in predecessors", C)
}
_, exists = testMemoryWithDelete.successors[CKey]
if exists {
t.Errorf("%v should not exist in successors", C)
}
_, exists = testMemoryWithDelete.predecessors.Load(DKey)
_, exists = testMemoryWithDelete.predecessors[DKey]
if !exists {
t.Errorf("could not find the entry of %v in predecessors", D)
}
Expand All @@ -122,15 +129,11 @@ func TestMemoryWithDelete_indexAndDelete(t *testing.T) {
}
// predecessors[C] contains B, predecessors[D] contains B,
// successors[B] contains C and D
value, _ = testMemoryWithDelete.predecessors.Load(CKey)
CPreds := value.(*sync.Map)
_, exists = CPreds.Load(BKey)
_, exists = testMemoryWithDelete.predecessors[CKey][BKey]
if !exists {
t.Errorf("could not find %v in predecessors of %v", B, C)
}
value, _ = testMemoryWithDelete.predecessors.Load(DKey)
DPreds := value.(*sync.Map)
_, exists = DPreds.Load(BKey)
_, exists = testMemoryWithDelete.predecessors[DKey][BKey]
if !exists {
t.Errorf("could not find %v in predecessors of %v", B, D)
}
Expand All @@ -147,13 +150,11 @@ func TestMemoryWithDelete_indexAndDelete(t *testing.T) {
testMemoryWithDelete.index(ctx, A, []ocispec.Descriptor{D})

// predecessors[D] contains A and B, successors[A] contains B and D
value, _ = testMemoryWithDelete.predecessors.Load(DKey)
DPreds = value.(*sync.Map)
_, exists = DPreds.Load(AKey)
_, exists = testMemoryWithDelete.predecessors[DKey][AKey]
if !exists {
t.Errorf("could not find %v in predecessors of %v", A, D)
}
_, exists = DPreds.Load(BKey)
_, exists = testMemoryWithDelete.predecessors[DKey][BKey]
if !exists {
t.Errorf("could not find %v in predecessors of %v", B, D)
}
Expand Down Expand Up @@ -185,7 +186,7 @@ func TestMemoryWithDelete_indexAndDelete(t *testing.T) {

// check the memory: B should NOT exist in successors, should still exist
// in predecessors
_, exists = testMemoryWithDelete.predecessors.Load(BKey)
_, exists = testMemoryWithDelete.predecessors[BKey]
if !exists {
t.Errorf("%v should exist in predecessors", B)
}
Expand All @@ -201,22 +202,41 @@ func TestMemoryWithDelete_indexAndDelete(t *testing.T) {
}

// B should NOT exist in predecessors[C], predecessors[D]
value, _ = testMemoryWithDelete.predecessors.Load(CKey)
CPreds = value.(*sync.Map)
_, exists = CPreds.Load(BKey)
_, exists = testMemoryWithDelete.predecessors[CKey][BKey]
if exists {
t.Errorf("should not find %v in predecessors of %v", B, C)
}
value, _ = testMemoryWithDelete.predecessors.Load(DKey)
DPreds = value.(*sync.Map)
_, exists = DPreds.Load(BKey)
_, exists = testMemoryWithDelete.predecessors[DKey][BKey]
if exists {
t.Errorf("should not find %v in predecessors of %v", B, D)
}

// A should STILL exist in predecessors[D]
_, exists = DPreds.Load(AKey)
_, exists = testMemoryWithDelete.predecessors[DKey][AKey]
if !exists {
t.Errorf("could not find %v in predecessors of %v", A, D)
}

// test 5: delete A
err = testMemoryWithDelete.Remove(ctx, A)
if err != nil {
t.Errorf("got error when removing %v from index: %v", A, err)
}

// check the memory: A should NOT exist in successors, should still exist
// in predecessors
_, exists = testMemoryWithDelete.predecessors[AKey]
if !exists {
t.Errorf("%v should exist in predecessors", A)
}
_, exists = testMemoryWithDelete.successors[AKey]
if exists {
t.Errorf("%v should not exist in successors", A)
}

// A should NOT exist in predecessors[D]
_, exists = testMemoryWithDelete.predecessors[DKey][AKey]
if exists {
t.Errorf("should not find %v in predecessors of %v", A, D)
}
}

0 comments on commit e7f7d34

Please sign in to comment.