aggregator: remove synchronous RPCs and extend to generic values #1535
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #1535 +/- ##
==========================================
+ Coverage 78.8% 78.81% +<.01%
==========================================
Files 164 164
Lines 30333 30378 +45
==========================================
+ Hits 23905 23941 +36
- Misses 6428 6437 +9
|
grondo
force-pushed
the
aggregator-generalize
branch
2 times, most recently
from
431549f
to
51c8f3d
Compare
|
I made some changes, squashed and forced a push. The testing now tests floating point, string, array, and object value types. The |
|
This seems like a useful enhancement! Should we open a separate issue on creating a C API and man pages for the aggregator? This can go in as far as I'm concerned, just needs a rebase. |
Update the aggregator to remove use of synchronous RPCs in the forward and sink methods.
Extend the aggregator such that any json_t value can be aggregated. Values will be aggregated for any json_t types where `json_equal` returns true. The aggregator will thus now work for integer and double types, as well as strings, arrays, and even objects. Summary statistics (max,min) are only supported for integer and real types. The max and min members are kept at the top level of the json result written to kvs by the "sink" implementation.
Loosen restrictions on the types of values aggregated by the flux-aggregate test utility. Allow value argument on flux-aggregate cmdline to be a string or a numeric value. Add a `-e, --execute=CODE` argument to allow values that are created from arbitrary Lua CODE. This allows generation of arrays and complex objects for aggregator testing.
Add tests to ensure floating point, string, and complex object types work as the value target of aggregates.
grondo
force-pushed
the
aggregator-generalize
branch
from
51c8f3d
to
a3563f9
Compare
|
Restarted a bunch of stalled builders - looks like a travis issue, not one of ours. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is overall a fairly minor improvement to the aggregator module.
First the remaining synchronous RPCs were removed. Then the
int64_t"value" (the target of the aggregate) was replaced with a genericjson_tvalue. This allows identical values to be aggregated other than just integers, including floating-point, strings, and even JSON objects. As long asjson_equal()returns true between two values, they will be coalesced by integer identifier.The "summary statistics" (min, max) are generalized into a summary object, however this is (currently) only supported for real and integer values.