Evict recentlyAccepted blocks based on wall-clock time (#3460)

Author: iansuvak

utils/window/window.go

@@ -33,7 +33,7 @@ type window[T any] struct {
 	minSize int
 
 	// mutex for synchronization
-	lock sync.RWMutex
+	lock sync.Mutex
 	// elements in the window
 	elements buffer.Deque[node[T]]
 }
@@ -77,8 +77,9 @@ func (w *window[T]) Add(value T) {
 
 // Oldest returns the oldest element in the window.
 func (w *window[T]) Oldest() (T, bool) {
-	w.lock.RLock()
-	defer w.lock.RUnlock()
+	w.lock.Lock()
+	defer w.lock.Unlock()
+	w.removeStaleNodes()
 
 	oldest, ok := w.elements.PeekLeft()
 	if !ok {
@@ -89,8 +90,9 @@ func (w *window[T]) Oldest() (T, bool) {
 
 // Length returns the number of elements in the window.
 func (w *window[T]) Length() int {
-	w.lock.RLock()
-	defer w.lock.RUnlock()
+	w.lock.Lock()
+	defer w.lock.Unlock()
+	w.removeStaleNodes()
 
 	return w.elements.Len()
 }
@@ -100,13 +102,9 @@ func (w *window[T]) removeStaleNodes() {
 	// If we're beyond the expiry threshold, removeStaleNodes this node from our
 	// window. Nodes are guaranteed to be strictly increasing in entry time,
 	// so we can break this loop once we find the first non-stale one.
-	newest, ok := w.elements.PeekRight()
-	if !ok {
-		return
-	}
 	for w.elements.Len() > w.minSize {
 		oldest, ok := w.elements.PeekLeft()
-		if !ok || newest.entryTime.Sub(oldest.entryTime) <= w.ttl {
+		if !ok || w.clock.Time().Sub(oldest.entryTime) <= w.ttl {
 			return
 		}
 		_, _ = w.elements.PopLeft()

utils/window/window_test.go

@@ -127,7 +127,7 @@ func TestTTLAdd(t *testing.T) {
 	require.Equal(4, oldest)
 }
 
-// TestTTLLength tests that elements are not evicted on Length
+// TestTTLLength tests that elements are evicted on Length
 func TestTTLLength(t *testing.T) {
 	require := require.New(t)
 
@@ -156,10 +156,10 @@ func TestTTLLength(t *testing.T) {
 	clock.Set(start.Add(testTTL + time.Second))
 
 	// No more elements should be present in the window.
-	require.Equal(3, window.Length())
+	require.Equal(0, window.Length())
 }
 
-// TestTTLOldest tests that stale elements are not evicted on calling Oldest
+// TestTTLOldest tests that stale elements are evicted on calling Oldest
 func TestTTLOldest(t *testing.T) {
 	require := require.New(t)
 
@@ -188,15 +188,25 @@ func TestTTLOldest(t *testing.T) {
 	require.Equal(1, oldest)
 	require.Equal(3, window.elements.Len())
 
-	// Now we're one second past the ttl of 10 seconds as defined in testTTL,
+	// Now we're one second before the ttl of 10 seconds as defined in testTTL,
 	// so all existing elements shoud still exist.
-	clock.Set(start.Add(testTTL + time.Second))
+	// Add 4 to the window to make it:
+	// [1, 2, 3, 4]
+	clock.Set(start.Add(testTTL - time.Second))
+	window.Add(4)
 
-	// Now there should be three elements in the window
 	oldest, ok = window.Oldest()
 	require.True(ok)
 	require.Equal(1, oldest)
-	require.Equal(3, window.elements.Len())
+	require.Equal(4, window.elements.Len())
+
+	// Now we're one second past the ttl of the initial 3 elements
+	// call to oldest should now evict 1,2,3 and return 4.
+	clock.Set(start.Add(testTTL + time.Second))
+	oldest, ok = window.Oldest()
+	require.True(ok)
+	require.Equal(4, oldest)
+	require.Equal(1, window.elements.Len())
 }
 
 // Tests that we bound the amount of elements in the window