phillip-haydon
commented
3 months ago Took a bit to follow this but looks like it'll do the trick. Cancelling a worker causes it to complete its current task before disposing right? Make sure to do a test of spamming a fixed number of exception reports and confirming that the correct number of exceptions show up in Raygun. Maybe force exceptions to occur at certain points of the worker processing and check that it behaves as expected as well.
I ran a benchmark with 100k messages and found a race condition during the draining which caused ~1-5 messages to get stuck and not processed. This happened because when the AdjustWorkers is run, it grabs a count and once a count is obtained, but before the adjustment occurs, a message could come in and be queued, but we only allow 1 thread to enter the method at a time.
Solution was to just double-check the queue size before exiting the AjustWorkers and the issue goes away.
Besides this it appears to work well. In all cases the queue gets processed quicker than the old implementation, but somewhere around ~1k messages in the queue the memory usage grows a bit more than the old implementation, I think this is due to the Concurrent Queue resizing internally. But at 100k messages the difference looks to be ~5mb difference (45mb vs 50mb)
This gets cleaned up tho when the queue is processed.
In the old implementation we used a BlockingCollection which has a capacity argument, the ConcurrentQueue doesn't have this, but I have a check in the Queue with a drain percentage so if we hit the capacity it will drain 10% before accepting anymore messages. Give it some breathing room.
Should resolve #541 where up to 8 tasks workers can sit idle doing nothing when there's no messages to be sent. This change will create the task workers on demand based on the queue length and dispose of them if idle.