[ https://issues.apache.org/jira/browse/YARN-6335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15963760#comment-15963760 ]
Jian He commented on YARN-6335: ------------------------------- Thanks Billie, looks good to me overall, few comments - I wonder why "yarn.resource.normalization.enabled" is introduced. For normalization, if it is an invalid value, i.e. if it's larger than max or less than zero, I think it should fail. Otherwise, yarn anyways cannot allocate the resources. Is there a use case we want it to not fail ? - In SliderClient#actionDestroy: IIUC, the last check of findAllLiveInstances cannot prevent the mentioned race condition because the create/submit call is asynchronous which takes much longer than the destroy function execution. e.g. 1) destroy deletes the appDir 2) client create an app and submit the app but app is not yet alive 3) the findAllLiveInstances will return empty 4) destroy succeeds and the create also succeeds. IMHO, given that there's anyways no guarantee on the semantics and it's hard to do so, also this situation rarely happens, the last check can be removed considering that findAllLiveInstances is not a low-cost call. We can document about this senario. - ServiceLauncherBaseTest is just an empty class. can it be removed ? or there's any purpose of it ? - The test package structure can be cleaned up a bit: e.g. org.apache.slider.test and org.apache.slider.tools may be put together and named as org.apache.slider.utils it's unclear what other package means: "org.apache.slider.other", a better name ? Looks like most of the tests are testing individual method only, we may need to add some end-to-end tests at some point. > Port slider's groovy unit tests to yarn native services > ------------------------------------------------------- > > Key: YARN-6335 > URL: https://issues.apache.org/jira/browse/YARN-6335 > Project: Hadoop YARN > Issue Type: Sub-task > Reporter: Billie Rinaldi > Assignee: Billie Rinaldi > Fix For: yarn-native-services > > Attachments: YARN-6335-yarn-native-services.001.patch, > YARN-6335-yarn-native-services.002.patch, > YARN-6335-yarn-native-services.003.patch, > YARN-6335-yarn-native-services.004.patch > > > Slider has a lot of useful unit tests implemented in groovy. We could convert > these to Java for YARN native services. This scope of this ticket will > include unit / minicluster tests only and will not include Slider's funtests > which require a running cluster. -- This message was sent by Atlassian JIRA (v6.3.15#6346) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org