[
https://issues.apache.org/jira/browse/YARN-2888?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15239364#comment-15239364
]
Carlo Curino commented on YARN-2888:
------------------------------------
Hi [~asuresh]
Sorry for the long delay reviewing this. The patch looks generally good, though
I don't have complete context of how the thresholds computed here are used, but
as much as I can see here things look reasonable.
General:
# First a general question. I see you add a few extra conf parameters. I was
wondering whether we can come up with a better mechanism to configure policies,
other than global conf parameters. For now you have a fixed policy and 4 param,
but as we develop other policies, we will start having more and more params.
This is a general issue, you are following the standard way of doing this, I am
just wondering whether we could do better than what is done so far.
# In general, I would prefer to see the policy to be configurable. Unless you
guys think this is the one and only policy we would want for queueing (at least
for a long while).
# Would it make sense to make the parameters you pass down in the
{{NodeHeartBeatResponse}} to be more general than "queuLimit"? In the future
you might want to have the central component to send down other information,
combined with local up to date information to make decisions. What I am
suggesting is to make the "data-bus" you are establishing between central
policy components and local enforcers/policies more generic, so that you can
add/change things inside it later. Maybe this just means renaming the
{{ContainerQueuingLimt}} to {{ContainerQueueingCommand}} or something like
that, which is consistent if later on you don't send down just "limits".
# in the .proto it would likely help other devs if you say max_wait_time_in_ms
or something like that, which indicates time granularity. Also is int32 always
enough? (likely silly question)
# spuriuos import in {{ClusterMonitor.java}}?
# as above maybe maybe of the classes/methods (e.g.,
{{getQueueLimitCalculator()}}) use the term "limit", where maybe "policy" would
allow you to be more general and fit more stuff under the same name later on.
In {{QueueLimitCalculator}}:
# Is it reasonable to assume the caller of {{QueueLimitCalculator.update()}}
will synchronize on topKNodes? This seems likely to create issues later on.
Maybe making the topKNodes datastructure a concurrenthasmap is enough, even if
the calculation is done on a slightly off view of the world, it should be
statistically relevant correct?
# If topKNodes is << than total nodes, you could create a local list of
{{ClusterNode}}(s) and scan over it instead of invoking
{{nodeSelector.getClusterNodes().get(nId)}} over and over again.
# Do you have a guarantee that the topKNodes list of Ids is already sorted by
"val"? If not the median calculation might be off.
# The median calculation would be busted if you have K=1 (line 567 of the
patch would lookup at -1) likely want to protect from that.
# stdevMedian is never updated (line 580 is a wrong copy and paste of 579)
# stedevMedian is typically referred to as Median Absolute Deviation, if I am
not mistaken stdev is used only for the deviation based on the mean.
# Can you guys comment on when using the MAD vs STDEV makes sense? Do you have
experiments showing the impact or one or the other?
# At line 630, 632 you simply cast-down float to int (which trims the decimal
values). Is this the right way of rounding this? Wouldn't Math.round() be
better?
> Corrective mechanisms for rebalancing NM container queues
> ---------------------------------------------------------
>
> Key: YARN-2888
> URL: https://issues.apache.org/jira/browse/YARN-2888
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: nodemanager, resourcemanager
> Reporter: Konstantinos Karanasos
> Assignee: Arun Suresh
> Attachments: YARN-2888-yarn-2877.001.patch,
> YARN-2888-yarn-2877.002.patch
>
>
> Bad queuing decisions by the LocalRMs (e.g., due to the distributed nature of
> the scheduling decisions or due to having a stale image of the system) may
> lead to an imbalance in the waiting times of the NM container queues. This
> can in turn have an impact in job execution times and cluster utilization.
> To this end, we introduce corrective mechanisms that may remove (whenever
> needed) container requests from overloaded queues, adding them to less-loaded
> ones.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
