diff --git a/lightnode/chunkgroup/assignment_queue.go b/lightnode/chunkgroup/assignment_queue.go index 157809866c..eaa5ee73c9 100644 --- a/lightnode/chunkgroup/assignment_queue.go +++ b/lightnode/chunkgroup/assignment_queue.go @@ -53,7 +53,7 @@ type assignmentQueue struct { // The number of elements in the queue. Tracked separately since the heap and NodeIdSet // may contain removed nodes that have not yet been fully garbage collected. - size uint + size uint32 } // newAssignmentQueue creates a new priority queue. @@ -67,7 +67,7 @@ func newAssignmentQueue() *assignmentQueue { } // Size returns the number of elements in the priority queue. -func (queue *assignmentQueue) Size() uint { +func (queue *assignmentQueue) Size() uint32 { return queue.size } @@ -124,6 +124,7 @@ func (queue *assignmentQueue) Remove(key assignmentKey) { queue.size-- queue.assignmentSet[key] = false + queue.collectGarbage() } // collectGarbage removes all nodes that have been removed from the queue but have not yet been fully deleted. @@ -134,7 +135,7 @@ func (queue *assignmentQueue) collectGarbage() { } // sanity check to prevent infinite loops - maxIterations := len(queue.heap.data) + maxIterations := len(queue.heap.data) + 1 for { maxIterations-- @@ -142,6 +143,10 @@ func (queue *assignmentQueue) collectGarbage() { panic("garbage collection did not terminate") } + if len(queue.heap.data) == 0 { + return + } + next := queue.heap.data[0] notRemoved, present := queue.assignmentSet[next.key] @@ -155,5 +160,6 @@ func (queue *assignmentQueue) collectGarbage() { } heap.Pop(queue.heap) + delete(queue.assignmentSet, next.key) } } diff --git a/lightnode/chunkgroup/assignment_queue_test.go b/lightnode/chunkgroup/assignment_queue_test.go index 74b071ef50..5c35d449c6 100644 --- a/lightnode/chunkgroup/assignment_queue_test.go +++ b/lightnode/chunkgroup/assignment_queue_test.go @@ -31,17 +31,17 @@ func randomAssignment(nextShuffleTime time.Time) *chunkGroupAssignment { func TestEmptyQueue(t *testing.T) { queue := newAssignmentQueue() - assert.Equal(t, uint(0), queue.Size()) + assert.Equal(t, uint32(0), queue.Size()) assert.Nil(t, queue.Pop()) assert.Nil(t, queue.Peek()) - assert.Equal(t, uint(0), queue.Size()) + assert.Equal(t, uint32(0), queue.Size()) } func TestInOrderInsertion(t *testing.T) { tu.InitializeRandom() queue := newAssignmentQueue() - assert.Equal(t, uint(0), queue.Size()) + assert.Equal(t, uint32(0), queue.Size()) startTime := tu.RandomTime() numberOfElements := uint(100) @@ -59,18 +59,18 @@ func TestInOrderInsertion(t *testing.T) { queue.Push(element) } - assert.Equal(t, i+1, queue.Size()) + assert.Equal(t, uint32(i+1), queue.Size()) } // Pop elements in order. for i := uint(0); i < numberOfElements; i++ { preview := queue.Peek() assert.Equal(t, expectedOrder[i], preview) - assert.Equal(t, numberOfElements-i, queue.Size()) + assert.Equal(t, uint32(numberOfElements-i), queue.Size()) next := queue.Pop() assert.Equal(t, expectedOrder[i], next) - assert.Equal(t, numberOfElements-i-1, queue.Size()) + assert.Equal(t, uint32(numberOfElements-i-1), queue.Size()) } } @@ -78,14 +78,14 @@ func TestReverseOrderInsertion(t *testing.T) { tu.InitializeRandom() queue := newAssignmentQueue() - assert.Equal(t, uint(0), queue.Size()) + assert.Equal(t, uint32(0), queue.Size()) startTime := tu.RandomTime() - numberOfElements := uint(100) + numberOfElements := uint32(100) expectedOrder := make([]*chunkGroupAssignment, 0, numberOfElements) // Generate the elements that will eventually be inserted. - for i := uint(0); i < numberOfElements; i++ { + for i := uint32(0); i < numberOfElements; i++ { element := randomAssignment(startTime.Add(time.Second * time.Duration(i))) expectedOrder = append(expectedOrder, element) } @@ -100,11 +100,11 @@ func TestReverseOrderInsertion(t *testing.T) { queue.Push(expectedOrder[i]) } - assert.Equal(t, numberOfElements-uint(i), queue.Size()) + assert.Equal(t, numberOfElements-uint32(i), queue.Size()) } // Pop elements in order. - for i := uint(0); i < numberOfElements; i++ { + for i := uint32(0); i < numberOfElements; i++ { preview := queue.Peek() assert.Equal(t, expectedOrder[i], preview) assert.Equal(t, numberOfElements-i, queue.Size()) @@ -119,7 +119,7 @@ func TestRandomInsertion(t *testing.T) { tu.InitializeRandom() queue := newAssignmentQueue() - assert.Equal(t, uint(0), queue.Size()) + assert.Equal(t, uint32(0), queue.Size()) startTime := tu.RandomTime() numberOfElements := uint(100) @@ -143,18 +143,18 @@ func TestRandomInsertion(t *testing.T) { queue.Push(expectedOrder[perm[i]]) } - assert.Equal(t, i+1, queue.Size()) + assert.Equal(t, uint32(i+1), queue.Size()) } // Pop elements in order. for i := uint(0); i < numberOfElements; i++ { preview := queue.Peek() assert.Equal(t, expectedOrder[i], preview) - assert.Equal(t, numberOfElements-i, queue.Size()) + assert.Equal(t, uint32(numberOfElements-i), queue.Size()) next := queue.Pop() assert.Equal(t, expectedOrder[i], next) - assert.Equal(t, numberOfElements-i-1, queue.Size()) + assert.Equal(t, uint32(numberOfElements-i-1), queue.Size()) } } @@ -162,7 +162,7 @@ func TestPeriodicRemoval(t *testing.T) { tu.InitializeRandom() queue := newAssignmentQueue() - assert.Equal(t, uint(0), queue.Size()) + assert.Equal(t, uint32(0), queue.Size()) startTime := tu.RandomTime() numberOfElements := uint(100) @@ -192,7 +192,7 @@ func TestPeriodicRemoval(t *testing.T) { queue.Push(expectedOrder[perm[i]]) } - assert.Equal(t, i+1, queue.Size()) + assert.Equal(t, uint32(i+1), queue.Size()) } removalCount := uint(0) @@ -209,7 +209,7 @@ func TestPeriodicRemoval(t *testing.T) { } removalCount++ - assert.Equal(t, numberOfElements-removalCount, queue.Size()) + assert.Equal(t, uint32(numberOfElements-removalCount), queue.Size()) } } @@ -217,11 +217,11 @@ func TestPeriodicRemoval(t *testing.T) { for i := uint(0); i < (numberOfElements - removalCount); i++ { preview := queue.Peek() assert.Equal(t, expectedOrderWithRemovals[i], preview) - assert.Equal(t, numberOfElements-i-removalCount, queue.Size()) + assert.Equal(t, uint32(numberOfElements-i-removalCount), queue.Size()) next := queue.Pop() assert.Equal(t, expectedOrderWithRemovals[i], next) - assert.Equal(t, numberOfElements-i-removalCount-1, queue.Size()) + assert.Equal(t, uint32(numberOfElements-i-removalCount-1), queue.Size()) } } @@ -229,7 +229,7 @@ func TestContiguousRemoval(t *testing.T) { tu.InitializeRandom() queue := newAssignmentQueue() - assert.Equal(t, uint(0), queue.Size()) + assert.Equal(t, uint32(0), queue.Size()) startTime := tu.RandomTime() numberOfElements := uint(100) @@ -259,7 +259,7 @@ func TestContiguousRemoval(t *testing.T) { queue.Push(expectedOrder[perm[i]]) } - assert.Equal(t, i+1, queue.Size()) + assert.Equal(t, uint32(i+1), queue.Size()) } removalCount := uint(0) @@ -276,7 +276,7 @@ func TestContiguousRemoval(t *testing.T) { } removalCount++ - assert.Equal(t, numberOfElements-removalCount, queue.Size()) + assert.Equal(t, uint32(numberOfElements-removalCount), queue.Size()) } } @@ -284,11 +284,11 @@ func TestContiguousRemoval(t *testing.T) { for i := uint(0); i < (numberOfElements - removalCount); i++ { preview := queue.Peek() assert.Equal(t, expectedOrderWithRemovals[i], preview) - assert.Equal(t, numberOfElements-i-removalCount, queue.Size()) + assert.Equal(t, uint32(numberOfElements-i-removalCount), queue.Size()) next := queue.Pop() assert.Equal(t, expectedOrderWithRemovals[i], next) - assert.Equal(t, numberOfElements-i-removalCount-1, queue.Size()) + assert.Equal(t, uint32(numberOfElements-i-removalCount-1), queue.Size()) } } @@ -296,7 +296,7 @@ func TestRemoveFollowedByPush(t *testing.T) { tu.InitializeRandom() queue := newAssignmentQueue() - assert.Equal(t, uint(0), queue.Size()) + assert.Equal(t, uint32(0), queue.Size()) startTime := tu.RandomTime() numberOfElements := uint(100) @@ -320,7 +320,7 @@ func TestRemoveFollowedByPush(t *testing.T) { queue.Push(expectedOrder[perm[i]]) } - assert.Equal(t, i+1, queue.Size()) + assert.Equal(t, uint32(i+1), queue.Size()) } removalCount := uint(0) @@ -337,7 +337,7 @@ func TestRemoveFollowedByPush(t *testing.T) { } removalCount++ - assert.Equal(t, numberOfElements-removalCount, queue.Size()) + assert.Equal(t, uint32(numberOfElements-removalCount), queue.Size()) } } @@ -346,7 +346,7 @@ func TestRemoveFollowedByPush(t *testing.T) { if i%7 == 0 { queue.Push(expectedOrder[i]) removalCount-- - assert.Equal(t, numberOfElements-removalCount, queue.Size()) + assert.Equal(t, uint32(numberOfElements-removalCount), queue.Size()) } } @@ -354,10 +354,10 @@ func TestRemoveFollowedByPush(t *testing.T) { for i := uint(0); i < (numberOfElements - removalCount); i++ { preview := queue.Peek() assert.Equal(t, expectedOrder[i], preview) - assert.Equal(t, numberOfElements-i, queue.Size()) + assert.Equal(t, uint32(numberOfElements-i), queue.Size()) next := queue.Pop() assert.Equal(t, expectedOrder[i], next) - assert.Equal(t, numberOfElements-i-1, queue.Size()) + assert.Equal(t, uint32(numberOfElements-i-1), queue.Size()) } } diff --git a/lightnode/chunkgroup/map_test.go b/lightnode/chunkgroup/map_test.go index 47a82d7caf..69a8f53a60 100644 --- a/lightnode/chunkgroup/map_test.go +++ b/lightnode/chunkgroup/map_test.go @@ -17,8 +17,31 @@ func randomRegistration(startTime time.Time, lastRegistration time.Time) *lightn tu.RandomTimeInRange(startTime, lastRegistration)) } -// TODO test that deletes things and ensures there is no garbage left in maps -// TODO take into account when a light node was registered +// Delete everything from the map and verify that the internal data structures have been cleaned up. +func verifyMapDeletion( + t *testing.T, + now time.Time, + m *Map, + expectedRegistrations *map[uint64]*lightnode.Registration) { + + // Delete remaining elements. + for id := range *expectedRegistrations { + m.Remove(id) + } + + assert.Equal(t, uint32(0), m.Size()) + assert.Equal(t, 0, len(m.assignmentMap)) + assert.Equal(t, 0, len(m.lightNodes)) + + assert.Equal(t, m.chunkGroupCount, uint32(len(m.chunkGroups))) + for chunkIndex := uint32(0); chunkIndex < m.chunkGroupCount; chunkIndex++ { + assert.Equal(t, 0, len(m.chunkGroups[chunkIndex])) + } + + assert.Equal(t, uint32(0), m.shuffleQueue.Size()) + assert.Equal(t, 0, len(m.shuffleQueue.assignmentSet)) + assert.Equal(t, 0, len(m.shuffleQueue.heap.data)) +} func TestAddRemoveGetOneAssignment(t *testing.T) { tu.InitializeRandom() @@ -78,6 +101,8 @@ func TestAddRemoveGetOneAssignment(t *testing.T) { assert.Equal(t, registration, cgMap.Get(id)) } } + + verifyMapDeletion(t, startTime, &cgMap, &expectedRegistrations) } func TestAddRemoveGetMultipleAssignments(t *testing.T) { @@ -138,6 +163,8 @@ func TestAddRemoveGetMultipleAssignments(t *testing.T) { assert.Equal(t, registration, cgMap.Get(id)) } } + + verifyMapDeletion(t, startTime, &cgMap, &expectedRegistrations) } func TestChunkGroupCalculationsSingleAssignment(t *testing.T) { @@ -230,6 +257,8 @@ func TestChunkGroupCalculationsSingleAssignment(t *testing.T) { assert.Equal(t, count, nodesReported) } + + verifyMapDeletion(t, startTime, &cgMap, &expectedRegistrations) } func TestChunkGroupCalculationsMultipleAssignments(t *testing.T) { @@ -348,6 +377,8 @@ func TestChunkGroupCalculationsMultipleAssignments(t *testing.T) { assert.Equal(t, expectedNodes, nodes) } } + + verifyMapDeletion(t, startTime, &cgMap, &expectedRegistrations) } func TestGetRandomNodeSingleAssignment(t *testing.T) { @@ -425,6 +456,8 @@ func TestGetRandomNodeSingleAssignment(t *testing.T) { } } } + + verifyMapDeletion(t, startTime, &cgMap, &expectedRegistrations) } func TestGetRandomNodeMultipleAssignments(t *testing.T) { @@ -511,6 +544,8 @@ func TestGetRandomNodeMultipleAssignments(t *testing.T) { } } } + + verifyMapDeletion(t, startTime, &cgMap, &expectedRegistrations) } func TestSingleChunkGroupSingleAssignment(t *testing.T) { @@ -602,6 +637,8 @@ func TestSingleChunkGroupSingleAssignment(t *testing.T) { assert.Equal(t, count, nodesReported) } + + verifyMapDeletion(t, startTime, &cgMap, &expectedRegistrations) } func TestSingleChunkGroupMultipleAssignments(t *testing.T) { @@ -693,4 +730,6 @@ func TestSingleChunkGroupMultipleAssignments(t *testing.T) { assert.Equal(t, count, nodesReported) } + + verifyMapDeletion(t, startTime, &cgMap, &expectedRegistrations) }