Subru Krishnan, 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.
- I addressed all comments with one exception: marshaller and unmarshallers cannot be preinitialized, but JSONJAXBContext can.
- 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.
- 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
- 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 testClusterChosenWithRightProbability)
- 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).