[jira] [Comment Edited] (YARN-9629) Support configurable MIN_LOG_ROLLING_INTERVAL

["Adam Antal (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/YARN-9629?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16877015#comment-16877015 ] 

Adam Antal edited comment on YARN-9629 at 7/2/19 2:24 PM:
----------------------------------------------------------

Thanks for the review [~sunilg].
1) 
Firstly if the user does not override default configs this message is displayed and can be concerning if WARN level logs are displayed during standard startup. Secondly I think this does not fall into WARN category. We want to inform the users about the rolling feature being disabled and not warning them about that.
Thirdly it was in INFO level log before this patch, so I'd keep it that way if there's no big reason to raise level.

Taking into account all of this I suggest to keep this message in INFO level.

About adding extra information: this is a static method and we have only a Configuration object during {{serviceInit}} anyways. That part is also not bound to a specific application, so we can not really add more information beside the NM's id (but it's the log of that NM where this message is displayed, so it does not really make sense to do that).

2) 
This information is included in other branches of that if condition. If the configuration value does not pass the minimum constraint in case of debug mode we just say that this has been set to that value, otherwise we should warn the customer that the configured value violates the minimum constraint. In either way, we provide them the information, so there's no need to display it again (so there's no need to move that last log out of that else cause).

Does these rationale make sense to you [~sunilg]?


was (Author: adam.antal):
Thanks for the review [~sunilg].
1) 
Firstly if the user does not override default configs this message is displayed and can be concerning if WARN level logs are displayed during standard startup. Secondly I think this does not falls into WARN level category. We want to inform the users about that the rolling feature is not enabled and not warning them about this.
Thirdly it was in INFO level log before this patch, so I'd keep it that way if there's no big reason to raise level.
Taking into account all of this I suggest to keep this in INFO level.

About adding an extra information: this is a static method and also we have only a Configuration object during {{serviceInit}}. That part is also not bound to a specific application, so we can not really add more information beside the NM's id (but it's the log of that NM where this message is displayed, so it does not really make sense).

2) 
This information is included in other branches of that if condition. If the configuration value does not pass the minimum constraint in case of debug mode we just say that this has been set to that value, otherwise we should warn the customer that the the configured value violates the minimum constraint. In either way, we provide the information, so there's no need to display is again (so there's no need to move that last log out of that else cause).

Does these rationale make sense to you [~sunilg]?

> Support configurable MIN_LOG_ROLLING_INTERVAL
> ---------------------------------------------
>
>                 Key: YARN-9629
>                 URL: https://issues.apache.org/jira/browse/YARN-9629
>             Project: Hadoop YARN
>          Issue Type: Improvement
>          Components: log-aggregation, nodemanager, yarn
>    Affects Versions: 3.2.0
>            Reporter: Adam Antal
>            Assignee: Adam Antal
>            Priority: Minor
>         Attachments: YARN-9629.001.patch, YARN-9629.002.patch, YARN-9629.003.patch, YARN-9629.004.patch, YARN-9629.005.patch
>
>
> One of the log-aggregation parameter, the minimum valid value for {{yarn.nodemanager.log-aggregation.roll-monitoring-interval-seconds}} is MIN_LOG_ROLLING_INTERVAL - it has been hardcoded since its addition in YARN-2583. 
> It has been empirically set as 1 hour, as lower values would too frequently put the NodeManagers under pressure. For bigger clusters that is indeed a valid limitation, but for smaller clusters it makes sense and a valid customer usecase to use lower values, even like not so lower 30 mins. At this point this can only be achieved by setting {{yarn.nodemanager.log-aggregation.debug-enabled}}, which I believe should be kept as debug purposes.
> I'm suggesting to make this min configurable, although a warning should be logged in the NodeManager startup when this value is lower than 1 hour.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org