You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2017/12/11 17:16:28 UTC
[GitHub] brooklyn-server pull request #915: BROOKLYN-569: aggregator casts vals to nu...
GitHub user aledsage opened a pull request:
https://github.com/apache/brooklyn-server/pull/915
BROOKLYN-569: aggregator casts vals to numbers
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/brooklyn-server BROOKLYN-569
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/915.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 #915
----
commit c41b3e758a30a511cf189109517a57a7b0447cf9
Author: Aled Sage <al...@gmail.com>
Date: 2017-12-11T17:15:50Z
BROOKLYN-569: aggregator casts vals to numbers
----
---
[GitHub] brooklyn-server issue #915: BROOKLYN-569: aggregator casts vals to numbers
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/915
Tested, LGTM, thanks @aledsage
---
[GitHub] brooklyn-server pull request #915: BROOKLYN-569: aggregator casts vals to nu...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/915
---
[GitHub] brooklyn-server pull request #915: BROOKLYN-569: aggregator casts vals to nu...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/915#discussion_r158230801
--- Diff: core/src/main/java/org/apache/brooklyn/enricher/stock/MathAggregatorFunctions.java ---
@@ -118,9 +123,10 @@ public T apply(@Nullable Collection<? extends Number> vals) {
List<Number> postProcessedVals = new ArrayList<>();
int count = 0;
if (vals != null) {
- for (Number val : vals) {
- if (val != null) {
- postProcessedVals.add(val);
+ for (Object val : vals) {
+ Maybe<Number> coercedVal = TypeCoercions.tryCoerce(val, Number.class);
+ if (coercedVal.isPresentAndNonNull()) {
+ postProcessedVals.add(coercedVal.get());
count++;
} else if (defaultValueForUnreportedSensors != null) {
--- End diff --
Done - but also guarding so we don't log repeatedly at warn. The aggregator might well get called every few seconds per enricher, so must avoid flooding the log with such warn messages.
---
[GitHub] brooklyn-server issue #915: BROOKLYN-569: aggregator casts vals to numbers
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/915
@aledsage minor comment, merge when ready
---
[GitHub] brooklyn-server pull request #915: BROOKLYN-569: aggregator casts vals to nu...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/915#discussion_r157765674
--- Diff: core/src/main/java/org/apache/brooklyn/enricher/stock/MathAggregatorFunctions.java ---
@@ -118,9 +123,10 @@ public T apply(@Nullable Collection<? extends Number> vals) {
List<Number> postProcessedVals = new ArrayList<>();
int count = 0;
if (vals != null) {
- for (Number val : vals) {
- if (val != null) {
- postProcessedVals.add(val);
+ for (Object val : vals) {
+ Maybe<Number> coercedVal = TypeCoercions.tryCoerce(val, Number.class);
+ if (coercedVal.isPresentAndNonNull()) {
+ postProcessedVals.add(coercedVal.get());
count++;
} else if (defaultValueForUnreportedSensors != null) {
--- End diff --
worth a `log.warn` saying the value if not coercible?
```
} else {
if (val!=null) {
log.warn("Input to numeric aggregator is not a number: "+val+" ("+val.getClass+")");
}
if (defaultValueForUnrepoertedSensors != null) {
...
}
}
```
---
[GitHub] brooklyn-server issue #915: BROOKLYN-569: aggregator casts vals to numbers
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/915
Retest this please
Test failure is unrelated: `ElectPrimaryTest.testSelectionModeFailoverReelectWithPreference`. The info log from jenkins has the lines shown below. This tells us that `ElectPrimaryEffector` is being called twice - once directly int the test, and presumably concurrently by the policy's `onEvent` (it triggers that whenever a child/member is added/removed or its serviceUp/serviceState/weighting sensor changes). There is a definite race there, which can happen when the effector is being called manually.
```
2017-12-11 17:28:13,488 INFO Test created app, and will now start BasicApplicationImpl{id=jt5d932r4f}
2017-12-11 17:28:13,515 INFO Detected new primary TestEntityImpl{id=jt0dh78cvw} at BasicApplicationImpl{id=jt5d932r4f} (previously had null)
2017-12-11 17:28:13,515 INFO Primary TestEntityImpl{id=jt0dh78cvw} at BasicApplicationImpl{id=jt5d932r4f} detected as healthy
2017-12-11 17:28:13,528 INFO Started application BasicApplicationImpl{id=jt5d932r4f}
2017-12-11 17:28:13,530 INFO Detected new primary TestEntityImpl{id=ndkegu8wyc} at BasicApplicationImpl{id=jt5d932r4f} (previously had TestEntityImpl{id=jt0dh78cvw})
2017-12-11 17:28:13,531 INFO Detected new primary TestEntityImpl{id=jt0dh78cvw} at BasicApplicationImpl{id=jt5d932r4f} (previously had TestEntityImpl{id=ndkegu8wyc})
2017-12-11 17:28:43,544 INFO failed succeeds-eventually, 75 attempts, 30000ms elapsed (rethrowing): java.lang.AssertionError: attribute=Sensor: primary (org.apache.brooklyn.api.entity.Entity); val=TestEntityImpl{id=jt0dh78cvw}
```
---