goiri commented on a change in pull request #3754:
URL: https://github.com/apache/hadoop/pull/3754#discussion_r764221624



##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ClusterMetrics.java
##########
@@ -117,13 +121,25 @@ public static ClusterMetrics getMetrics() {
         if(INSTANCE == null){
           INSTANCE = new ClusterMetrics();
           registerMetrics();
+          INSTANCE.initialize();
           isInitialized.set(true);
         }
       }
     }
     return INSTANCE;
   }
 
+  private void initialize() {
+    allocateLatencyGuarQuantiles = registry

Review comment:
       For readability, I think you can split as:
   ```
   allocateLatencyGuarQuantiles = registry.newQuantiles(
       "AllocateLatencyGuaranteed",
       "Latency to fulfill an Allocate(Guaranteed) requests", "ops",
        "latency", 5);
   ```

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptMetrics.java
##########
@@ -242,4 +253,149 @@ public Resource getApplicationAttemptHeadroom() {
   public void setApplicationAttemptHeadRoom(Resource headRoom) {
     this.applicationHeadroom = headRoom;
   }
+
+  /**
+   * Add allocationID latency to the application ID with a timestap =
+   * CurrentTime (guaranteed)
+   *
+   * @param allocId the allocation Id to add If the allocation ID is already
+   *                present (which shouldn't happen) it ignores the entry
+   */
+  public void addAllocationGuarLatencyIfNotExists(long allocId) {
+    addAllocationGuarLatencyIfNotExists(allocId, System.currentTimeMillis());
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a specific timestamp
+   * (guaranteed)
+   *
+   * @param allocId   allocationId
+   * @param timestamp the timestamp to associate
+   */
+  public void addAllocationGuarLatencyIfNotExists(long allocId,
+      long timestamp) {
+    allocationGuaranteedLatencies.putIfAbsent(allocId, timestamp);
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a timestap =
+   * CurrentTime (opportunistic)
+   *
+   * @param allocId the allocation Id to add If the allocation ID is already
+   *                present (which shouldn't happen) it ignores the entry
+   */
+  public void addAllocationOppLatencyIfNotExists(long allocId) {
+    this.addAllocationOppLatencyIfNotExists(allocId,
+        System.currentTimeMillis());
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a specific timestamp
+   * (opportunistic)
+   *
+   * @param allocId   allocationId
+   * @param timestamp the timestamp to associate
+   */
+  public void addAllocationOppLatencyIfNotExists(long allocId, long timestamp) 
{
+    allocationOpportunisticLatencies.putIfAbsent(allocId, timestamp);
+  }
+
+  /**
+   * Returns the time associated when the allocation Id was added This method
+   * removes the allocation Id from the class (guaranteed)
+   *
+   * @param allocId the allocation ID to get the associated time
+   * @return the timestamp associated with that allocation id as well as stop
+   * tracking it
+   */
+  public long getAndRemoveGuaAllocationLatencies(long allocId) {
+    Long ret = allocationGuaranteedLatencies.remove(new Long(allocId));
+    return ret != null ? ret : 0l;
+  }
+
+  /**
+   * Returns the time associated when the allocation Id was added This method
+   * removes the allocation Id from the class (opportunistic)
+   *
+   * @param allocId the allocation ID to get the associated time
+   * @return the timestamp associated with that allocation id as well as stop
+   * tracking it
+   */
+  public long getAndRemoveOppAllocationLatencies(long allocId) {
+    Long ret = allocationOpportunisticLatencies.remove(new Long(allocId));

Review comment:
       There's a deprecation and I think it might be enough to pass allocId 
without creating it.

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptMetrics.java
##########
@@ -242,4 +253,149 @@ public Resource getApplicationAttemptHeadroom() {
   public void setApplicationAttemptHeadRoom(Resource headRoom) {
     this.applicationHeadroom = headRoom;
   }
+
+  /**
+   * Add allocationID latency to the application ID with a timestap =
+   * CurrentTime (guaranteed)
+   *
+   * @param allocId the allocation Id to add If the allocation ID is already
+   *                present (which shouldn't happen) it ignores the entry
+   */
+  public void addAllocationGuarLatencyIfNotExists(long allocId) {
+    addAllocationGuarLatencyIfNotExists(allocId, System.currentTimeMillis());
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a specific timestamp
+   * (guaranteed)
+   *
+   * @param allocId   allocationId
+   * @param timestamp the timestamp to associate
+   */
+  public void addAllocationGuarLatencyIfNotExists(long allocId,
+      long timestamp) {
+    allocationGuaranteedLatencies.putIfAbsent(allocId, timestamp);
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a timestap =
+   * CurrentTime (opportunistic)
+   *
+   * @param allocId the allocation Id to add If the allocation ID is already
+   *                present (which shouldn't happen) it ignores the entry
+   */
+  public void addAllocationOppLatencyIfNotExists(long allocId) {
+    this.addAllocationOppLatencyIfNotExists(allocId,
+        System.currentTimeMillis());
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a specific timestamp
+   * (opportunistic)
+   *
+   * @param allocId   allocationId
+   * @param timestamp the timestamp to associate
+   */
+  public void addAllocationOppLatencyIfNotExists(long allocId, long timestamp) 
{
+    allocationOpportunisticLatencies.putIfAbsent(allocId, timestamp);
+  }
+
+  /**
+   * Returns the time associated when the allocation Id was added This method
+   * removes the allocation Id from the class (guaranteed)
+   *
+   * @param allocId the allocation ID to get the associated time
+   * @return the timestamp associated with that allocation id as well as stop
+   * tracking it
+   */
+  public long getAndRemoveGuaAllocationLatencies(long allocId) {
+    Long ret = allocationGuaranteedLatencies.remove(new Long(allocId));
+    return ret != null ? ret : 0l;
+  }
+
+  /**
+   * Returns the time associated when the allocation Id was added This method
+   * removes the allocation Id from the class (opportunistic)
+   *
+   * @param allocId the allocation ID to get the associated time
+   * @return the timestamp associated with that allocation id as well as stop
+   * tracking it
+   */
+  public long getAndRemoveOppAllocationLatencies(long allocId) {
+    Long ret = allocationOpportunisticLatencies.remove(new Long(allocId));
+    return ret != null ? ret : 0l;
+  }
+
+  /**
+   * Set timestamp for the provided ResourceRequest. It will correctly identify
+   * their ExecutionType, provided they have they have allocateId != 0 
(DEFAULT)
+   * This is used in conjunction with This is used in conjunction with
+   * updatePromoteLatencies method method
+   *
+   * @param requests the ResourceRequests to add.
+   */
+  public void setAllocateLatenciesTimestamps(List<ResourceRequest> requests) {
+    long now = Time.now();
+    for (ResourceRequest req : requests) {
+      if (req.getNumContainers() > 0) {
+        // we dont support tracking with negative or zero allocationIds
+        long allocationRequestId = req.getAllocationRequestId();
+        if (allocationRequestId > 0) {
+          if (req.getExecutionTypeRequest() != null) {
+            if (ExecutionType.GUARANTEED

Review comment:
       Extract req.getExecutionTypeRequest() so the lines get shorter.

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptMetrics.java
##########
@@ -63,13 +69,18 @@
       new int[NodeType.values().length][NodeType.values().length];
   private volatile int totalAllocatedContainers;
 
+  private ConcurrentHashMap<Long, Long> allocationGuaranteedLatencies = null;

Review comment:
       Can you leave it without the null setting and make it final so the 
constructor sets it and that's it?

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java
##########
@@ -472,6 +485,23 @@ public AllocateResponse allocate(AllocateRequest request)
     }
   }
 
+  protected RMAppAttemptMetrics getAppAttemptMetrics(
+      ApplicationAttemptId appAttemptId) {
+    if (appAttemptId == null) {
+      return null;
+    }
+    RMApp app = 
this.rmContext.getRMApps().get(appAttemptId.getApplicationId());

Review comment:
       I would extract things a little and check for nulls.

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptMetrics.java
##########
@@ -242,4 +253,149 @@ public Resource getApplicationAttemptHeadroom() {
   public void setApplicationAttemptHeadRoom(Resource headRoom) {
     this.applicationHeadroom = headRoom;
   }
+
+  /**
+   * Add allocationID latency to the application ID with a timestap =
+   * CurrentTime (guaranteed)
+   *
+   * @param allocId the allocation Id to add If the allocation ID is already
+   *                present (which shouldn't happen) it ignores the entry
+   */
+  public void addAllocationGuarLatencyIfNotExists(long allocId) {
+    addAllocationGuarLatencyIfNotExists(allocId, System.currentTimeMillis());
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a specific timestamp
+   * (guaranteed)
+   *
+   * @param allocId   allocationId
+   * @param timestamp the timestamp to associate
+   */
+  public void addAllocationGuarLatencyIfNotExists(long allocId,
+      long timestamp) {
+    allocationGuaranteedLatencies.putIfAbsent(allocId, timestamp);
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a timestap =
+   * CurrentTime (opportunistic)
+   *
+   * @param allocId the allocation Id to add If the allocation ID is already
+   *                present (which shouldn't happen) it ignores the entry
+   */
+  public void addAllocationOppLatencyIfNotExists(long allocId) {
+    this.addAllocationOppLatencyIfNotExists(allocId,
+        System.currentTimeMillis());
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a specific timestamp
+   * (opportunistic)
+   *
+   * @param allocId   allocationId
+   * @param timestamp the timestamp to associate
+   */
+  public void addAllocationOppLatencyIfNotExists(long allocId, long timestamp) 
{
+    allocationOpportunisticLatencies.putIfAbsent(allocId, timestamp);
+  }
+
+  /**
+   * Returns the time associated when the allocation Id was added This method
+   * removes the allocation Id from the class (guaranteed)
+   *
+   * @param allocId the allocation ID to get the associated time
+   * @return the timestamp associated with that allocation id as well as stop
+   * tracking it
+   */
+  public long getAndRemoveGuaAllocationLatencies(long allocId) {
+    Long ret = allocationGuaranteedLatencies.remove(new Long(allocId));
+    return ret != null ? ret : 0l;
+  }
+
+  /**
+   * Returns the time associated when the allocation Id was added This method
+   * removes the allocation Id from the class (opportunistic)
+   *
+   * @param allocId the allocation ID to get the associated time
+   * @return the timestamp associated with that allocation id as well as stop
+   * tracking it
+   */
+  public long getAndRemoveOppAllocationLatencies(long allocId) {
+    Long ret = allocationOpportunisticLatencies.remove(new Long(allocId));
+    return ret != null ? ret : 0l;
+  }
+
+  /**
+   * Set timestamp for the provided ResourceRequest. It will correctly identify
+   * their ExecutionType, provided they have they have allocateId != 0 
(DEFAULT)
+   * This is used in conjunction with This is used in conjunction with
+   * updatePromoteLatencies method method
+   *
+   * @param requests the ResourceRequests to add.
+   */
+  public void setAllocateLatenciesTimestamps(List<ResourceRequest> requests) {
+    long now = Time.now();
+    for (ResourceRequest req : requests) {
+      if (req.getNumContainers() > 0) {
+        // we dont support tracking with negative or zero allocationIds
+        long allocationRequestId = req.getAllocationRequestId();
+        if (allocationRequestId > 0) {
+          if (req.getExecutionTypeRequest() != null) {
+            if (ExecutionType.GUARANTEED
+                .equals(req.getExecutionTypeRequest().getExecutionType())) {
+              addAllocationGuarLatencyIfNotExists(allocationRequestId, now);
+            } else {
+              addAllocationOppLatencyIfNotExists(allocationRequestId, now);
+            }
+          }
+        } else {
+          LOG.warn(String.format(
+              "Can't register allocate latency for %s container "
+                  + "with negative or zero allocation IDs",
+              req.getExecutionTypeRequest().getExecutionType()));
+        }
+      }
+    }
+  }
+
+  /**
+   * Updated the JMX metrics class (ClusterMetrics) with the delta time when
+   * these containers where added. It will correctly identify their
+   * ExecutionType, provided they have they have allocateId != 0 (DEFAULT)
+   *
+   * @param response the list of the containers to allocate.
+   */
+  public void updateAllocateLatencies(List<Container> response) {
+
+    for (Container container : response) {
+      long allocationRequestId = container.getAllocationRequestId();
+      // we dont support tracking with negative or zero allocationIds
+      if (allocationRequestId > 0) {
+        long now = System.currentTimeMillis();
+        long allocIdTime =
+            (container.getExecutionType() == ExecutionType.GUARANTEED) ?
+                getAndRemoveGuaAllocationLatencies(allocationRequestId) :
+                getAndRemoveOppAllocationLatencies(allocationRequestId);
+        if (allocIdTime != 0) {
+          if (container.getExecutionType() == ExecutionType.GUARANTEED) {
+            ClusterMetrics.getMetrics()
+                .addAllocateGuarLatencyEntry(now - allocIdTime);
+          } else {
+            ClusterMetrics.getMetrics()
+                .addAllocateOppLatencyEntry(now - allocIdTime);
+          }
+        } else {
+          LOG.error(String.format(

Review comment:
       {}

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptMetrics.java
##########
@@ -242,4 +253,149 @@ public Resource getApplicationAttemptHeadroom() {
   public void setApplicationAttemptHeadRoom(Resource headRoom) {
     this.applicationHeadroom = headRoom;
   }
+
+  /**
+   * Add allocationID latency to the application ID with a timestap =
+   * CurrentTime (guaranteed)
+   *
+   * @param allocId the allocation Id to add If the allocation ID is already
+   *                present (which shouldn't happen) it ignores the entry
+   */
+  public void addAllocationGuarLatencyIfNotExists(long allocId) {
+    addAllocationGuarLatencyIfNotExists(allocId, System.currentTimeMillis());
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a specific timestamp
+   * (guaranteed)
+   *
+   * @param allocId   allocationId
+   * @param timestamp the timestamp to associate
+   */
+  public void addAllocationGuarLatencyIfNotExists(long allocId,
+      long timestamp) {
+    allocationGuaranteedLatencies.putIfAbsent(allocId, timestamp);
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a timestap =
+   * CurrentTime (opportunistic)
+   *
+   * @param allocId the allocation Id to add If the allocation ID is already
+   *                present (which shouldn't happen) it ignores the entry
+   */
+  public void addAllocationOppLatencyIfNotExists(long allocId) {
+    this.addAllocationOppLatencyIfNotExists(allocId,
+        System.currentTimeMillis());
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a specific timestamp
+   * (opportunistic)
+   *
+   * @param allocId   allocationId
+   * @param timestamp the timestamp to associate
+   */
+  public void addAllocationOppLatencyIfNotExists(long allocId, long timestamp) 
{
+    allocationOpportunisticLatencies.putIfAbsent(allocId, timestamp);
+  }
+
+  /**
+   * Returns the time associated when the allocation Id was added This method
+   * removes the allocation Id from the class (guaranteed)
+   *
+   * @param allocId the allocation ID to get the associated time
+   * @return the timestamp associated with that allocation id as well as stop
+   * tracking it
+   */
+  public long getAndRemoveGuaAllocationLatencies(long allocId) {
+    Long ret = allocationGuaranteedLatencies.remove(new Long(allocId));
+    return ret != null ? ret : 0l;
+  }
+
+  /**
+   * Returns the time associated when the allocation Id was added This method
+   * removes the allocation Id from the class (opportunistic)
+   *
+   * @param allocId the allocation ID to get the associated time
+   * @return the timestamp associated with that allocation id as well as stop
+   * tracking it
+   */
+  public long getAndRemoveOppAllocationLatencies(long allocId) {
+    Long ret = allocationOpportunisticLatencies.remove(new Long(allocId));
+    return ret != null ? ret : 0l;
+  }
+
+  /**
+   * Set timestamp for the provided ResourceRequest. It will correctly identify
+   * their ExecutionType, provided they have they have allocateId != 0 
(DEFAULT)
+   * This is used in conjunction with This is used in conjunction with
+   * updatePromoteLatencies method method
+   *
+   * @param requests the ResourceRequests to add.
+   */
+  public void setAllocateLatenciesTimestamps(List<ResourceRequest> requests) {
+    long now = Time.now();
+    for (ResourceRequest req : requests) {
+      if (req.getNumContainers() > 0) {
+        // we dont support tracking with negative or zero allocationIds
+        long allocationRequestId = req.getAllocationRequestId();
+        if (allocationRequestId > 0) {
+          if (req.getExecutionTypeRequest() != null) {
+            if (ExecutionType.GUARANTEED
+                .equals(req.getExecutionTypeRequest().getExecutionType())) {
+              addAllocationGuarLatencyIfNotExists(allocationRequestId, now);
+            } else {
+              addAllocationOppLatencyIfNotExists(allocationRequestId, now);
+            }
+          }
+        } else {
+          LOG.warn(String.format(
+              "Can't register allocate latency for %s container "
+                  + "with negative or zero allocation IDs",
+              req.getExecutionTypeRequest().getExecutionType()));
+        }
+      }
+    }
+  }
+
+  /**
+   * Updated the JMX metrics class (ClusterMetrics) with the delta time when
+   * these containers where added. It will correctly identify their
+   * ExecutionType, provided they have they have allocateId != 0 (DEFAULT)
+   *
+   * @param response the list of the containers to allocate.
+   */
+  public void updateAllocateLatencies(List<Container> response) {
+
+    for (Container container : response) {
+      long allocationRequestId = container.getAllocationRequestId();
+      // we dont support tracking with negative or zero allocationIds
+      if (allocationRequestId > 0) {
+        long now = System.currentTimeMillis();
+        long allocIdTime =
+            (container.getExecutionType() == ExecutionType.GUARANTEED) ?
+                getAndRemoveGuaAllocationLatencies(allocationRequestId) :
+                getAndRemoveOppAllocationLatencies(allocationRequestId);
+        if (allocIdTime != 0) {
+          if (container.getExecutionType() == ExecutionType.GUARANTEED) {
+            ClusterMetrics.getMetrics()
+                .addAllocateGuarLatencyEntry(now - allocIdTime);
+          } else {
+            ClusterMetrics.getMetrics()
+                .addAllocateOppLatencyEntry(now - allocIdTime);
+          }
+        } else {
+          LOG.error(String.format(
+              "Can't register allocate latency for %s container %s; 
allotTime=%d ",
+              container.getExecutionType(), container.getId(), allocIdTime));
+        }
+      } else {
+        LOG.warn(String.format("Cant register promotion latency "

Review comment:
       {}

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptMetrics.java
##########
@@ -242,4 +253,149 @@ public Resource getApplicationAttemptHeadroom() {
   public void setApplicationAttemptHeadRoom(Resource headRoom) {
     this.applicationHeadroom = headRoom;
   }
+
+  /**
+   * Add allocationID latency to the application ID with a timestap =
+   * CurrentTime (guaranteed)
+   *
+   * @param allocId the allocation Id to add If the allocation ID is already
+   *                present (which shouldn't happen) it ignores the entry
+   */
+  public void addAllocationGuarLatencyIfNotExists(long allocId) {
+    addAllocationGuarLatencyIfNotExists(allocId, System.currentTimeMillis());
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a specific timestamp
+   * (guaranteed)
+   *
+   * @param allocId   allocationId
+   * @param timestamp the timestamp to associate
+   */
+  public void addAllocationGuarLatencyIfNotExists(long allocId,
+      long timestamp) {
+    allocationGuaranteedLatencies.putIfAbsent(allocId, timestamp);
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a timestap =
+   * CurrentTime (opportunistic)
+   *
+   * @param allocId the allocation Id to add If the allocation ID is already
+   *                present (which shouldn't happen) it ignores the entry
+   */
+  public void addAllocationOppLatencyIfNotExists(long allocId) {
+    this.addAllocationOppLatencyIfNotExists(allocId,
+        System.currentTimeMillis());
+  }
+
+  /**
+   * Add allocationID latency to the application ID with a specific timestamp
+   * (opportunistic)
+   *
+   * @param allocId   allocationId
+   * @param timestamp the timestamp to associate
+   */
+  public void addAllocationOppLatencyIfNotExists(long allocId, long timestamp) 
{
+    allocationOpportunisticLatencies.putIfAbsent(allocId, timestamp);
+  }
+
+  /**
+   * Returns the time associated when the allocation Id was added This method
+   * removes the allocation Id from the class (guaranteed)
+   *
+   * @param allocId the allocation ID to get the associated time
+   * @return the timestamp associated with that allocation id as well as stop
+   * tracking it
+   */
+  public long getAndRemoveGuaAllocationLatencies(long allocId) {
+    Long ret = allocationGuaranteedLatencies.remove(new Long(allocId));
+    return ret != null ? ret : 0l;
+  }
+
+  /**
+   * Returns the time associated when the allocation Id was added This method
+   * removes the allocation Id from the class (opportunistic)
+   *
+   * @param allocId the allocation ID to get the associated time
+   * @return the timestamp associated with that allocation id as well as stop
+   * tracking it
+   */
+  public long getAndRemoveOppAllocationLatencies(long allocId) {
+    Long ret = allocationOpportunisticLatencies.remove(new Long(allocId));
+    return ret != null ? ret : 0l;
+  }
+
+  /**
+   * Set timestamp for the provided ResourceRequest. It will correctly identify
+   * their ExecutionType, provided they have they have allocateId != 0 
(DEFAULT)
+   * This is used in conjunction with This is used in conjunction with
+   * updatePromoteLatencies method method
+   *
+   * @param requests the ResourceRequests to add.
+   */
+  public void setAllocateLatenciesTimestamps(List<ResourceRequest> requests) {
+    long now = Time.now();
+    for (ResourceRequest req : requests) {
+      if (req.getNumContainers() > 0) {
+        // we dont support tracking with negative or zero allocationIds
+        long allocationRequestId = req.getAllocationRequestId();
+        if (allocationRequestId > 0) {
+          if (req.getExecutionTypeRequest() != null) {
+            if (ExecutionType.GUARANTEED
+                .equals(req.getExecutionTypeRequest().getExecutionType())) {
+              addAllocationGuarLatencyIfNotExists(allocationRequestId, now);
+            } else {
+              addAllocationOppLatencyIfNotExists(allocationRequestId, now);
+            }
+          }
+        } else {
+          LOG.warn(String.format(

Review comment:
       Can we just use {} instead of format?

##########
File path: 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java
##########
@@ -395,7 +396,19 @@ public AllocateResponse allocate(AllocateRequest request)
 
     ApplicationAttemptId appAttemptId =
         amrmTokenIdentifier.getApplicationAttemptId();
+    RMAppAttemptMetrics rmMetrics = getAppAttemptMetrics(appAttemptId);

Review comment:
       This can be null, no?




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

Reply via email to