[ https://issues.apache.org/jira/browse/YARN-10623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17290013#comment-17290013 ]
Gergely Pollak commented on YARN-10623: --------------------------------------- [~zhuqi] thank you for the patch! I really like the idea that the solution is a configurable extra component (scheduling edit policy), this way it is cleanly decoupled from the main code base, won't cause any issue for those who don't want to use it. I very much agree with [~gandras] that last modification check was necessary however I think it introduced a bug: {code:java} 126 if (lastModified > lastSuccessfulReload && 127 time > lastModified + monitoringInterval) { {code} So in the edge case when you update this file MORE frequently than the monitoringInterval (which is configurable, so it might be in the minute or even 10 minutes range), then you won't ever refresh the config, since last modified + monitoringInterval will be ALWAYS greater than the current time. I think you should go with {code:java} 126 if (lastModified > lastSuccessfulReload && 127 time > lastSuccessfulReload + monitoringInterval) { {code} Or even better introduce a lastReloadAttempt, since a reload can fail, and in this case it would result keep trying to reload the invalid configuration, so if you'd introduce a lastReloadAttempt and set it each time before you try to reload the configuration, then you could use {code:java} 126 if (lastModified > lastReloadAttempt && 127 time > lastReloadAttempt + monitoringInterval) { {code} This would guarantee that you don't reload more frequently than the monitoringInterval, you don't reload if the configuration hasn't been modified, but still keep reloading if the file is updated frequently. Otherwise the patch looks good to me (non-binding). > 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