Github user revans2 commented on a diff in the pull request: https://github.com/apache/storm/pull/2485#discussion_r159735761 --- 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 -- My assumption was that if a resource is not around then it's value is 0. It looks like in this case, that if a resources is missing an exception will be thrown. I am not sure that this will not happen in normal operation, or if a user submits a topology with a resource that is not currently supported.
---