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

Subru Krishnan commented on YARN-5324:
--------------------------------------

Thanks [~curino] for addressing my comments. 

The patch looks very close, have a few follow up comments:
  * {{PriorityRouterPolicy}} seems to be missing in the latest version.
  * Are we handling the null case for *policyInfo* in 
{{BaseWeightedRouterPolicy}}?

  
  * bq. check for active subclusters is indeed somewhat repeated
  In that case, we should have a base version in  {{BaseWeightedRouterPolicy}} 
which others can override in case they have acustom logic.

  
  * The suggestion of adding *selectSubCluster* is not for API purposes but 
purely for readability as every _RouterPolicy_ has the same pattern.
  * Rename {{BaseFederationPoliciesTest}} to 
{{BaseFederationRouterPoliciesTest}}
  * Why can't we move *testNoSubclusters* to 
{{BaseFederationRouterPoliciesTest}}?

  
  * bq. In all/most tests the set of "activeSubclusters" is chosen to be a 
subset of the one specified in the policy weights. All policies are basically 
stateless, previous decisions should not affect following ones so the multi 
invocation tests are only relevant if we check statistical properties 
  IIUC then, the Javadocs _Generate large number of randomized tests_ in tests 
seem misleading, can you update.


  * bq. Some of the method in FederationPoliciesTestUtil are used by the 
upcoming patches for AMRMProxy (I was trying to avoid editing that class over 
and over at every patch).
  We should _only_ have related changes in the patch. Editing same files 
incrementally over multiple patches is the norm as otherwise we will loose 
track of provenance which is required for selective cherry-picking, roll-backs 
etc.



> Stateless router policies implementation
> ----------------------------------------
>
>                 Key: YARN-5324
>                 URL: https://issues.apache.org/jira/browse/YARN-5324
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: nodemanager, resourcemanager
>    Affects Versions: YARN-2915
>            Reporter: Carlo Curino
>            Assignee: Carlo Curino
>         Attachments: YARN-5324-YARN-2915.06.patch, 
> YARN-5324-YARN-2915.07.patch, YARN-5324-YARN-2915.08.patch, 
> YARN-5324-YARN-2915.09.patch, YARN-5324-YARN-2915.10.patch, 
> YARN-5324-YARN-2915.11.patch, YARN-5324-YARN-2915.12.patch, 
> YARN-5324-YARN-2915.13.patch, YARN-5324.01.patch, YARN-5324.02.patch, 
> YARN-5324.03.patch, YARN-5324.04.patch, YARN-5324.05.patch
>
>
> These are policies at the Router that do not require maintaing state across 
> choices (e.g., weighted random).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
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