gerashegalov commented on code in PR #8770:
URL: https://github.com/apache/hadoop/pull/8770#discussion_r4215813362


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/timeline/TimelineUtils.java:
##########
@@ -60,6 +73,73 @@ public class TimelineUtils {
     YarnJacksonJaxbJsonProvider.configObjectMapper(mapper);
   }
 
+  @Private
+  public static Map<String, Map<String, Object>> getCustomResourceInfo(
+      Resource resource) {
+    Map<String, Map<String, Object>> resources = new HashMap<>();
+    for (ResourceInformation information : resource.getResources()) {
+      String name = information.getName();
+      if (!ResourceInformation.MEMORY_URI.equals(name)
+          && !ResourceInformation.VCORES_URI.equals(name)
+          && information.getValue() != 0) {
+        Map<String, Object> allocation = new HashMap<>();
+        allocation.put("value", information.getValue());

Review Comment:
   Thanks—addressed in `d60acf7`. `value` and `units` now have shared constants 
in `ContainerMetricsConstants`, used by both the writer and reader. The stored 
field names are unchanged.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/timeline/TimelineUtils.java:
##########
@@ -60,6 +73,73 @@ public class TimelineUtils {
     YarnJacksonJaxbJsonProvider.configObjectMapper(mapper);
   }
 
+  @Private
+  public static Map<String, Map<String, Object>> getCustomResourceInfo(

Review Comment:
   Added Javadocs in `d60acf7` describing the stored map shape, omitted 
allocations, and how the reader handles unknown or malformed entries.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/timeline/TimelineUtils.java:
##########
@@ -60,6 +73,73 @@ public class TimelineUtils {
     YarnJacksonJaxbJsonProvider.configObjectMapper(mapper);
   }
 
+  @Private
+  public static Map<String, Map<String, Object>> getCustomResourceInfo(
+      Resource resource) {
+    Map<String, Map<String, Object>> resources = new HashMap<>();
+    for (ResourceInformation information : resource.getResources()) {
+      String name = information.getName();
+      if (!ResourceInformation.MEMORY_URI.equals(name)
+          && !ResourceInformation.VCORES_URI.equals(name)
+          && information.getValue() != 0) {
+        Map<String, Object> allocation = new HashMap<>();
+        allocation.put("value", information.getValue());
+        allocation.put("units", information.getUnits());
+        resources.put(name, allocation);
+      }
+    }
+    return resources;
+  }
+
+  @Private
+  public static Resource getContainerResource(Map<String, Object> entityInfo) {
+    if (entityInfo == null) {
+      return Resource.newInstance(0, 0);
+    }
+    long memory = ((Number) entityInfo.getOrDefault(
+        ContainerMetricsConstants.ALLOCATED_MEMORY_INFO, 0L)).longValue();
+    int vcores = ((Number) entityInfo.getOrDefault(
+        ContainerMetricsConstants.ALLOCATED_VCORE_INFO, 0)).intValue();
+    Resource resource = Resource.newInstance(memory, vcores);
+    Object allocationInfo = entityInfo.get(
+        ContainerMetricsConstants.ALLOCATED_RESOURCES_INFO);
+    if (allocationInfo instanceof Map) {
+      Map<?, ?> allocations = (Map<?, ?>) allocationInfo;
+      for (Map.Entry<?, ?> entry : allocations.entrySet()) {
+        try {
+          String name = (String) entry.getKey();
+          if (ResourceInformation.MEMORY_URI.equals(name)
+              || ResourceInformation.VCORES_URI.equals(name)) {
+            continue;
+          }
+          if (!ResourceUtils.getResourceTypes().containsKey(name)) {
+            LOG.debug("Skipping unknown resource type {} in container 
history", name);
+            continue;
+          }
+          Map<?, ?> allocation = (Map<?, ?>) entry.getValue();
+          Number storedValue = (Number) allocation.get("value");
+          if (storedValue instanceof Float || storedValue instanceof Double) {
+            throw new IllegalArgumentException("Floating-point resource 
value");
+          }
+          long value = storedValue instanceof Integer || storedValue 
instanceof Long
+              ? storedValue.longValue()
+              : new BigDecimal(storedValue.toString()).longValueExact();
+          String units = (String) allocation.get("units");
+          String defaultUnits = 
resource.getResourceInformation(name).getUnits();
+          resource.setResourceValue(name,
+              UnitsConversionUtil.convert(units, defaultUnits, value));
+        } catch (ClassCastException | NullPointerException

Review Comment:
   Thanks for the optional suggestion. I’d keep the specific catches here: 
after the configured-type check, a `ResourceNotFoundException` would indicate 
inconsistent resource metadata rather than a malformed stored entry, so 
silently skipping it could hide a separate problem.



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