Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 5 additions & 15 deletions pkg/epp/flowcontrol/contracts/mocks/mocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,7 @@ type MockSafeQueue struct {
CapabilitiesV []flowcontrol.QueueCapability
LenV int
ByteSizeV uint64
PeekHeadV flowcontrol.QueueItemAccessor
PeekTailV flowcontrol.QueueItemAccessor
PeekV flowcontrol.QueueItemAccessor
AddFunc func(item flowcontrol.QueueItemAccessor)
RemoveFunc func(handle flowcontrol.QueueItemHandle) (flowcontrol.QueueItemAccessor, error)
CleanupFunc func(predicate contracts.PredicateFunc) []flowcontrol.QueueItemAccessor
Expand All @@ -176,12 +175,8 @@ func (m *MockSafeQueue) Capabilities() []flowcontrol.QueueCapability { return m.
func (m *MockSafeQueue) Len() int { return m.LenV }
func (m *MockSafeQueue) ByteSize() uint64 { return m.ByteSizeV }

func (m *MockSafeQueue) PeekHead() flowcontrol.QueueItemAccessor {
return m.PeekHeadV
}

func (m *MockSafeQueue) PeekTail() flowcontrol.QueueItemAccessor {
return m.PeekTailV
func (m *MockSafeQueue) Peek() flowcontrol.QueueItemAccessor {
return m.PeekV
}

func (m *MockSafeQueue) Add(item flowcontrol.QueueItemAccessor) {
Expand Down Expand Up @@ -366,8 +361,8 @@ func (m *MockManagedQueue) ByteSize() uint64 {
return size
}

// PeekHead returns the first item found in the mock queue. Note: map iteration order is not guaranteed.
func (m *MockManagedQueue) PeekHead() flowcontrol.QueueItemAccessor {
// Peek returns the first item found in the mock queue. Note: map iteration order is not guaranteed.
func (m *MockManagedQueue) Peek() flowcontrol.QueueItemAccessor {
m.mu.Lock()
defer m.mu.Unlock()
m.init()
Expand All @@ -376,8 +371,3 @@ func (m *MockManagedQueue) PeekHead() flowcontrol.QueueItemAccessor {
}
return nil // Queue is empty
}

// PeekTail is not implemented for this mock.
func (m *MockManagedQueue) PeekTail() flowcontrol.QueueItemAccessor {
return nil
}
2 changes: 1 addition & 1 deletion pkg/epp/flowcontrol/controller/internal/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ func (sp *Processor) selectItem(
}
// The queue itself is responsible for explicit ordering via its configured OrderingPolicy.
// We simply peek at the head.
return queue.PeekHead(), nil
return queue.Peek(), nil
}

// dispatchItem handles the final steps of dispatching an item: removing it from the queue and finalizing its outcome.
Expand Down
43 changes: 7 additions & 36 deletions pkg/epp/flowcontrol/framework/plugins/queue/benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ func BenchmarkQueues(b *testing.B) {
benchmarkAddPeekRemove(b, q)
})

b.Run("AddPeekTailRemove", func(b *testing.B) {
benchmarkAddPeekTailRemove(b, q)
})

b.Run("BulkAddThenBulkRemove", func(b *testing.B) {
benchmarkBulkAddThenBulkRemove(b, q)
})
Expand Down Expand Up @@ -79,10 +75,10 @@ func benchmarkAddRemove(b *testing.B, q contracts.SafeQueue) {
})
}

// benchmarkAddPeekRemove measures the throughput of a serial Add, PeekHead, and Remove sequence. This simulates a
// benchmarkAddPeekRemove measures the throughput of a serial Add, Peek, and Remove sequence. This simulates a
// common consumer pattern where a single worker peeks at an item before deciding to process and remove it.
func benchmarkAddPeekRemove(b *testing.B, q contracts.SafeQueue) {
// Pre-add one item so PeekHead doesn't fail on the first iteration.
// Pre-add one item so Peek doesn't fail on the first iteration.
initialItem := mocks.NewMockQueueItemAccessor(1, "initial", benchmarkFlowKey)
q.Add(initialItem)

Expand All @@ -91,11 +87,11 @@ func benchmarkAddPeekRemove(b *testing.B, q contracts.SafeQueue) {
for b.Loop() {
item := mocks.NewMockQueueItemAccessor(1, "item", benchmarkFlowKey)
q.Add(item)
peeked := q.PeekHead()
peeked := q.Peek()
if peeked == nil {
// In a concurrent benchmark, this could happen if the queue becomes empty.
// In a serial one, it's a fatal error.
b.Fatal("PeekHead failed")
b.Fatal("Peek failed")
}

_, err := q.Remove(peeked.Handle())
Expand All @@ -121,9 +117,9 @@ func benchmarkBulkAddThenBulkRemove(b *testing.B, q contracts.SafeQueue) {

// Remove the same number of items
for range items {
peeked := q.PeekHead()
peeked := q.Peek()
if peeked == nil {
b.Fatal("PeekHead failed")
b.Fatal("Peek failed")
}
if _, err := q.Remove(peeked.Handle()); err != nil {
b.Fatalf("Remove failed: %v", err)
Expand All @@ -132,31 +128,6 @@ func benchmarkBulkAddThenBulkRemove(b *testing.B, q contracts.SafeQueue) {
}
}

// benchmarkAddPeekTailRemove measures the throughput of a serial Add, PeekTail, and Remove sequence. This is useful for
// understanding the performance of accessing the lowest-priority item.
func benchmarkAddPeekTailRemove(b *testing.B, q contracts.SafeQueue) {
// Pre-add one item so PeekTail doesn't fail on the first iteration.
initialItem := mocks.NewMockQueueItemAccessor(1, "initial", benchmarkFlowKey)
q.Add(initialItem)

b.ReportAllocs()

for b.Loop() {
item := mocks.NewMockQueueItemAccessor(1, "item", benchmarkFlowKey)
q.Add(item)

peeked := q.PeekTail()
if peeked == nil {
b.Fatal("PeekTail failed")
}

_, err := q.Remove(peeked.Handle())
if err != nil {
b.Fatalf("Remove failed: %v", err)
}
}
}

// benchmarkHighContention simulates a more realistic workload with multiple producers and consumers operating on the
// queue concurrently.
func benchmarkHighContention(b *testing.B, q contracts.SafeQueue) {
Expand Down Expand Up @@ -190,7 +161,7 @@ func benchmarkHighContention(b *testing.B, q contracts.SafeQueue) {
// Consumers drive the benchmark.
b.RunParallel(func(pb *testing.PB) {
for pb.Next() {
peeked := q.PeekHead()
peeked := q.Peek()
if peeked != nil {
_, _ = q.Remove(peeked.Handle())
}
Expand Down
44 changes: 15 additions & 29 deletions pkg/epp/flowcontrol/framework/plugins/queue/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ var reverseEnqueueTimePolicy = &mocks.MockOrderingPolicy{
},
}

// testLifecycleAndOrdering is a helper function to execute a standard sequence of Add, PeekHead, and Remove operations
// testLifecycleAndOrdering is a helper function to execute a standard sequence of Add, Peek, and Remove operations
// on a queue. It verifies the queue's state (length, byte size) and item ordering based on the provided `itemsInOrder`
// slice, which should be pre-sorted according to the `comparatorName` (which is just a string for
// logging/identification).
Expand All @@ -71,11 +71,9 @@ func testLifecycleAndOrdering(
) {
t.Helper()

// PeekHead/PeekTail on empty queue
peeked := q.PeekHead()
assert.Nil(t, peeked, "[%s] PeekHead on empty queue should return a nil item", comparatorName)
peeked = q.PeekTail()
assert.Nil(t, peeked, "[%s] PeekTail on empty queue should return a nil item", comparatorName)
// Peek on empty queue
peeked := q.Peek()
assert.Nil(t, peeked, "[%s] Peek on empty queue should return a nil item", comparatorName)

// Add items
currentExpectedLen := 0
Expand Down Expand Up @@ -112,28 +110,20 @@ func testLifecycleAndOrdering(
expectedLen := initialLen
expectedByteSize := expectedTotalByteSize
for i, expectedItem := range itemsInOrder {
// Verify PeekHead
peeked = q.PeekHead()
require.NotNil(t, peeked, "[%s] PeekHead should return a non-nil item (iteration %d)", comparatorName, i)
// Verify Peek
peeked = q.Peek()
require.NotNil(t, peeked, "[%s] Peek should return a non-nil item (iteration %d)", comparatorName, i)
assert.Equal(t, expectedItem.OriginalRequest().ID(), peeked.OriginalRequest().ID(),
"[%s] PeekHead must return the item (ID: %s) at the head of the queue (iteration %d)",
"[%s] Peek must return the item (ID: %s) at the head of the queue (iteration %d)",
comparatorName, expectedItem.OriginalRequest().ID(), i)
peekedHandle := peeked.Handle()
require.NotNil(t, peekedHandle, "[%s] Handle from a peeked item must not be nil (iteration %d)", comparatorName, i)
require.False(t, peekedHandle.IsInvalidated(),
"[%s] Handle from a peeked item must not be invalidated (iteration %d)", comparatorName, i)
assert.Equal(t, expectedLen, q.Len(), "[%s] Len() must be unchanged after PeekHead (iteration %d)",
assert.Equal(t, expectedLen, q.Len(), "[%s] Len() must be unchanged after Peek (iteration %d)",
comparatorName, i)
assert.Equal(t, expectedByteSize, q.ByteSize(),
"[%s] ByteSize() must be unchanged after PeekHead (iteration %d)", comparatorName, i)

// Verify PeekTail
peekedTail := q.PeekTail()
require.NotNil(t, peekedTail, "[%s] PeekTail should return a non-nil item (iteration %d)", comparatorName, i)
// The tail is the last item in the *remaining* ordered slice.
expectedTailItem := itemsInOrder[len(itemsInOrder)-1]
assert.Equal(t, expectedTailItem.OriginalRequest().ID(), peekedTail.OriginalRequest().ID(),
"[%s] PeekTail must return the item with the lowest priority (iteration %d)", comparatorName, i)
"[%s] ByteSize() must be unchanged after Peek (iteration %d)", comparatorName, i)

// Remove the head item
removed, removeErr := q.Remove(peekedHandle)
Expand All @@ -156,8 +146,8 @@ func testLifecycleAndOrdering(
assert.Zero(t, q.Len(), "[%s] Queue length should be 0 after all items are removed", comparatorName)
assert.Zero(t, q.ByteSize(), "[%s] Queue byte size should be 0 after all items are removed", comparatorName)

peeked = q.PeekHead()
assert.Nil(t, peeked, "[%s] PeekHead on an empty queue should return a nil item again", comparatorName)
peeked = q.Peek()
assert.Nil(t, peeked, "[%s] Peek on an empty queue should return a nil item again", comparatorName)
}

// TestQueueConformance is the main conformance test suite for SafeQueue implementations.
Expand Down Expand Up @@ -404,7 +394,7 @@ func TestQueueConformance(t *testing.T) {
// Verify remaining items are correct
var remainingIDs []string
for q.Len() > 0 {
peeked := q.PeekHead()
peeked := q.Peek()
item, _ := q.Remove(peeked.Handle())
remainingIDs = append(remainingIDs, item.OriginalRequest().ID())
}
Expand Down Expand Up @@ -517,13 +507,9 @@ func TestQueueConformance(t *testing.T) {
case 2: // Inspect
_ = q.Len()
_ = q.ByteSize()
peeked := q.PeekHead()
if q.Len() == 0 {
assert.Nil(t, peeked, "PeekHead on empty queue expected nil")
}
peeked = q.PeekTail()
peeked := q.Peek()
if q.Len() == 0 {
assert.Nil(t, peeked, "PeekTail on empty queue expected nil")
assert.Nil(t, peeked, "Peek on empty queue expected nil")
}
case 3: // Cleanup
q.Cleanup(func(item flowcontrol.QueueItemAccessor) bool { return false })
Expand Down
16 changes: 2 additions & 14 deletions pkg/epp/flowcontrol/framework/plugins/queue/listqueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ func (lq *listQueue) ByteSize() uint64 {
return lq.byteSize.Load()
}

// PeekHead returns the item at the front of the queue without removing it.
func (lq *listQueue) PeekHead() flowcontrol.QueueItemAccessor {
// Peek returns the item at the front of the queue without removing it.
func (lq *listQueue) Peek() flowcontrol.QueueItemAccessor {
lq.mu.RLock()
defer lq.mu.RUnlock()

Expand All @@ -213,15 +213,3 @@ func (lq *listQueue) PeekHead() flowcontrol.QueueItemAccessor {
element := lq.requests.Front()
return element.Value.(flowcontrol.QueueItemAccessor)
}

// PeekTail returns the item at the back of the queue without removing it.
func (lq *listQueue) PeekTail() flowcontrol.QueueItemAccessor {
lq.mu.RLock()
defer lq.mu.RUnlock()

if lq.requests.Len() == 0 {
return nil
}
element := lq.requests.Back()
return element.Value.(flowcontrol.QueueItemAccessor)
}
Loading
Loading