goiri commented on code in PR #4656:
URL: https://github.com/apache/hadoop/pull/4656#discussion_r935933008
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/policies/router/TestPriorityRouterPolicy.java:
##########
@@ -54,10 +55,11 @@ public void setUp() throws Exception {
// with 5% omit a subcluster
if (getRand().nextFloat() < 0.95f || i == 5) {
- SubClusterInfo sci = mock(SubClusterInfo.class);
- when(sci.getState()).thenReturn(SubClusterState.SC_RUNNING);
- when(sci.getSubClusterId()).thenReturn(sc.toId());
- getActiveSubclusters().put(sc.toId(), sci);
+ long now = Time.now();
+ SubClusterInfo federationSubClusterInfo =
Review Comment:
Not sure the indetation is correct.
Let's do:
```
SubClusterInfo federationSubClusterInfo = SubClusterInfo.newInstance(
sc.toId(), "dns1:80", "dns1:81", "dns1:82", "dns1:83",
now - 1000, SubClusterState.SC_RUNNING, now - 2000,
generateClusterMetricsInfo(i));
```
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/GetSubClustersInfoResponse.java:
##########
@@ -43,6 +44,16 @@ public static GetSubClustersInfoResponse newInstance(
return subClusterInfos;
}
+ @Public
+ @Unstable
+ public static GetSubClustersInfoResponse newInstance(
+ Collection<SubClusterInfo> subClusters) {
+ GetSubClustersInfoResponse subClusterInfos =
+ Records.newRecord(GetSubClustersInfoResponse.class);
Review Comment:
Indentation doesn't seem correct.
I think it even fits in one line.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/utils/FederationPoliciesTestUtil.java:
##########
@@ -159,6 +159,50 @@ public static void initializePolicyContext(
new Configuration());
}
+ public static FederationPolicyInitializationContext initializePolicyContext2(
+ ConfigurableFederationPolicy policy, WeightedPolicyInfo policyInfo,
+ Map<SubClusterId, SubClusterInfo> activeSubClusters,
+ FederationStateStoreFacade facade) throws YarnException {
+ FederationPolicyInitializationContext context =
+ new FederationPolicyInitializationContext(null, initResolver(), facade,
+ SubClusterId.newInstance("homesubcluster"));
+ return initializePolicyContext2(context, policy, policyInfo,
activeSubClusters);
+ }
+
+ public static FederationPolicyInitializationContext initializePolicyContext2(
+ ConfigurableFederationPolicy policy, WeightedPolicyInfo policyInfo,
+ Map<SubClusterId, SubClusterInfo> activeSubClusters)
+ throws YarnException {
+ return initializePolicyContext2(policy, policyInfo, activeSubClusters,
initFacade());
+ }
+
+ public static FederationPolicyInitializationContext initializePolicyContext2(
+ FederationPolicyInitializationContext fpc,
+ ConfigurableFederationPolicy policy, WeightedPolicyInfo policyInfo,
+ Map<SubClusterId, SubClusterInfo> activeSubClusters)
+ throws YarnException {
+ ByteBuffer buf = policyInfo.toByteBuffer();
+ fpc.setSubClusterPolicyConfiguration(SubClusterPolicyConfiguration
+ .newInstance("queue1", policy.getClass().getCanonicalName(), buf));
+
+ if (fpc.getFederationStateStoreFacade() == null) {
+ FederationStateStoreFacade facade =
FederationStateStoreFacade.getInstance();
+ FederationStateStore fss = mock(FederationStateStore.class);
+
+ if (activeSubClusters == null) {
+ activeSubClusters = new HashMap<>();
+ }
+ GetSubClustersInfoResponse response =
Review Comment:
One line?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/policies/BaseFederationPoliciesTest.java:
##########
@@ -177,11 +185,64 @@ public void setHomeSubCluster(SubClusterId
homeSubCluster) {
public void setMockActiveSubclusters(int numSubclusters) {
for (int i = 1; i <= numSubclusters; i++) {
SubClusterIdInfo sc = new SubClusterIdInfo("sc" + i);
- SubClusterInfo sci = mock(SubClusterInfo.class);
- when(sci.getState()).thenReturn(SubClusterState.SC_RUNNING);
- when(sci.getSubClusterId()).thenReturn(sc.toId());
+ SubClusterInfo sci = SubClusterInfo.newInstance(sc.toId(),
+ "dns1:80", "dns1:81", "dns1:82", "dns1:83",
SubClusterState.SC_RUNNING,
+ System.currentTimeMillis(), "something");
getActiveSubclusters().put(sc.toId(), sci);
}
}
+ public String generateClusterMetricsInfo(int id) {
+ long mem = 1024 * getRand().nextInt(277 * 100 - 1);
+ // plant a best cluster
+ if (id == 5) {
+ mem = 1024 * 277 * 100;
+ }
+ String clusterMetrics =
Review Comment:
The indentation looks too aggressive, and you could use the 100 chars limit
better.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/GetSubClustersInfoResponse.java:
##########
@@ -63,4 +74,13 @@ public static GetSubClustersInfoResponse newInstance(
@Unstable
public abstract void setSubClusters(List<SubClusterInfo> subClusters);
+ /**
+ * Set the Collection of {@link SubClusterInfo} representing the information
about
+ * all sub-clusters that are currently participating in Federation.
+ *
+ * @param subClusters the list of {@link SubClusterInfo}
+ */
+ @Private
+ @Unstable
+ public abstract void setSubClusters(Collection<SubClusterInfo> subClusters);
Review Comment:
I don't think we need to add a new one, we can just replace the List one.
As the type is more generic, we should have this be backwards compatible.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/policies/router/TestWeightedRandomRouterPolicy.java:
##########
@@ -68,10 +67,12 @@ public void configureWeights(float numSubClusters) {
SubClusterIdInfo sc = new SubClusterIdInfo("sc" + i);
// with 5% omit a subcluster
if (getRand().nextFloat() < 0.95f) {
- SubClusterInfo sci = mock(SubClusterInfo.class);
- when(sci.getState()).thenReturn(SubClusterState.SC_RUNNING);
- when(sci.getSubClusterId()).thenReturn(sc.toId());
- getActiveSubclusters().put(sc.toId(), sci);
+ long now = Time.now();
+ SubClusterInfo federationSubClusterInfo =
Review Comment:
Same as before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
