[
https://issues.apache.org/jira/browse/YARN-10623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17290235#comment-17290235
]
Peter Bacsko commented on YARN-10623:
-------------------------------------
I have some minor comments.
1. {{LOG.info("Auto refreshed queue successfully!");}} The sentence "Queue
auto refresh completed successfully" sounds better.
2.
{noformat}
LOG.error("Can't refresh queue: " + e.getMessage());
...
LOG.error("Can't get file status for refresh : " + e.getMessage());
{noformat}
We don't have the stack trace. Having the stack trace is very important for
debugging, so either use {{LOG.error("Can't refresh queue", e);}} or log it
separately.
3.
{noformat}
public FileSystem getFs() {
return fs;
}
public Path getAllocCsFile() {
return allocCsFile;
}
public ResourceCalculator getResourceCalculator() {
return rc;
}
public RMContext getRmContext() {
return rmContext;
}
public CapacityScheduler getScheduler() {
return scheduler;
}
{noformat}
Are these methods used? To me it looks like that not even the test code calls
these methods. So remove those which are unused.
4.
{noformat}
try {
Thread.sleep(3000);
} catch (Exception e) {
// do nothing
}
{noformat}
Just as I mentioned in a different review, we should refrain from
{{Thread.sleep()}}. It unnecessarily slows down the test.
Use {{GenerticTestUtils.waitFor()}}.
5.
{noformat}
try {
rm = new MockRM(configuration);
rm.init(configuration);
rm.start();
} catch(Exception ex) {
fail("Should not get any exceptions");
}
{noformat}
You don't have to catch the exceptions from MockRM. If it fails, the test fails
anyway. In this case, it will be counted as a failed test. But if it cannot
start, that's really a test error, which is a separate counter in JUnit. Just
remove the try-catch block.
> Capacity scheduler should support refresh queue automatically by a thread
> policy.
> ---------------------------------------------------------------------------------
>
> Key: YARN-10623
> URL: https://issues.apache.org/jira/browse/YARN-10623
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: capacity scheduler
> Reporter: Qi Zhu
> Assignee: Qi Zhu
> Priority: Major
> Attachments: YARN-10623.001.patch, YARN-10623.002.patch,
> YARN-10623.003.patch
>
>
> In fair scheduler, it is supported that refresh queue related conf
> automatically by a thread to reload, but in capacity scheduler we only
> support to refresh queue related changes by refreshQueues, it is needed for
> our cluster to realize queue manage.
> cc [~wangda] [~ztang] [~pbacsko] [~snemeth] [~gandras] [~bteke] [~shuzirra]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
