Skip to content

Commit

Permalink
fix of bug with removed keys in processDeque (#30)
Browse files Browse the repository at this point in the history
* removed potential bug with removed keys in processDeque

* adjust ProcessDeque test in order to catch bug with removedkv
  • Loading branch information
sivukhin authored Dec 5, 2023
1 parent c903895 commit c3506a9
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
8 changes: 3 additions & 5 deletions internal/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import (
"sync/atomic"
"time"

"github.com/Yiling-J/theine-go/internal/bf"
"github.com/gammazero/deque"
"github.com/zeebo/xxh3"

"github.com/Yiling-J/theine-go/internal/bf"
)

const (
Expand Down Expand Up @@ -369,7 +370,6 @@ func (s *Store[K, V]) processDeque(shard *Shard[K, V]) {
removedkv := make([]dequeKV[K, V], 0, 2)
// expired
expiredkv := make([]dequeKV[K, V], 0, 2)
// expired
for shard.qlen > int(shard.qsize) {
evicted := shard.deque.PopBack()
evicted.deque = false
Expand All @@ -394,9 +394,7 @@ func (s *Store[K, V]) processDeque(shard *Shard[K, V]) {
deleted := shard.delete(evicted)
// double check because entry maybe removed already by Delete API
if deleted {
removedkv = append(
expiredkv, s.kvBuilder(evicted),
)
removedkv = append(removedkv, s.kvBuilder(evicted))
s.postDelete(evicted)
}
}
Expand Down
14 changes: 14 additions & 0 deletions internal/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,20 @@ func TestProcessDeque(t *testing.T) {
keys = append(keys, e.key)
}
require.Equal(t, []int{3, 4, 123}, keys)
require.Equal(t, 0, len(evicted))

// test evicted callback, cost less than threshold will be evicted immediately
store.policy.threshold.Store(100)
for i := 10; i < 15; i++ {
entry := &Entry[int, int]{key: i}
entry.cost.Store(1)
store.shards[index].deque.PushFront(entry)
store.shards[index].qlen += 1
store.shards[index].hashmap[i] = entry
}
store.shards[index].mu.Lock()
store.processDeque(store.shards[index])
require.Equal(t, 5, len(evicted))
}

func TestRemoveDeque(t *testing.T) {
Expand Down

0 comments on commit c3506a9

Please sign in to comment.