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

Billie Rinaldi commented on YARN-8122:
--------------------------------------

Thanks for the patch, [~gsaha]! Overall the design of the feature looks good. I 
still have to do some testing of the patch, but I wanted to give you a few 
minor comments.
* Use the YarnServiceConf getInt and getLong helper methods to initialize the 
new configuration properties
* If YARN-8060 gets committed before this, please add the properties to the 
newly created Component-Level configuration section of the Configurations docs
* It would be good to test the validity of the property values in ServiceApiUtil
* Use YarnServiceConf properties instead of hardcoded strings in the test
* Test comments say 2 secs, but actual setting appears to be 3 secs
* In the log line "Health is going below threshold for the first time" I think 
"for the first time" might be confusing, since it might not be the first time 
it has happened. Perhaps something like "Health is going below threshold, 
starting health threshold timer" or something like that
* The info log "Resetting first occurence to 0" perhaps should be a debug log, 
since the fact that we are using 0 as an internal indicator that the component 
is currently healthy is an implementation detail that the user doesn't need to 
know about
* I like the log format consistency with the Component logging, using the 
prefix [Component {}]. Component uses uppercase [COMPONENT {}], so that would 
be even better

> Component health threshold monitor
> ----------------------------------
>
>                 Key: YARN-8122
>                 URL: https://issues.apache.org/jira/browse/YARN-8122
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Gour Saha
>            Assignee: Gour Saha
>            Priority: Major
>         Attachments: YARN-8122.001.patch, YARN-8122.002.patch, 
> YARN-8122.draft.patch
>
>
> Slider supported component health threshold monitoring with SLIDER-1246. It 
> would be good to have this feature for YARN Service too.



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

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

Reply via email to