Unit test and fix for multi-threaded executor service
Review Request #1064 - updated 3 years, 4 months ago
Why is this class public? HBasePriorityBlockingQueue is a very bad class name. BoundedPriorityBlockingQueue would be better. It's annoying that PriorityQueue is unbounded, and because of that PriorityBlockingQueue is unbounded too.
This statement is unnecessary.
It's far cheaper to call super.size() [one lock acquisition] than getActiveCount() [one lock acquisition + 1 volatile reads per worker thread] Plus, super.size() returns a consistent value, whereas getActiveCount() doesn't guarantee an exact value (because each volatile read is done independently, without consistency guarantee across all reads). You would also no longer need to retain a reference to the thread pool from this class.
This sort of defeats the purpose of having a priority queue. If the priority queue is "full", you want to reject whichever element has the lowest priority, if the one you're trying to enqueue has a higher priority. If you don't need a priority queue, you can switch to an ArrayBlockingQueue.
Remove priority queue -- not needed. Fix naming format for threads. Make the thread pool max and corepool size the same so we have a fixed pool size. Adjust thread pool sizes down. They were upped when we weren't getting throughput when we thought executorservice was doing parallelism but wasnt'.