[
https://issues.apache.org/jira/browse/HBASE-14331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14738350#comment-14738350
]
Hiroshi Ikeda commented on HBASE-14331:
---------------------------------------
{quote}
In SemaphoreBasedPriorityCallQueue<E>, do you really need the private static
abstract class Container<E>? Can the comparator directly work on the Element
and the ConcurrentkSkipListMap use that comparator which operates on the
element E directly?
{quote}
{{Container}} is needed, and you can add the same object twice or more, which
is expected for a queue.
{{Container}} is not needed to be abstract. That is a relic of trying to
implement the {{BlockingQueue}}, and another concrete class was used to extract
a sub map to implement the method {{contains}} etc.
{quote}
If the value part of ConcurrentkSkipListMap is not used can we use
ConcurrentSkipListSet.
{quote}
Using {{ConcurrentSkipListSet}} would be better if it had a method
{{addIfAbsent}} or something. It is possible that two containers for the same
element have the same hash code and adding a new container is failed.
{quote}
private static int compareAddition(Container<?> c1, Container<?> c2) {
Is this really needed? Can we just allow the natural ordering to happen here?
{quote}
Comparison on {{Container}} depends on the given comparator on contained
elements. It is possible to change {{Container}} to be a non-static class and
implement {{Comparable}} which refers the outside instance variable of the
given comparator, but I think that is more confusing.
> a single callQueue related improvements
> ---------------------------------------
>
> Key: HBASE-14331
> URL: https://issues.apache.org/jira/browse/HBASE-14331
> Project: HBase
> Issue Type: Improvement
> Components: IPC/RPC, Performance
> Reporter: Hiroshi Ikeda
> Assignee: Hiroshi Ikeda
> Priority: Minor
> Attachments: BlockingQueuesPerformanceTestApp-output.pdf,
> BlockingQueuesPerformanceTestApp-output.txt,
> BlockingQueuesPerformanceTestApp.java, CallQueuePerformanceTestApp.java,
> HBASE-14331-V2.patch, HBASE-14331-V3.patch, HBASE-14331.patch,
> HBASE-14331.patch, SemaphoreBasedBlockingQueue.java,
> SemaphoreBasedLinkedBlockingQueue.java,
> SemaphoreBasedPriorityBlockingQueue.java
>
>
> {{LinkedBlockingQueue}} well separates locks between the {{take}} method and
> the {{put}} method, but not between takers, and not between putters. These
> methods are implemented to take locks at the almost beginning of their logic.
> HBASE-11355 introduces multiple call-queues to reduce such possible
> congestion, but I doubt that it is required to stick to {{BlockingQueue}}.
> There are the other shortcomings of using {{BlockingQueue}}. When using
> multiple queues, since {{BlockingQueue}} blocks threads it is required to
> prepare enough threads for each queue. It is possible that there is a queue
> starving for threads while there is another queue where threads are idle.
> Even if you can tune parameters to avoid such situations, the tuning is not
> so trivial.
> I suggest using a single {{ConcurrentLinkedQueue}} with {{Semaphore}}.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
