[ 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