From a633d5f3e5ca0b950a3cfe4484f8c1318eb46312 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 23 Feb 2021 12:13:41 -0500 Subject: [PATCH] lib/ttlcache: never decrease the expiry on update --- lib/ttlcache/eviction.go | 16 +++++++++++----- lib/ttlcache/eviction_test.go | 13 ++++++++++--- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/ttlcache/eviction.go b/lib/ttlcache/eviction.go index d07b2bf46..7e985a130 100644 --- a/lib/ttlcache/eviction.go +++ b/lib/ttlcache/eviction.go @@ -76,8 +76,10 @@ func (h *ExpiryHeap) Add(key string, expiry time.Duration) *Entry { return entry } -// Update the entry that is currently at idx with the new expiry time. The heap -// will be rebalanced after the entry is updated. +// Update the entry that is currently at idx with the new expiry time, if the new +// expiry time is further into the future. The heap will be rebalanced after the +// entry is updated. If the new expiry time is earlier than the existing expiry +// time than the expiry is not modified. // // Must be synchronized by the caller. func (h *ExpiryHeap) Update(idx int, expiry time.Duration) { @@ -86,11 +88,15 @@ func (h *ExpiryHeap) Update(idx int, expiry time.Duration) { return } entry := h.entries[idx] - entry.expiry = time.Now().Add(expiry) + newExpiry := time.Now().Add(expiry) + + // Ignore the new expiry if the time is earlier than the existing expiry. + if entry.expiry.After(newExpiry) { + return + } + entry.expiry = newExpiry heap.Fix((*entryHeap)(h), idx) - // If the previous index and current index are both zero then Fix did not - // swap the entry, and notify must be called here. if idx == 0 || entry.heapIndex == 0 { h.notify() } diff --git a/lib/ttlcache/eviction_test.go b/lib/ttlcache/eviction_test.go index 1fdd8e485..2bcfc5483 100644 --- a/lib/ttlcache/eviction_test.go +++ b/lib/ttlcache/eviction_test.go @@ -47,8 +47,8 @@ func TestExpiryHeap(t *testing.T) { testNoMessage(t, ch) }) - runStep(t, "update entry3 to expire first", func(t *testing.T) { - h.Update(entry3.heapIndex, 10*time.Millisecond) + runStep(t, "update so that entry3 expires first", func(t *testing.T) { + h.Update(entry.heapIndex, 2000*time.Millisecond) assert.Equal(t, 1, entry.heapIndex) assert.Equal(t, 0, entry3.heapIndex) testMessage(t, ch) @@ -56,12 +56,19 @@ func TestExpiryHeap(t *testing.T) { }) runStep(t, "0th element change triggers a notify", func(t *testing.T) { - h.Update(entry3.heapIndex, 20) + h.Update(entry3.heapIndex, 1500*time.Millisecond) assert.Equal(t, 1, entry.heapIndex) // no move assert.Equal(t, 0, entry3.heapIndex) testMessage(t, ch) testNoMessage(t, ch) // one message }) + + runStep(t, "update can not decrease expiry time", func(t *testing.T) { + h.Update(entry.heapIndex, 100*time.Millisecond) + assert.Equal(t, 1, entry.heapIndex) // no move + assert.Equal(t, 0, entry3.heapIndex) + testNoMessage(t, ch) // no notify, because no change in the heap + }) } func testNoMessage(t *testing.T, ch <-chan struct{}) {