GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2463
STORM-2859: Fix a number of issues with NormalizedResources, and prevâ¦
â¦ent it from leaking static state in tests.
This is part of making the build run on the new Travis Ubuntu image. Travis
is probably running the tests in a slightly different order than before.
NormalizedResources had a static field used to track the generic resource
types registered for supervisors. It was leaking from the
GenericResourceAwareSchedulerTest to DefaultResourceAwareSchedulerTest, where
it was causing a test failure. When the unused resource type showed up in the
test, NormalizedResources would return 0.0 from calculateMinPercentageUsedBy
for all racks because there was a resource type where the cluster had 0 of that
resource in total. This breaks rack sorting and causes the test to fail.
I'm not entirely sure why NormalizedResources used the static field +
double arrays for tracking resources, but I've replaced it with regular
resourceName -> resourceValue maps. Let me know if this is a problem.
Other issues fixed:
* calculateMinPercentageUsedBy threw division by 0 error if total cpu or
memory was 0.
* Removed static map between resource names and array indices. It was
causing tests to leak information to each other. Rewrote parts of
NormalizedResources to make this map unnecessary.
* Added check that all resources in a rack/node are also present in the
total.
* calculateAvg was being inconsistent about the divisor in the average
between cpu/memory and other resources. Fixed so skipped resources are never
counted in the divisor.
* calculateMin didn't handle resource totals being 0 very well. The method
defined (anyNumber)/0 to result in 0, so if any resource in the total had a 0
value, all racks/nodes would be prioritized equally. Change calculateMin and
calculateAvg to simply skip any resources where the total is 0 in the
calculation. I think it makes sense to disregard such resources and fall back
on prioritizing by any remaining non-0 resources. It was already done for CPU
and memory in calculateAvg. In case there are no non-0 resources, we can just
return 100% for both average and minimum to disable prioritization, since any
rack/node is as good as any other.
* Fixed up NormalizedResources constructor so it doesn't call an
overridable method.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/srdo/storm STORM-2859
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2463.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2463
----
commit b654a4f065a496a273601e05888e79b35a991653
Author: Stig Rohde Døssing <[email protected]>
Date: 2017-12-13T15:47:31Z
STORM-2859: Fix a number of issues with NormalizedResources, and prevent it
from leaking static state in tests.
----
---
