[
https://issues.apache.org/jira/browse/YARN-3890?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16947624#comment-16947624
]
Adam Antal commented on YARN-3890:
----------------------------------
Overall looks very nice! Thanks for working on this.
Some comments:
- I see that {{SchedulerHealth$updateSchedulerReleaseDetails()}} is not called
by the FS after your patch. I think reporting those has also major importance.
Would you wire that as well to FS?
- I am not entirely convinced that testing is sufficient, so I suggest that
TestSchedulerHealth class should be parameterized to use FairScheduler as well
(along with CS) to ensure full integration. I'm also ok if you file a follow-up
jira and resolve it in that.
Question: Did you run it on a cluster, and verified that the HTML is rendered
properly? (One should do that before the merge of this patch.)
nits:
- Remove TODO from {{FairScheduler.java}}. Was there anything you were supposed
to put there?
- In {{TestFSAppAttempt.java}} remove wildcard import
- Decapitalize comments in {{TestFSAppAttempt$testHealthMetric}}
- Checkstyle
I also saw that {{SchedulerHealth}} class is in a really bad shape, I filed
YARN-9883 for the cleanup.
> FairScheduler should show the scheduler health metrics similar to ones added
> in CapacityScheduler
> -------------------------------------------------------------------------------------------------
>
> Key: YARN-3890
> URL: https://issues.apache.org/jira/browse/YARN-3890
> Project: Hadoop YARN
> Issue Type: Bug
> Components: fairscheduler
> Reporter: Anubhav Dhoot
> Assignee: Zoltan Siegl
> Priority: Major
> Attachments: YARN-3890.001.patch, YARN-3890.002.patch,
> YARN-3890.003.patch
>
>
> We should add information displayed in YARN-3293 in FairScheduler as well
> possibly sharing the implementation.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
