@jchadwick-buf, @lrewega was trying out the least-loaded picker and it panick'ed! 😱
It turns out, the update operation -- which reconciles the heap with a new set of connections provided from a resolve -- was iterating from start to finish of the slice, but it would also change the length of the slice while iterating, when it decided it needed to pop an item that should no longer be present.
So the panic was an out-of-bounds slice index. But the iteration is incorrect for more than just that reason: removing an item from the heap that way will also re-order items, to sift things up and down to preserve heap invariants within the slice. So to proceed iterating through the slice means we might visit the same item more than once and fail to visit some items, as their order may have changed underneath us.
So now the logic does a single pass through the slice to remove unneeded items, by simply overwriting them (and setting to nil if necessary). So at the end of the first pass, all that is left in the slice are the items in the new set of connections, compacted to the beginning of the slice (everything after is set to nil). Then we append any new connections. And at the very end, we re-heapify to restore heap invariants after all of that.
This adds a test that is hopefully pretty convincing that it all works correctly now. The test is a sequence of operations, including acquiring connections, releasing them, and updating the resolved set.
@jchadwick-buf, @lrewega was trying out the least-loaded picker and it panick'ed! 😱
It turns out, the
updateoperation -- which reconciles the heap with a new set of connections provided from a resolve -- was iterating from start to finish of the slice, but it would also change the length of the slice while iterating, when it decided it needed to pop an item that should no longer be present.So the panic was an out-of-bounds slice index. But the iteration is incorrect for more than just that reason: removing an item from the heap that way will also re-order items, to sift things up and down to preserve heap invariants within the slice. So to proceed iterating through the slice means we might visit the same item more than once and fail to visit some items, as their order may have changed underneath us.
So now the logic does a single pass through the slice to remove unneeded items, by simply overwriting them (and setting to nil if necessary). So at the end of the first pass, all that is left in the slice are the items in the new set of connections, compacted to the beginning of the slice (everything after is set to nil). Then we append any new connections. And at the very end, we re-heapify to restore heap invariants after all of that.
This adds a test that is hopefully pretty convincing that it all works correctly now. The test is a sequence of operations, including acquiring connections, releasing them, and updating the resolved set.