joseluisll commented on code in PR #8770:
URL: https://github.com/apache/hadoop/pull/8770#discussion_r4181416993
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/timeline/TimelineUtils.java:
##########
@@ -60,6 +72,57 @@ 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)) {
+ 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);
+ Map<?, ?> allocations = (Map<?, ?>) entityInfo.get(
+ ContainerMetricsConstants.ALLOCATED_RESOURCES_INFO);
+ if (allocations != null) {
+ for (Map.Entry<?, ?> entry : allocations.entrySet()) {
+ String name = (String) entry.getKey();
+ if (ResourceInformation.MEMORY_URI.equals(name)
+ || ResourceInformation.VCORES_URI.equals(name)) {
+ continue;
+ }
+ if (!ResourceUtils.getResourceTypes().containsKey(name)) {
+ LOG.warn("Skipping unknown resource type {} in container history",
name);
+ continue;
+ }
+ Map<?, ?> allocation = (Map<?, ?>) entry.getValue();
Review Comment:
**Make per-entry parsing tolerant of bad data**
This loop has no error handling, so one malformed entry fails the whole
conversion:
- a missing `value` gives an NPE;
- a value or units of an unexpected type gives a `ClassCastException`;
- an unknown unit, or a unit conversion that overflows, gives an
`IllegalArgumentException` from `UnitsConversionUtil.convert`.
Unknown resource types already get "warn and skip". I think bad entries
deserve the same treatment instead of failing the request. Otherwise one bad
entry breaks the container report, and on the list endpoints it breaks every
container of the attempt. Something like:
```java
try {
Map<?, ?> allocation = (Map<?, ?>) entry.getValue();
long value = ((Number) allocation.get("value")).longValue();
String units = (String) allocation.get("units");
String defaultUnits = resource.getResourceInformation(name).getUnits();
resource.setResourceValue(name,
UnitsConversionUtil.convert(units, defaultUnits, value));
} catch (RuntimeException e) {
LOG.debug("Skipping malformed allocation for resource type {}", name, e);
}
```
A small test with a missing `value` and with an unknown unit would cover it.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/timeline/TimelineUtils.java:
##########
@@ -60,6 +72,57 @@ 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)) {
+ 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);
+ Map<?, ?> allocations = (Map<?, ?>) entityInfo.get(
+ ContainerMetricsConstants.ALLOCATED_RESOURCES_INFO);
+ if (allocations != null) {
+ for (Map.Entry<?, ?> entry : allocations.entrySet()) {
+ String name = (String) entry.getKey();
+ if (ResourceInformation.MEMORY_URI.equals(name)
+ || ResourceInformation.VCORES_URI.equals(name)) {
+ continue;
+ }
+ if (!ResourceUtils.getResourceTypes().containsKey(name)) {
+ LOG.warn("Skipping unknown resource type {} in container history",
name);
Review Comment:
**Avoid log spam for unknown resource types**
This warning runs for every container on every read. For example, take an
AHS without `resource-types.xml` that serves a container list for a large app.
It would write one warning per container per request. Could we log this at
DEBUG, or warn only once per resource type (e.g. track the names already
reported in a `ConcurrentHashMap.newKeySet()`)?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/timeline/TimelineUtils.java:
##########
@@ -60,6 +72,57 @@ public class TimelineUtils {
YarnJacksonJaxbJsonProvider.configObjectMapper(mapper);
}
+ @Private
+ public static Map<String, Map<String, Object>> getCustomResourceInfo(
Review Comment:
**Skip the field when there are no custom resources**
This always returns a map, and the publishers always `put` it. So on
clusters without custom resources, every container entity gets an empty
`YARN_CONTAINER_ALLOCATED_RESOURCES: {}`. That's an extra column per container
in ATSv2/HBase and extra bytes in ATSv1. Readers already handle the field being
absent, so the publishers could skip the `put` when the map is empty.
--
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]
