hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Arpit Agarwal (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-10282) Create a FairCallQueue: a multi-level call queue which schedules incoming calls and multiplexes outgoing calls
Date Wed, 18 Jun 2014 06:36:04 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-10282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14034877#comment-14034877
] 

Arpit Agarwal commented on HADOOP-10282:
----------------------------------------

Hi Chris,

I reviewed this patch before its dependencies. My comments below:

# Initial question - FairCallQueue will not be on by default, correct?
# Do you need a configuration prefix ({{ns}}) or can you use a hardcoded setting name? If
the name is constructed dynamically it makes it difficult to find usages in the code.
# Will these settings be documented and if so then where?
# {{FairCallQueue#put}} violates the contract of {{BlockingQueue#put}}, which states that
put must wait indefinitely for space to become available.
# {{FairCallQueue#put}} - can we wraparound {{priorityLevel}} to zero?
# {{FairCallQueue#getFirstNonEmptyQueue}} - javadoc should be _returns the first non-empty
queue_.
# I am curious why do you need a Reentrant lock? If it is not acquired recursively can it
be replaced with a different synchronization primitive?
# Javadoc on {{#take}} also violates the contract of its overridden method which states that
it _Retrieves and removes the head of this queue, waiting if necessary until an element becomes
available_. However it looks like it does not actually return null in practice. So perhaps
we just need to fix the Javadoc?
# I am also unsureabout this comment - _As such, one should prefer poll() over take()_. There
is no such expectation of {{BlockingQueue#take}}. Is the comment necessary?
# {{FairCallQueue#iterator}} - should it just throw {{NotImplementedException}}?
# Nitpick: _spurriously_ -> _spuriously_ 

I am still reviewing TestFairCallQueue.java.

> Create a FairCallQueue: a multi-level call queue which schedules incoming calls and multiplexes
outgoing calls
> --------------------------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-10282
>                 URL: https://issues.apache.org/jira/browse/HADOOP-10282
>             Project: Hadoop Common
>          Issue Type: Sub-task
>            Reporter: Chris Li
>            Assignee: Chris Li
>         Attachments: HADOOP-10282.patch
>
>
> The FairCallQueue ensures quality of service by altering the order of RPC calls internally.

> It consists of three parts:
> 1. a Scheduler (`HistoryRpcScheduler` is provided) which provides a priority number from
0 to N (0 being highest priority)
> 2. a Multi-level queue (residing in `FairCallQueue`) which provides a way to keep calls
in priority order internally
> 3. a Multiplexer (`WeightedRoundRobinMultiplexer` is provided) which provides logic to
control which queue to take from
> Currently the Mux and Scheduler are not pluggable, but they probably should be (up for
discussion).
> This is how it is used:
> // Production
> 1. Call is created and given to the CallQueueManager
> 2. CallQueueManager requests a `put(T call)` into the `FairCallQueue` which implements
`BlockingQueue`
> 3. `FairCallQueue` asks its scheduler for a scheduling decision, which is an integer
e.g. 12
> 4. `FairCallQueue` inserts Call into the 12th queue: `queues.get(12).put(call)`
> // Consumption
> 1. CallQueueManager requests `take()` or `poll()` on FairCallQueue
> 2. `FairCallQueue` asks its multiplexer for which queue to draw from, which will also
be an integer e.g. 2
> 3. `FairCallQueue` draws from this queue if it has an available call (or tries other
queues if it is empty)
> Additional information is available in the linked JIRAs regarding the Scheduler and Multiplexer's
roles.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message