Carlo Curino commented on YARN-5324:

[~subru], thanks for the good review. I think I have addressed all your 
concerns. Below I answer your questions, or explain some of the choices made in 
addressing your comments. 

Regarding {{SubClusterIdInfo}}
 # Done 
Regarding {{WeightedPolicyInfo}}
 # I addressed all comments with one exception: marshaller and unmarshallers 
cannot be preinitialized, but {{JSONJAXBContext}} can.
Regarding {{BaseWeightedRouterPolicy}}:
 # the policyInfo check is correct I think (if the current info is not null and 
is the same as the new one than nothing should be reinitialized)
 # for a router policy we don't care about what happens for amrmproxy weights.
 # I moved the checks in a validate method, but I think it is a bit redundant 
at the moment (the only class that would use it is {{LoadBasedRaouterPolicy}} 
which it doesn't because it would be clumsy).
 # I don't think adding {{selectSubCluster}} changes things much. The general 
policy API we are implementing is pretty much the same I think. 
 # check for active subclusters is indeed somewhat repeated, though some 
policies (e.g, {{UniformRandomRouterPolicy}}) use it in different ways.
Regarding {{LoadBasedRaouterPolicy}}
 # I think we should repeat the available memory check every time as this might 
change based on the latest set of info the FederationStateStoreFacade is 
returning for the activeSubclusters
Regarding {{OrderRouterPolicy}}
  # Agreed to rename this to {{PriorityRouterPolicy}}. This simple policy is 
very good for testing (no randomness), and can also provide simple failover 
type of semantics "I want jobs in this queue to run on subcluster 1, and if 
that is not available then use subcluster 2, etc.."
 Regarding "Common across tests":
 # 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 (e.g., like we do in 
 # Factored out all common parts, good advice (cut 10kb and increased coverage).
 # 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). 

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

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