-
Notifications
You must be signed in to change notification settings - Fork 120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug Report: v3.0.0 gc may failed #76
Comments
As documented in #76, an entry which is both GC'd and deleted (either via a delete or an update) will result in the internal link list having a nil tail (because removing the same node multiple times from the linked list does that). doDelete was already aware of "invalid" nodes (where item.node == nil), so the solution seems to be as simple as setting item.node = nil during GC.
thanks for the detailed report, it really helped narrow it down. I added a slightly modified version of your test :) |
I think there is still a problem here. If a node is in both the
If you keep this test case running, it will fail func Test_CachePrune(t *testing.T) {
maxSize := int64(500)
cache := New(Configure[string]().MaxSize(maxSize).ItemsToPrune(50))
epoch := 0
for {
epoch += 1
expired := make([]string, 0)
for i := 0; i < 50; i += 1 {
key := strconv.FormatInt(rand.Int63n(maxSize*20), 10)
item := cache.Get(key)
if item == nil || item.TTL() > 1*time.Minute {
expired = append(expired, key)
}
}
for _, key := range expired {
cache.Set(key, key, 5*time.Minute)
}
if epoch%500 == 0 {
fmt.Printf("size=%d, dropped=%d\n", cache.GetSize(), cache.GetDropped())
}
if cache.GetSize() > 5000 {
fmt.Printf("size=%d, dropped=%d\n", cache.GetSize(), cache.GetDropped())
t.Fail()
return
}
}
} I think the fix is let list.node = nil (and maybe item.promotions = -2) after func (c *Cache[T]) doDelete(item *Item[T]) {
if item.node == nil {
item.promotions = -2
} else {
c.size -= item.size
if c.onDelete != nil {
c.onDelete(item)
}
c.list.Remove(item.node)
item.node = nil // fix
item.promotions = -2 // fix
}
} |
FYI, not sure I'll be able to look into this until early next week, sorry. |
Also, item.promotions doesn't need to be loaded/stored using atomic. Once upon a time it did. Cache was updated long ago to not use atomic operations on it, but LayeredCache wasn't. They are both consistent now (they don't use atomic operations). Fixes: #76
I think this commit solves the issue, could you release a new tag? thanks. |
sure, done |
As documented in karlseguin/ccache#76, an entry which is both GC'd and deleted (either via a delete or an update) will result in the internal link list having a nil tail (because removing the same node multiple times from the linked list does that). doDelete was already aware of "invalid" nodes (where item.node == nil), so the solution seems to be as simple as setting item.node = nil during GC.
Also, item.promotions doesn't need to be loaded/stored using atomic. Once upon a time it did. Cache was updated long ago to not use atomic operations on it, but LayeredCache wasn't. They are both consistent now (they don't use atomic operations). Fixes: karlseguin/ccache#76
Thanks for this great software! In my usage scenario, i observed that cache size keep going up and dropped equals 0 for a long time, this eventually result in oom. I think there maybe some bug here.
Here is a test code:
When running this code, the size will greater than 5000, and dropped keep equals 0.
This may be the reason:
oldItem
is send todeletables
but not consumedgc()
triggered,oldItem
removed byList.Remove()
,oldItem
.node.Prev set to nildeletables
consumed,oldItem
removed again byList.Remove()
,l.Tail
set to nil (oldItem.node.Prev)gc()
will be skipped, becausenode = c.list.Tail = nil
, size keep going up and dropped keep equals 0deletables
not been gc,l.Tail
maybe set to non-nil,gc()
may recovered, but it will fail in the furture with the same reasonThe text was updated successfully, but these errors were encountered: