Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2485#discussion_r159736224
--- 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++;
+ continue;
}
+ double usedValue;
+ if (i >= used.otherResources.length) {
+ //Resources missing from used are using none of that
resource
+ usedValue = 0.0;
+ } else {
+ usedValue = used.otherResources[i];
+ }
+ total += usedValue / totalValue;
+ }
+ //Adjust the divisor for the average to account for any skipped
resources (those where the total was 0)
+ int divisor = 2 + otherResources.length - skippedResourceTypes;
+ if (divisor == 0) {
+ /*This is an arbitrary choice to make the result consistent
with calculateMin.
+ Any value would be valid here, becase there are no (non-zero)
resources in the total set of resources,
+ so we're trying to average 0 values.
+ */
+ return 100.0;
+ } else {
+ return (total * 100.0) / divisor;
}
- //To get the count we divide by we need to take the maximum length
because we are doing an average.
- return (total * 100.0) / (2 + Math.max(otherResources.length,
used.otherResources.length));
}
/**
- * Calculate the minimum resource usage percentage with this being the
total resources and
- * used being the amounts used.
+ * Calculate the minimum resource usage percentage with this being the
total resources and used being the amounts used.
+ *
* @param used the amount of resources used.
- * @return the minimum percentage used 0.0 to 100.0.
+ * @return the minimum percentage used 0.0 to 100.0. Clamps to 100.0
in case there are no available resources in the total.
*/
public double calculateMinPercentageUsedBy(NormalizedResources used) {
double totalMemory = getTotalMemoryMb();
double totalCpu = getTotalCpu();
+
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Calculating min percentage used by. Used CPU: {}
Total CPU: {} Used Mem: {} Total Mem: {}"
+ + " Other Used: {} Other Total: {}", totalCpu,
used.getTotalCpu(), totalMemory, used.getTotalMemoryMb(),
+ toNormalizedOtherResources(),
used.toNormalizedOtherResources());
+ }
+
+ if (used.otherResources.length > otherResources.length) {
+
throwBecauseResourceIsMissingFromTotal(used.otherResources.length);
+ }
+
if (used.otherResources.length != otherResources.length
|| totalMemory == 0.0
|| totalCpu == 0.0) {
//If the lengths don't match one of the resources will be 0,
which means we would calculate the percentage to be 0.0
// and so the min would be 0.0 (assuming that we can never go
negative on a resource being used.
return 0.0;
}
- double min = used.getTotalMemoryMb() / totalMemory;
- min = Math.min(min, used.getTotalCpu() / getTotalCpu());
+
+ double min = 100.0;
+ if (totalMemory != 0.0) {
+ min = Math.min(min, used.getTotalMemoryMb() / totalMemory);
+ }
+ if (totalCpu != 0.0) {
--- End diff --
Again the math looks OK, but could you explain that 0 % used is ignored in
the javadocs, because it is not obvious to me that it what will happen.
---
