move treeNodeCache with other cache implementations onet-273

cache.go

@@ -82,8 +82,9 @@ func (c *cacheTTL) get(key uuid.UUID) interface{} {
 	c.Lock()
 	defer c.Unlock()
 
-	entry := c.entries[key]
-	if entry != nil {
+	entry, ok := c.entries[key]
+	if ok && time.Now().Before(entry.expiration) {
+		entry.expiration = time.Now().Add(c.entryExpiration)
 		return entry.item
 	}
 
@@ -102,13 +103,13 @@ func newTreeCache(interval, expiration time.Duration) *treeCacheTTL {
 
 // Set stores the given tree in the cache
 func (c *treeCacheTTL) Set(tree *Tree) {
-	c.cacheTTL.set(uuid.UUID(tree.ID), tree)
+	c.set(uuid.UUID(tree.ID), tree)
 }
 
 // Get retrieves the tree with the given ID if it exists
 // or returns nil
 func (c *treeCacheTTL) Get(id TreeID) *Tree {
-	tree := c.cacheTTL.get(uuid.UUID(id))
+	tree := c.get(uuid.UUID(id))
 	if tree != nil {
 		return tree.(*Tree)
 	}
@@ -133,13 +134,13 @@ func newRosterCache(interval, expiration time.Duration) *rosterCacheTTL {
 
 // Set stores the roster in the cache
 func (c *rosterCacheTTL) Set(roster *Roster) {
-	c.cacheTTL.set(uuid.UUID(roster.ID), roster)
+	c.set(uuid.UUID(roster.ID), roster)
 }
 
 // Get retrieves the Roster with the given ID if it exists
 // or it returns nil
 func (c *rosterCacheTTL) Get(id RosterID) *Roster {
-	roster := c.cacheTTL.get(uuid.UUID(id))
+	roster := c.get(uuid.UUID(id))
 	if roster != nil {
 		return roster.(*Roster)
 	}
@@ -151,3 +152,64 @@ func (c *rosterCacheTTL) Get(id RosterID) *Roster {
 func (c *rosterCacheTTL) GetFromToken(token *Token) *Roster {
 	return c.Get(token.RosterID)
 }
+
+// treeNodeCacheTTL is a cache that maps from token to treeNode. Since
+// the mapping is not 1-1 (many Tokens can point to one TreeNode, but
+// one token leads to one TreeNode), we have to do certain lookup, but
+// that's better than searching the tree each time.
+type treeNodeCacheTTL struct {
+	*cacheTTL
+}
+
+func newTreeNodeCache(interval, expiration time.Duration) *treeNodeCacheTTL {
+	return &treeNodeCacheTTL{
+		cacheTTL: newCacheTTL(interval, expiration),
+	}
+}
+
+func (c *treeNodeCacheTTL) Set(tree *Tree, treeNode *TreeNode) {
+	c.Lock()
+	ce, ok := c.entries[uuid.UUID(tree.ID)]
+	if !ok {
+		ce = &cacheTTLEntry{
+			item:       make(map[TreeNodeID]*TreeNode),
+			expiration: time.Now().Add(c.entryExpiration),
+		}
+	}
+	treeNodeMap := ce.item.(map[TreeNodeID]*TreeNode)
+	// add treenode
+	treeNodeMap[treeNode.ID] = treeNode
+	// add parent if not root
+	if treeNode.Parent != nil {
+		treeNodeMap[treeNode.Parent.ID] = treeNode.Parent
+	}
+	// add children
+	for _, c := range treeNode.Children {
+		treeNodeMap[c.ID] = c
+	}
+	// add cache
+	c.entries[uuid.UUID(tree.ID)] = ce
+	c.Unlock()
+}
+
+func (c *treeNodeCacheTTL) GetFromToken(tok *Token) *TreeNode {
+	c.Lock()
+	defer c.Unlock()
+	if tok == nil {
+		return nil
+	}
+	ce, ok := c.entries[uuid.UUID(tok.TreeID)]
+	if !ok || time.Now().After(ce.expiration) {
+		// no tree cached for this token
+		return nil
+	}
+	ce.expiration = time.Now().Add(c.entryExpiration)
+
+	treeNodeMap := ce.item.(map[TreeNodeID]*TreeNode)
+	tn, ok := treeNodeMap[tok.TreeNodeID]
+	if !ok {
+		// no treeNode cached for this token
+		return nil
+	}
+	return tn
+}

cache_test.go

@@ -34,6 +34,8 @@ func TestTreeCache(t *testing.T) {
 			cache.Set(tree)
 			require.NotNil(t, cache.Get(tree.ID))
 			time.Sleep(10 * time.Millisecond)
+			// test if it persists with the Get request
+			cache.Get(trees[0].ID)
 		}
 		wg.Done()
 	}
@@ -45,11 +47,14 @@ func TestTreeCache(t *testing.T) {
 	wg.Wait()
 
 	token := &Token{}
-	for _, tree := range trees[nbrOfItems-5:] {
+	for _, tree := range trees[nbrOfItems-8:] {
 		token.TreeID = tree.ID
 		require.Equal(t, tree, cache.GetFromToken(token))
 	}
 
+	require.Equal(t, trees[0], cache.Get(trees[0].ID))
+	require.Nil(t, cache.Get(trees[1].ID))
+
 	time.Sleep(125 * time.Millisecond)
 
 	for _, tree := range trees {
@@ -59,7 +64,7 @@ func TestTreeCache(t *testing.T) {
 }
 
 func TestRosterCache(t *testing.T) {
-	cache := newRosterCache(25*time.Millisecond, 100*time.Millisecond)
+	cache := newRosterCache(1*time.Minute, 50*time.Millisecond)
 	defer cache.stop()
 
 	r := &Roster{}
@@ -71,6 +76,40 @@ func TestRosterCache(t *testing.T) {
 	token := &Token{}
 	token.RosterID = r.ID
 	require.Equal(t, r, cache.GetFromToken(token))
+
+	// test that get ignore expired item
+	time.Sleep(100 * time.Millisecond)
+	require.Nil(t, cache.GetFromToken(token))
+}
+
+func generateID() uuid.UUID {
+	id, _ := uuid.NewV1()
+	return id
+}
+
+func TestTreeNodeCache(t *testing.T) {
+	cache := newTreeNodeCache(1*time.Minute, 50*time.Millisecond)
+	defer cache.stop()
+
+	tree := &Tree{ID: TreeID(generateID())}
+	tn1 := &TreeNode{ID: TreeNodeID(generateID())}
+	tn2 := &TreeNode{ID: TreeNodeID(generateID())}
+
+	cache.Set(tree, tn1)
+	cache.Set(tree, tn2)
+
+	tok := &Token{TreeID: tree.ID, TreeNodeID: tn1.ID}
+	require.Equal(t, tn1, cache.GetFromToken(tok))
+	tok.TreeNodeID = tn2.ID
+	require.Equal(t, tn2, cache.GetFromToken(tok))
+	tok.TreeNodeID = TreeNodeID(generateID())
+	require.Nil(t, cache.GetFromToken(tok))
+
+	require.Nil(t, cache.GetFromToken(&Token{}))
+
+	// test that get ignore expired item
+	time.Sleep(100 * time.Millisecond)
+	require.Nil(t, cache.GetFromToken(tok))
 }
 
 func TestExpirationAndCleaning(t *testing.T) {

overlay.go

@@ -22,7 +22,7 @@ type Overlay struct {
 	treeCache   *treeCacheTTL
 	rosterCache *rosterCacheTTL
 	// cache for relating token(~Node) to TreeNode
-	cache *treeNodeCache
+	treeNodeCache *treeNodeCacheTTL
 
 	// TreeNodeInstance part
 	instances         map[TokenID]*TreeNodeInstance
@@ -58,7 +58,7 @@ func NewOverlay(c *Server) *Overlay {
 		server:             c,
 		treeCache:          newTreeCache(cleanInterval, expirationTime),
 		rosterCache:        newRosterCache(cleanInterval, expirationTime),
-		cache:              newTreeNodeCache(),
+		treeNodeCache:      newTreeNodeCache(cleanInterval, expirationTime),
 		instances:          make(map[TokenID]*TreeNodeInstance),
 		instancesInfo:      make(map[TokenID]bool),
 		protocolInstances:  make(map[TokenID]ProtocolInstance),
@@ -79,7 +79,7 @@ func NewOverlay(c *Server) *Overlay {
 
 // stop stops goroutines associated with this overlay.
 func (o *Overlay) stop() {
-	o.cache.stop()
+	o.treeNodeCache.stop()
 	o.treeCache.stop()
 	o.rosterCache.stop()
 }
@@ -292,7 +292,7 @@ func (o *Overlay) TreeNodeFromToken(t *Token) (*TreeNode, error) {
 		return nil, errors.New("didn't find tree-node: No token given")
 	}
 	// First, check the cache
-	if tn := o.cache.GetFromToken(t); tn != nil {
+	if tn := o.treeNodeCache.GetFromToken(t); tn != nil {
 		return tn, nil
 	}
 	// If cache has not, then search the tree
@@ -305,7 +305,7 @@ func (o *Overlay) TreeNodeFromToken(t *Token) (*TreeNode, error) {
 		return nil, errors.New("didn't find treenode")
 	}
 	// Since we found treeNode, cache it.
-	o.cache.Set(tree, tn)
+	o.treeNodeCache.Set(tree, tn)
 	return tn, nil
 }
 
@@ -665,112 +665,6 @@ type pendingMsg struct {
 	MessageProxy
 }
 
-// treeNodeCache is a cache that maps from token to treeNode. Since
-// the mapping is not 1-1 (many Tokens can point to one TreeNode, but
-// one token leads to one TreeNode), we have to do certain lookup, but
-// that's better than searching the tree each time.
-type treeNodeCache struct {
-	Entries  map[TreeID]*cacheEntry
-	stopCh   chan (struct{})
-	stopOnce sync.Once
-	sync.Mutex
-}
-
-type cacheEntry struct {
-	treeNodeMap map[TreeNodeID]*TreeNode
-	expiration  time.Time
-}
-
-var cacheTime = 5 * time.Minute
-var cleanEvery = 1 * time.Minute
-
-func newTreeNodeCache() *treeNodeCache {
-	tnc := &treeNodeCache{
-		Entries: make(map[TreeID]*cacheEntry),
-		stopCh:  make(chan struct{}),
-	}
-	go tnc.cleaner()
-	return tnc
-}
-
-func (tnc *treeNodeCache) stop() {
-	tnc.stopOnce.Do(func() {
-		close(tnc.stopCh)
-	})
-}
-
-func (tnc *treeNodeCache) cleaner() {
-	for {
-		select {
-		case <-time.After(cleanEvery):
-			tnc.clean()
-		case <-tnc.stopCh:
-			return
-		}
-	}
-}
-
-func (tnc *treeNodeCache) clean() {
-	tnc.Lock()
-	now := time.Now()
-	for k, e := range tnc.Entries {
-		if now.After(e.expiration) {
-			delete(tnc.Entries, k)
-		}
-	}
-	tnc.Unlock()
-}
-
-// Set sets an entry in the cache. It will also cache the parent and
-// children of the treenode since that's most likely what we are going
-// to query.
-func (tnc *treeNodeCache) Set(tree *Tree, treeNode *TreeNode) {
-	tnc.Lock()
-	ce, ok := tnc.Entries[tree.ID]
-	if !ok {
-		ce = &cacheEntry{
-			treeNodeMap: make(map[TreeNodeID]*TreeNode),
-			expiration:  time.Now().Add(cacheTime),
-		}
-	}
-	// add treenode
-	ce.treeNodeMap[treeNode.ID] = treeNode
-	// add parent if not root
-	if treeNode.Parent != nil {
-		ce.treeNodeMap[treeNode.Parent.ID] = treeNode.Parent
-	}
-	// add children
-	for _, c := range treeNode.Children {
-		ce.treeNodeMap[c.ID] = c
-	}
-	// add cache
-	tnc.Entries[tree.ID] = ce
-	tnc.Unlock()
-}
-
-// GetFromToken returns the TreeNode that the token is pointing at, or
-// nil if there is none for this token.
-func (tnc *treeNodeCache) GetFromToken(tok *Token) *TreeNode {
-	tnc.Lock()
-	defer tnc.Unlock()
-	if tok == nil {
-		return nil
-	}
-	ce, ok := tnc.Entries[tok.TreeID]
-	if !ok || time.Now().After(ce.expiration) {
-		// no tree cached for this token
-		return nil
-	}
-	ce.expiration = time.Now().Add(cacheTime)
-
-	tn, ok := ce.treeNodeMap[tok.TreeNodeID]
-	if !ok {
-		// no treeNode cached for this token
-		return nil
-	}
-	return tn
-}
-
 // defaultProtoIO implements the ProtocoIO interface but using the "regular/old"
 // wire format protocol,i.e. it wraps a message into a ProtocolMessage
 type defaultProtoIO struct {