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 -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
