[
https://issues.apache.org/jira/browse/YARN-2017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13996165#comment-13996165
]
Wangda Tan commented on YARN-2017:
----------------------------------
Hi Jian,
Thanks for your efforts on this patch, some comments,
1) SchedulerNode.java
{code}
private synchronized void deductAvailableResource(Resource resource) {
if (resource == null) {
LOG.error("Invalid deduction of null resource for "
+ rmNode.getNodeAddress());
{code}
Since this is original logic of SchedulerNode, I think it's better to throw
exception instead of print a irresponsible log here. Null object passed in
should be considered big problem in scheduler. And several following places in
this class.
2) SchedulerNode.java
{code}
+ private synchronized boolean isValidContainer(Container c) {
+ if (launchedContainers.containsKey(c.getId()))
+ return true;
+ return false;
+ }
{code}
Better add "{..}" following "if"
3) SchedulerNode.java
{code}
+ public synchronized RMContainer getReservedContainer() {
+ return reservedContainer;
+ }
{code}
I think it's better to add a setReservedContainer(...) instead of manipulating
super.reservedContainer in its sub classes. And change "protected
reservedContainer" to private
4) In YarnScheduler.java
{code}
+ /**
+ * Get the whole resource capacity of the cluster.
+ * @return the whole resource capacity of the cluster.
+ */
+ @LimitedPrivate("yarn")
+ @Unstable
+ public Resource getClusterResource();
{code}
I'm wondering if it is meaningful to merge this method, too much code changes
due to this merge. I found there're no common logic (like
SchedulerNode/SchedulerAppAttempt) use it.
5) In FairScheduler.java
{code}
+ protected FSSchedulerApp getCurrentAttemptForContainer(ContainerId
containerId) {
+ return (FSSchedulerApp) super.getCurrentAttemptForContainer(containerId);
}
{code}
I understand this is a "adaptor", I agree with [~sandyr] about using generic to
eliminate such type casting?
> Merge some of the common lib code in schedulers
> -----------------------------------------------
>
> Key: YARN-2017
> URL: https://issues.apache.org/jira/browse/YARN-2017
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: resourcemanager
> Reporter: Jian He
> Assignee: Jian He
> Attachments: YARN-2017.1.patch
>
>
> A bunch of same code is repeated among schedulers, e.g: between
> FicaSchedulerNode and FSSchedulerNode. It's good to merge and share them in a
> common base.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
