[
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: [email protected]
For additional commands, e-mail: [email protected]
