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

Erik Krogen commented on HDFS-14403:
------------------------------------

Thanks for sharing [~daryn]! It looks like the patch is built on top of a few 
custom things deviating from branch-2.8, but nothing looks significant. A 
rebase to make it easily apply would be appreciated.

It looks like the major difference in design decision was to make all of this a 
first-class concept within the IPC layer, whereas we approached it from keeping 
most of the lock-specific code within the HDFS module. Your approach ends up 
being a bit cleaner since, as a member of the IPC layer, it can be more 
directly integrated and avoids some of the tricks we had to do, like making the 
NameNode reach into the Server to set its own {{CostProvider}} implementation. 
Our YARN folks have recently been complaining about lock synchronization within 
the RM, so it may be that having this mechanism be general would be beneficial 
to them also. cc [~jhung] to see if there may be interest in building support 
for such functionality into the RM.

You also make static/ThreadLocal access to the {{Call}} object, something I'm 
not really fond of and that we tried to avoid in our patch, but it is 
interesting to see just how much simpler this made things by storing the 
response time in the {{Call}} object itself rather than maintaining an 
independent map of usernames that had to be kept in sync with the corresponding 
map in {{DecayRpcScheduler}}. Given that {{Server}} already maintains the call 
context and exposes it for use, it seems it may be worth it to leverage it here 
to reduce the complexity.

I would love to get some feedback from others on the two approaches -- looks 
like there are quite a few watchers so hopefully some will chime in :)

Two specific comments I want to make while I am thinking about them, to ensure 
I don't forget them if we decide to move forward with your patch:
* Regarding the {{scheduler instanceof DecayRpcScheduler}} comment, it doesn't 
seem to me that disrupting the interface would be a bad thing. We can add a 
{{default}} implementation to maintain backwards compatibility and mark the old 
version of {{addProcessingTime}} as deprecated.
* In {{DecayRpcScheduler}}, if you enable the cost-based version, you still 
make use of the counter-based version ({{getAndIncrementCallCounts()}}, so the 
costs will be off (the stored value will be cost + 1 for each call).

> Cost-Based RPC FairCallQueue
> ----------------------------
>
>                 Key: HDFS-14403
>                 URL: https://issues.apache.org/jira/browse/HDFS-14403
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: ipc, namenode
>            Reporter: Erik Krogen
>            Assignee: Christopher Gregorian
>            Priority: Major
>              Labels: qos, rpc
>         Attachments: CostBasedFairCallQueueDesign_v0.pdf, 
> HDFS-14403.001.patch, HDFS-14403.002.patch, HDFS-14403.003.patch, 
> HDFS-14403.branch-2.8.patch
>
>
> HADOOP-15016 initially described extensions to the Hadoop FairCallQueue 
> encompassing both cost-based analysis of incoming RPCs, as well as support 
> for reservations of RPC capacity for system/platform users. This JIRA intends 
> to track the former, as HADOOP-15016 was repurposed to more specifically 
> focus on the reservation portion of the work.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to