-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57752/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier and Joris Van Remoortere.
Bugs: MESOS-7197
https://issues.apache.org/jira/browse/MESOS-7197
Repository: mesos
Description
-------
When parsing resources, inputs such as "cpus:0" are considered "empty"
and are omitted from the `Resources` object. However, a very small
resource value like "cpus:0.00001" was considered non-empty, despite the
fact that the numerical behavior for resources means that these two
values compare as equal. This inconsistency resulted in a `CHECK`
failure in the sorter.
This commit fixes the resource parsing logic to treat such very small
resource values as empty resources, which resolves the inconsistency
above. The resulting behavior might be a bit surprising to users (since
the task will launch with zero resources instead of a very small
resource value), but improving this is left as future work (MESOS-1807).
Diffs
-----
src/common/resources.cpp b64fd76d1d3d8179bae2ac5b335bdf4ad980c9a7
src/tests/master_tests.cpp 5fdb9493c9aed31b90e03062544c1446eb200040
src/tests/resources_tests.cpp 18e8b670f2f0be83be30b4a73503e786d5ae9b48
src/v1/resources.cpp 4ffe9503b0c792e832dda77e38098509d40e0f18
Diff: https://reviews.apache.org/r/57752/diff/1/
Testing
-------
`make check`; added new unit tests both for resource parsing/comparison logic
and for end-to-end system behavior (launching a task with a tiny resource value
and ensuring it doesn't crash the master).
I was concerned that changing `Resources::isEmpty` to introduce a local
`Value::Scalar` variable would regress resource parsing performance, but that
doesn't appear to be the case. I added a new microbenchmark to validate this;
perf w/o the change:
```
[----------] 2 tests from Resources_Parse/Resources_Parse_BENCHMARK_Test
[ RUN ] Resources_Parse/Resources_Parse_BENCHMARK_Test.Parse/0
[ OK ] Resources_Parse/Resources_Parse_BENCHMARK_Test.Parse/0 (741 ms)
[ RUN ] Resources_Parse/Resources_Parse_BENCHMARK_Test.Parse/1
[ OK ] Resources_Parse/Resources_Parse_BENCHMARK_Test.Parse/1 (6138 ms)
[----------] 2 tests from Resources_Parse/Resources_Parse_BENCHMARK_Test (6879
ms total)
```
Perf w/ the change:
```
[----------] 2 tests from Resources_Parse/Resources_Parse_BENCHMARK_Test
[ RUN ] Resources_Parse/Resources_Parse_BENCHMARK_Test.Parse/0
[ OK ] Resources_Parse/Resources_Parse_BENCHMARK_Test.Parse/0 (732 ms)
[ RUN ] Resources_Parse/Resources_Parse_BENCHMARK_Test.Parse/1
[ OK ] Resources_Parse/Resources_Parse_BENCHMARK_Test.Parse/1 (5751 ms)
[----------] 2 tests from Resources_Parse/Resources_Parse_BENCHMARK_Test (6483
ms total)
```
(It obviously doesn't improve performance, but these results suggest to me that
the change doesn't significantly regress performance, either.)
Thanks,
Neil Conway
