Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
HBase >> mail # dev >> Review Request: Unit test and fix for multi-threaded executor service


Copy link to this message
-
Re: Review Request: Unit test and fix for multi-threaded executor service

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1064/#review1629
-----------------------------------------------------------

trunk/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
<http://review.cloudera.org/r/1064/#comment5485>

    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.

trunk/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
<http://review.cloudera.org/r/1064/#comment5486>

    This statement is unnecessary.

trunk/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
<http://review.cloudera.org/r/1064/#comment5488>

    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.

trunk/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
<http://review.cloudera.org/r/1064/#comment5487>

    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.
- Benoit
On 2010-10-21 14:59:17, Jonathan Gray wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1064/
> -----------------------------------------------------------
>
> (Updated 2010-10-21 14:59:17)
>
>
> Review request for hbase and stack.
>
>
> Summary
> -------
>
> See HBASE-3139
>
>
> Diffs
> -----
>
>   trunk/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java 1026145
>   trunk/src/test/java/org/apache/hadoop/hbase/executor/TestExecutorService.java PRE-CREATION
>
> Diff: http://review.cloudera.org/r/1064/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jonathan
>
>