Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2485#discussion_r159957666
--- Diff:
storm-server/src/main/java/org/apache/storm/scheduler/resource/NormalizedResources.java
---
@@ -246,59 +259,133 @@ public boolean
couldHoldIgnoringSharedMemory(NormalizedResources other) {
return true;
}
+ private void throwBecauseResourceIsMissingFromTotal(int resourceIndex)
{
+ String resourceName = null;
+ for (Map.Entry<String, Integer> entry : resourceNames.entrySet()) {
+ int index = entry.getValue();
+ if (index == resourceIndex) {
+ resourceName = entry.getKey();
+ break;
+ }
+ }
+ if (resourceName == null) {
+ throw new IllegalStateException("Array index " + resourceIndex
+ " is not mapped in the resource names map."
+ + " This should not be possible, and is likely a bug in
the Storm code.");
+ }
+ throw new IllegalArgumentException("Total resources does not
contain resource '"
+ + resourceName
+ + "'. All resources should be represented in the total. This
is likely a bug in the Storm code");
+ }
+
/**
- * Calculate the average resource usage percentage with this being the
total resources and
- * used being the amounts used.
+ * Calculate the average resource usage percentage with this being the
total resources and used being the amounts used.
+ *
* @param used the amount of resources used.
- * @return the average percentage used 0.0 to 100.0.
+ * @return the average percentage used 0.0 to 100.0. Clamps to 100.0
in case there are no available resources in the total
*/
public double calculateAveragePercentageUsedBy(NormalizedResources
used) {
+ int skippedResourceTypes = 0;
double total = 0.0;
double totalMemory = getTotalMemoryMb();
if (totalMemory != 0.0) {
total += used.getTotalMemoryMb() / totalMemory;
+ } else {
+ skippedResourceTypes++;
}
double totalCpu = getTotalCpu();
if (totalCpu != 0.0) {
total += used.getTotalCpu() / getTotalCpu();
+ } else {
+ skippedResourceTypes++;
}
- //If total is 0 we add in a 0% used, so we can just skip over
anything that is not in both.
- int length = Math.min(used.otherResources.length,
otherResources.length);
- for (int i = 0; i < length; i++) {
- if (otherResources[i] != 0.0) {
- total += used.otherResources[i] / otherResources[i];
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Calculating avg percentage used by. Used CPU: {}
Total CPU: {} Used Mem: {} Total Mem: {}"
+ + " Other Used: {} Other Total: {}", totalCpu,
used.getTotalCpu(), totalMemory, used.getTotalMemoryMb(),
+ this.toNormalizedOtherResources(),
used.toNormalizedOtherResources());
+ }
+
+ if (used.otherResources.length > otherResources.length) {
+
throwBecauseResourceIsMissingFromTotal(used.otherResources.length);
--- End diff --
I could only find references to this method and calculateMin in the two
resource aware strategies, where they seem to be used to estimate how large a
percentage of total cluster resources a rack or node represents. That's why I
assumed that hitting this case would always be a bug, because the total cluster
resources should contain all the resources of the racks.
I'm happy to instead pretend that otherResources has been padded with
zeroes, and skip the zero values like we do a few lines down.
---
