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

ASF GitHub Bot commented on YARN-11236:
---------------------------------------

goiri commented on code in PR #4711:
URL: https://github.com/apache/hadoop/pull/4711#discussion_r939549105


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/ReservationHomeSubCluster.java:
##########
@@ -0,0 +1,124 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership.  The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations 
under
+ * the License.
+ */
+
+package org.apache.hadoop.yarn.server.federation.store.records;
+
+import org.apache.hadoop.classification.InterfaceAudience.Private;
+import org.apache.hadoop.classification.InterfaceAudience.Public;
+import org.apache.hadoop.classification.InterfaceStability.Unstable;
+import org.apache.hadoop.yarn.api.records.ReservationId;
+import org.apache.hadoop.yarn.util.Records;
+
+/**
+ * <p>
+ * ReservationHomeSubCluster is a report of the runtime information of the
+ * reservation that is running in the federated cluster.
+ *
+ * <p>
+ * It includes information such as:
+ * <ul>
+ * <li>{@link ReservationId}</li>
+ * <li>{@link SubClusterId}</li>
+ * </ul>
+ *
+ */
+@Private
+@Unstable
+public abstract class ReservationHomeSubCluster {
+
+  @Private
+  @Unstable
+  public static ReservationHomeSubCluster newInstance(ReservationId appId,
+      SubClusterId homeSubCluster) {
+    ReservationHomeSubCluster appMapping =
+        Records.newRecord(ReservationHomeSubCluster.class);
+    appMapping.setReservationId(appId);
+    appMapping.setHomeSubCluster(homeSubCluster);
+    return appMapping;
+  }
+
+  /**
+   * Get the {@link ReservationId} representing the unique identifier of the
+   * Reservation.
+   *
+   * @return the reservation identifier
+   */
+  @Public
+  @Unstable
+  public abstract ReservationId getReservationId();
+
+  /**
+   * Set the {@link ReservationId} representing the unique identifier of the
+   * Reservation.
+   *
+   * @param reservationId the reservation identifier
+   */
+  @Private
+  @Unstable
+  public abstract void setReservationId(ReservationId reservationId);
+
+  /**
+   * Get the {@link SubClusterId} representing the unique identifier of the 
home
+   * subcluster in which the reservation is mapped to.
+   *
+   * @return the home subcluster identifier
+   */
+  @Public
+  @Unstable
+  public abstract SubClusterId getHomeSubCluster();
+
+  /**
+   * Set the {@link SubClusterId} representing the unique identifier of the 
home
+   * subcluster in which the ReservationMaster of the reservation is running.
+   *
+   * @param homeSubCluster the home subcluster identifier
+   */
+  @Private
+  @Unstable
+  public abstract void setHomeSubCluster(SubClusterId homeSubCluster);
+
+  @Override
+  public boolean equals(Object obj) {
+    if (this == obj) {

Review Comment:
   EqualsBuilder and HashBuilder?



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/MemoryFederationStateStore.java:
##########
@@ -312,4 +324,39 @@ public Version loadVersion() {
     return null;
   }
 
+  @Override
+  public AddReservationHomeSubClusterResponse addReservationHomeSubCluster(
+      AddReservationHomeSubClusterRequest request) throws YarnException {
+    FederationReservationHomeSubClusterStoreInputValidator.validate(request);
+    ReservationId reservationId =
+        request.getReservationHomeSubCluster().getReservationId();
+    if (!reservations.containsKey(reservationId)) {
+      reservations.put(reservationId,
+          request.getReservationHomeSubCluster().getHomeSubCluster());
+    }
+    return 
AddReservationHomeSubClusterResponse.newInstance(reservations.get(reservationId));
+  }
+
+  @Override
+  public GetReservationHomeSubClusterResponse getReservationHomeSubCluster(
+      GetReservationHomeSubClusterRequest request) throws YarnException {
+    FederationReservationHomeSubClusterStoreInputValidator.validate(request);
+    ReservationId reservationId = request.getReservationId();
+    if (!reservations.containsKey(reservationId)) {
+      throw new YarnException("Reservation " + reservationId + " does not 
exist");
+    }
+    return GetReservationHomeSubClusterResponse.newInstance(
+        ReservationHomeSubCluster.newInstance(reservationId, 
reservations.get(reservationId)));
+  }
+
+  @Override
+  public GetReservationsHomeSubClusterResponse getReservationsHomeSubCluster(
+      GetReservationsHomeSubClusterRequest request) throws YarnException {
+    List<ReservationHomeSubCluster> result = new ArrayList<>();
+    for (Entry<ReservationId, SubClusterId> e : reservations.entrySet()) {
+      result.add(ReservationHomeSubCluster.newInstance(e.getKey(), 
e.getValue()));

Review Comment:
   Extract key and value.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/MemoryFederationStateStore.java:
##########
@@ -312,4 +324,39 @@ public Version loadVersion() {
     return null;
   }
 
+  @Override
+  public AddReservationHomeSubClusterResponse addReservationHomeSubCluster(
+      AddReservationHomeSubClusterRequest request) throws YarnException {
+    FederationReservationHomeSubClusterStoreInputValidator.validate(request);
+    ReservationId reservationId =
+        request.getReservationHomeSubCluster().getReservationId();
+    if (!reservations.containsKey(reservationId)) {
+      reservations.put(reservationId,
+          request.getReservationHomeSubCluster().getHomeSubCluster());
+    }
+    return 
AddReservationHomeSubClusterResponse.newInstance(reservations.get(reservationId));
+  }
+
+  @Override
+  public GetReservationHomeSubClusterResponse getReservationHomeSubCluster(
+      GetReservationHomeSubClusterRequest request) throws YarnException {
+    FederationReservationHomeSubClusterStoreInputValidator.validate(request);
+    ReservationId reservationId = request.getReservationId();
+    if (!reservations.containsKey(reservationId)) {
+      throw new YarnException("Reservation " + reservationId + " does not 
exist");
+    }
+    return GetReservationHomeSubClusterResponse.newInstance(

Review Comment:
   extractreservations.get(reservationId))



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/ZookeeperFederationStateStore.java:
##########
@@ -637,4 +643,22 @@ private static long getCurrentTime() {
     Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
     return cal.getTimeInMillis();
   }
+
+  @Override
+  public AddReservationHomeSubClusterResponse addReservationHomeSubCluster(
+      AddReservationHomeSubClusterRequest request) throws YarnException {
+    return null;
+  }
+
+  @Override
+  public GetReservationHomeSubClusterResponse getReservationHomeSubCluster(
+      GetReservationHomeSubClusterRequest request) throws YarnException {
+    return null;

Review Comment:
   Should we throw the not implemented exception?



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/policies/router/BaseRouterPoliciesTest.java:
##########
@@ -126,4 +134,55 @@ public void testNullReservationContext() throws Exception {
         () -> policy.getReservationHomeSubcluster(null));
   }
 
+  @Test
+  public void testUnknownReservation() throws Exception {
+    ReservationSubmissionRequest resReq = getReservationSubmissionRequest();
+    ReservationId reservationId = 
ReservationId.newInstance(System.currentTimeMillis(), 1);
+    when(resReq.getQueue()).thenReturn("queue1");
+    when(resReq.getReservationId()).thenReturn(reservationId);
+
+    // route an application that uses this app
+    ApplicationSubmissionContext applicationSubmissionContext =
+        ApplicationSubmissionContext.newInstance(
+            ApplicationId.newInstance(System.currentTimeMillis(), 1), "app1",
+            "queue1", Priority.newInstance(1), null, false, false, 1, null, 
null, false);
+
+    applicationSubmissionContext.setReservationID(resReq.getReservationId());
+    FederationRouterPolicy policy = (FederationRouterPolicy) getPolicy();
+
+    LambdaTestUtils.intercept(YarnException.class,
+        "Reservation " + reservationId + " does not exist",
+        () -> policy.getHomeSubcluster(applicationSubmissionContext, new 
ArrayList<>()));
+  }
+
+  @Test
+  public void testFollowReservation() throws YarnException {
+    ReservationSubmissionRequest resReq = getReservationSubmissionRequest();
+    when(resReq.getQueue()).thenReturn("queue1");
+    when(resReq.getReservationId())
+        .thenReturn(ReservationId.newInstance(System.currentTimeMillis(), 1));
+
+    // first we invoke a reservation placement
+    SubClusterId chosen = ((FederationRouterPolicy) getPolicy())
+        .getReservationHomeSubcluster(resReq);
+
+    // add this to the store
+    this.getFederationPolicyContext().getFederationStateStoreFacade()
+        .addReservationHomeSubCluster(ReservationHomeSubCluster.newInstance(
+             resReq.getReservationId(), chosen));
+
+    // route an application that uses this app
+    ApplicationSubmissionContext applicationSubmissionContext =
+        ApplicationSubmissionContext.newInstance(
+            ApplicationId.newInstance(System.currentTimeMillis(), 1), "app1",

Review Comment:
   Kind of out of scope but should we use Time.now()?



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/policies/router/BaseRouterPoliciesTest.java:
##########
@@ -126,4 +134,55 @@ public void testNullReservationContext() throws Exception {
         () -> policy.getReservationHomeSubcluster(null));
   }
 
+  @Test
+  public void testUnknownReservation() throws Exception {
+    ReservationSubmissionRequest resReq = getReservationSubmissionRequest();
+    ReservationId reservationId = 
ReservationId.newInstance(System.currentTimeMillis(), 1);
+    when(resReq.getQueue()).thenReturn("queue1");
+    when(resReq.getReservationId()).thenReturn(reservationId);
+
+    // route an application that uses this app
+    ApplicationSubmissionContext applicationSubmissionContext =
+        ApplicationSubmissionContext.newInstance(
+            ApplicationId.newInstance(System.currentTimeMillis(), 1), "app1",
+            "queue1", Priority.newInstance(1), null, false, false, 1, null, 
null, false);
+
+    applicationSubmissionContext.setReservationID(resReq.getReservationId());
+    FederationRouterPolicy policy = (FederationRouterPolicy) getPolicy();
+
+    LambdaTestUtils.intercept(YarnException.class,
+        "Reservation " + reservationId + " does not exist",
+        () -> policy.getHomeSubcluster(applicationSubmissionContext, new 
ArrayList<>()));
+  }
+
+  @Test
+  public void testFollowReservation() throws YarnException {
+    ReservationSubmissionRequest resReq = getReservationSubmissionRequest();
+    when(resReq.getQueue()).thenReturn("queue1");
+    when(resReq.getReservationId())
+        .thenReturn(ReservationId.newInstance(System.currentTimeMillis(), 1));
+
+    // first we invoke a reservation placement
+    SubClusterId chosen = ((FederationRouterPolicy) getPolicy())
+        .getReservationHomeSubcluster(resReq);
+
+    // add this to the store
+    this.getFederationPolicyContext().getFederationStateStoreFacade()
+        .addReservationHomeSubCluster(ReservationHomeSubCluster.newInstance(
+             resReq.getReservationId(), chosen));
+
+    // route an application that uses this app
+    ApplicationSubmissionContext applicationSubmissionContext =
+        ApplicationSubmissionContext.newInstance(
+            ApplicationId.newInstance(System.currentTimeMillis(), 1), "app1",
+            "queue1", Priority.newInstance(1), null, false, false, 1, null,
+            null, false);
+    applicationSubmissionContext.setReservationID(resReq.getReservationId());
+    SubClusterId chosen2 = ((FederationRouterPolicy) getPolicy())

Review Comment:
   Let's extract the router policy.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/impl/MemoryFederationStateStore.java:
##########
@@ -312,4 +324,39 @@ public Version loadVersion() {
     return null;
   }
 
+  @Override
+  public AddReservationHomeSubClusterResponse addReservationHomeSubCluster(
+      AddReservationHomeSubClusterRequest request) throws YarnException {
+    FederationReservationHomeSubClusterStoreInputValidator.validate(request);
+    ReservationId reservationId =
+        request.getReservationHomeSubCluster().getReservationId();
+    if (!reservations.containsKey(reservationId)) {
+      reservations.put(reservationId,
+          request.getReservationHomeSubCluster().getHomeSubCluster());
+    }
+    return 
AddReservationHomeSubClusterResponse.newInstance(reservations.get(reservationId));
+  }
+
+  @Override
+  public GetReservationHomeSubClusterResponse getReservationHomeSubCluster(
+      GetReservationHomeSubClusterRequest request) throws YarnException {
+    FederationReservationHomeSubClusterStoreInputValidator.validate(request);
+    ReservationId reservationId = request.getReservationId();
+    if (!reservations.containsKey(reservationId)) {
+      throw new YarnException("Reservation " + reservationId + " does not 
exist");
+    }
+    return GetReservationHomeSubClusterResponse.newInstance(
+        ReservationHomeSubCluster.newInstance(reservationId, 
reservations.get(reservationId)));
+  }
+
+  @Override
+  public GetReservationsHomeSubClusterResponse getReservationsHomeSubCluster(
+      GetReservationsHomeSubClusterRequest request) throws YarnException {
+    List<ReservationHomeSubCluster> result = new ArrayList<>();
+    for (Entry<ReservationId, SubClusterId> e : reservations.entrySet()) {
+      result.add(ReservationHomeSubCluster.newInstance(e.getKey(), 
e.getValue()));

Review Comment:
   and the reservation actually



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/utils/FederationStateStoreFacade.java:
##########
@@ -400,6 +406,40 @@ public Configuration getConf() {
     return this.conf;
   }
 
+
+
+  /**
+   * Adds the home {@link SubClusterId} for the specified {@link 
ReservationId}.
+   *
+   * @param appHomeSubCluster the mapping of the reservation to it's home
+   *          sub-cluster
+   * @return the stored Subcluster from StateStore
+   * @throws YarnException if the call to the state store is unsuccessful
+   */
+  public SubClusterId addReservationHomeSubCluster(
+      ReservationHomeSubCluster appHomeSubCluster) throws YarnException {
+    AddReservationHomeSubClusterResponse response =
+        stateStore.addReservationHomeSubCluster(
+        AddReservationHomeSubClusterRequest.newInstance(appHomeSubCluster));
+    return response.getHomeSubCluster();
+  }
+
+  /**
+   * Returns the home {@link SubClusterId} for the specified
+   * {@link ReservationId}.
+   *
+   * @param reservationId the identifier of the reservation
+   * @return the home sub cluster identifier
+   * @throws YarnException if the call to the state store is unsuccessful
+   */
+  public SubClusterId getReservationHomeSubCluster(ReservationId reservationId)
+      throws YarnException {
+    GetReservationHomeSubClusterResponse response =
+        stateStore.getReservationHomeSubCluster(

Review Comment:
   Indentation





> Implement FederationReservationHomeSubClusterStore With MemoryStore
> -------------------------------------------------------------------
>
>                 Key: YARN-11236
>                 URL: https://issues.apache.org/jira/browse/YARN-11236
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: federation
>    Affects Versions: 3.4.0
>            Reporter: fanshilun
>            Assignee: fanshilun
>            Priority: Major
>              Labels: pull-request-available
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to