Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2485#discussion_r159735958
--- 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);
+ }
+
+ for (int i = 0; i < otherResources.length; i++) {
+ double totalValue = otherResources[i];
+ if (totalValue == 0.0) {
+ //Skip any resources where the total is 0, the percent
used for this resource isn't meaningful.
+ //We fall back to prioritizing by cpu, memory and any
other resources by ignoring this value
+ skippedResourceTypes++;
--- End diff --
This looks fine to me, but could we explain in the javadoc for this method
that 0 values are handled differently then might be expected?
---
