Merge pull request #646 from iotaledger/fix/eviction-state

Fix eviction state for 0 value - covering all edge cases

ds/reactive/eviction_state.go

@@ -3,9 +3,10 @@ package reactive
 // EvictionState is a reactive component that implements a slot based eviction mechanism.
 type EvictionState[Type EvictionStateSlotType] interface {
 	// LastEvictedSlot returns a reactive variable that contains the index of the last evicted slot.
-	LastEvictedSlot() Variable[Type]
+	LastEvictedSlot() Type
 
-	// EvictionEvent returns the event that is triggered when the given slot was evicted.
+	// EvictionEvent returns the event that is triggered when the given slot was evicted. It returns a triggered event
+	// as default if the slot was already evicted.
 	EvictionEvent(slot Type) Event
 
 	// Evict evicts the given slot and triggers the corresponding eviction events.

ds/reactive/eviction_state_impl.go

@@ -1,13 +1,18 @@
 package reactive
 
 import (
+	"sync"
+
 	"github.com/iotaledger/hive.go/ds/shrinkingmap"
+	"github.com/iotaledger/hive.go/lo"
 )
 
 // evictionState is the default implementation of the EvictionState interface.
 type evictionState[Type EvictionStateSlotType] struct {
+	mutex sync.RWMutex
+
 	// lastEvictedSlot is the index of the last evicted slot.
-	lastEvictedSlot Variable[Type]
+	lastEvictedSlot *Type
 
 	// evictionEvents is the map of all eviction events that were not evicted yet.
 	evictionEvents *shrinkingmap.ShrinkingMap[Type, Event]
@@ -16,29 +21,31 @@ type evictionState[Type EvictionStateSlotType] struct {
 // newEvictionState creates a new evictionState instance.
 func newEvictionState[Type EvictionStateSlotType]() *evictionState[Type] {
 	return &evictionState[Type]{
-		lastEvictedSlot: NewVariable[Type](),
-		evictionEvents:  shrinkingmap.New[Type, Event](),
+		evictionEvents: shrinkingmap.New[Type, Event](),
 	}
 }
 
-// LastEvictedSlot returns a reactive variable that contains the index of the last evicted slot.
-func (e *evictionState[Type]) LastEvictedSlot() Variable[Type] {
-	return e.lastEvictedSlot
+func (e *evictionState[Type]) LastEvictedSlot() Type {
+	e.mutex.RLock()
+	defer e.mutex.RUnlock()
+
+	if e.lastEvictedSlot == nil {
+		return 0
+	}
+
+	return *e.lastEvictedSlot
 }
 
 // EvictionEvent returns the event that is triggered when the given slot was evicted.
 func (e *evictionState[Type]) EvictionEvent(slot Type) Event {
-	evictionEvent := evictedSlotEvent
-
-	e.lastEvictedSlot.Read(func(lastEvictedSlotIndex Type) {
-		var zeroValue Type
+	e.mutex.RLock()
+	defer e.mutex.RUnlock()
 
-		if slot > lastEvictedSlotIndex || (slot == zeroValue && lastEvictedSlotIndex == zeroValue) {
-			evictionEvent, _ = e.evictionEvents.GetOrCreate(slot, NewEvent)
-		}
-	})
+	if e.lastEvictedSlot == nil || slot > *e.lastEvictedSlot {
+		return lo.Return1(e.evictionEvents.GetOrCreate(slot, NewEvent))
+	}
 
-	return evictionEvent
+	return evictedSlotEvent
 }
 
 // Evict evicts the given slot and triggers the corresponding eviction events.
@@ -49,31 +56,30 @@ func (e *evictionState[Type]) Evict(slot Type) {
 }
 
 // evict advances the lastEvictedSlot to the given slot and returns the events that shall be triggered.
-func (e *evictionState[Type]) evict(slot Type) (eventsToTrigger []Event) {
-	var zeroValue Type
-
-	e.lastEvictedSlot.Compute(func(lastEvictedSlotIndex Type) Type {
-		var startingSlot Type
-		if slot <= lastEvictedSlotIndex {
-			// We only continue if the slot is the zero value, and we have not evicted any slot yet.
-			if slot != zeroValue || lastEvictedSlotIndex != zeroValue {
-				return lastEvictedSlotIndex
-			}
-
-			startingSlot = slot
-		} else {
-			startingSlot = lastEvictedSlotIndex + Type(1)
-		}
+func (e *evictionState[Type]) evict(slot Type) []Event {
+	e.mutex.Lock()
+	defer e.mutex.Unlock()
+
+	if e.lastEvictedSlot != nil && slot <= *e.lastEvictedSlot {
+		return nil
+	}
 
-		for i := startingSlot; i <= slot; i++ {
-			if slotEvictedEvent, exists := e.evictionEvents.Get(i); exists {
-				eventsToTrigger = append(eventsToTrigger, slotEvictedEvent)
-				e.evictionEvents.Delete(i)
-			}
+	var startingSlot Type
+	if e.lastEvictedSlot == nil {
+		startingSlot = 0
+	} else {
+		startingSlot = *e.lastEvictedSlot + Type(1)
+	}
+
+	var eventsToTrigger []Event
+	for i := startingSlot; i <= slot; i++ {
+		if slotEvictedEvent, exists := e.evictionEvents.Get(i); exists {
+			eventsToTrigger = append(eventsToTrigger, slotEvictedEvent)
+			e.evictionEvents.Delete(i)
 		}
+	}
 
-		return slot
-	})
+	e.lastEvictedSlot = &slot
 
 	return eventsToTrigger
 }

ds/reactive/eviction_state_test.go

@@ -6,30 +6,20 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// TestNewEvictionState tests the newEvictionState function
-func TestNewEvictionState(t *testing.T) {
-	state := newEvictionState[int]()
-	require.NotNil(t, state, "newEvictionState should not return nil")
-	require.NotNil(t, state.evictionEvents, "Eviction events map should be initialized")
-}
-
-// TestLastEvictedSlot tests the LastEvictedSlot method
-func TestLastEvictedSlot(t *testing.T) {
-	state := newEvictionState[int]()
-	slot := state.LastEvictedSlot()
-	require.NotNil(t, slot, "LastEvictedSlot should not return nil")
-}
-
 // TestEvictionEvent tests the EvictionEvent method
 func TestEvictionEvent(t *testing.T) {
 	state := newEvictionState[int]()
 
 	// Test with a slot that should create a new event
-	event := state.EvictionEvent(1)
+	event := state.EvictionEvent(0)
+	require.NotNil(t, event, "EvictionEvent should not return nil for a new slot")
+
+	// Test with a slot that should create a new event
+	event = state.EvictionEvent(1)
 	require.NotNil(t, event, "EvictionEvent should not return nil for a new slot")
 
 	// Test with a slot that should not create a new event
-	state.lastEvictedSlot.Set(1)
+	state.Evict(1)
 	event = state.EvictionEvent(0)
 	require.Equal(t, evictedSlotEvent, event, "EvictionEvent should return the evictedSlotEvent for a slot lower than lastEvictedSlot")
 }
@@ -59,27 +49,43 @@ func TestEvict(t *testing.T) {
 		_, exists := state.evictionEvents.Get(slotToEvict)
 		require.False(t, exists, "Evicted slot should no longer exist in evictionEvents")
 	}
+
 }
 
 // TestEvictPrivate tests the private evict method
 func TestEvictTriggered(t *testing.T) {
 	state := newEvictionState[int]()
 
-	// Test with a slot less than the current lastEvictedSlotIndex
-	state.lastEvictedSlot.Set(2)                  // Set the last evicted slot to 2
-	state.evictionEvents.Set(1, evictedSlotEvent) // Set an event for slot 1
-	eventsToTrigger := state.evict(1)             // Try to evict slot 1, which is less than the last evicted slot
-	require.Len(t, eventsToTrigger, 0, "evict should not return any events when slot is less than or equal to lastEvictedSlotIndex")
-
-	// Test with a slot equal to the current lastEvictedSlotIndex
-	eventsToTrigger = state.evict(2) // Try to evict slot 2, which is equal to the last evicted slot
-	require.Len(t, eventsToTrigger, 0, "evict should not return any events when slot is less than or equal to lastEvictedSlotIndex")
-
-	// Test with a slot greater than the current lastEvictedSlotIndex
-	slotToEvict := 3
-	state.evictionEvents.Set(slotToEvict, NewEvent())
-	eventToTrigger := state.EvictionEvent(slotToEvict)
-	require.False(t, eventToTrigger.WasTriggered(), "evicted event should not have been triggered")
-	state.Evict(slotToEvict)
-	require.True(t, eventToTrigger.WasTriggered(), "evicted event should have been triggered")
+	{
+		var expectedEventsToTrigger []Event
+		expectedEventsToTrigger = append(expectedEventsToTrigger, state.EvictionEvent(0))
+		expectedEventsToTrigger = append(expectedEventsToTrigger, state.EvictionEvent(1))
+		expectedEventsToTrigger = append(expectedEventsToTrigger, state.EvictionEvent(3))
+
+		actualEventsToTrigger := state.evict(5)
+		require.Equal(t, expectedEventsToTrigger, actualEventsToTrigger, "evict should return the correct events to trigger")
+	}
+
+	// Test with a slot smaller than current lastEvictedSlot
+	{
+		// Try to evict slot 0, which is less than the last evicted slot
+		require.Len(t, state.evict(0), 0, "evict should not return any events when slot is less than or equal to lastEvictedSlotIndex")
+		require.Len(t, state.evict(4), 0, "evict should not return any events when slot is less than or equal to lastEvictedSlotIndex")
+		require.Len(t, state.evict(5), 0, "evict should not return any events when slot is less than or equal to lastEvictedSlotIndex")
+	}
+
+	{
+		var expectedEventsToTrigger []Event
+		expectedEventsToTrigger = append(expectedEventsToTrigger, state.EvictionEvent(6))
+		expectedEventsToTrigger = append(expectedEventsToTrigger, state.EvictionEvent(8))
+
+		actualEventsToTrigger := state.evict(10)
+		require.Equal(t, expectedEventsToTrigger, actualEventsToTrigger, "evict should return the correct events to trigger")
+	}
+
+	// Requesting a EvictionEvent of an evicted slot should return the evictedSlotEvent
+	{
+		require.Equal(t, evictedSlotEvent, state.EvictionEvent(0), "EvictionEvent should return the evictedSlotEvent for a slot lower than lastEvictedSlot")
+		require.Equal(t, evictedSlotEvent, state.EvictionEvent(10), "EvictionEvent should return the evictedSlotEvent for a slot lower than lastEvictedSlot")
+	}
 }