[ 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