This is an automatically generated e-mail. To reply, visit:

docs/attributes-resources.md (line 7)

    Not really, just a minor doc cleanup I made along the way. Happy to split 
into a separate patch if you'd prefer.

docs/attributes-resources.md (line 39)

    Two spaces after a period is considered good style in English prose 
according to some people (this style is used in various places throughout the 
comments and docs, but we aren't consistent). I don't have a strong view, but 
it will require changing a lot more places than just here to adopt a single 

src/common/values.cpp (line 52)

    I'd prefer to keep the newline: without a newline, it suggests that the 
comment is specific to the function that follows (`operator<<`), which would be 
misleading: the comment applies to the following ~7 functions.

src/common/values.cpp (line 60)

    Truncation rather than rounding might also be reasonable behavior; I'm 
curious what other people think.

src/common/values.cpp (line 61)

    I didn't use `setprecision`, because:
    1. There is value in making sure we use the same rounding method in this 
operator as elsewhere, e.g, for corner-cases like 1.2345
    2. `setprecision` is a side-effecting operation, so modifying the caller's 
`ostream` seems problematic.

src/common/values.cpp (line 67)

    What should we do in case of overflow?
    Note that we don't check for overflow in `operator+` (or for negative 
result values in `operator-`)...

- Neil Conway

On Feb. 16, 2016, 11:29 p.m., Neil Conway wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43635/
> -----------------------------------------------------------
> (Updated Feb. 16, 2016, 11:29 p.m.)
> Review request for mesos, Joris Van Remoortere and Michael Park.
> Bugs: MESOS-4687
>     https://issues.apache.org/jira/browse/MESOS-4687
> Repository: mesos
> Description
> -------
> Scalar resource values are represented using floating point. As a result, 
> users
> could see unexpected results when accepting offers and making reservations for
> fractional resources: values like "0.1" cannot be precisely represented using
> standard floating point, and the resource values returned to frameworks might
> contain an unpredictable amount of roundoff error.
> This commit adjusts the master to use fixed-point when doing internal
> computations on scalar resource values. The fixed-point format only supports
> three decimal digits of precision: that is, fractional resource values like
> "0.001" will be supported, but "0.0001" will not be.
> Diffs
> -----
>   docs/attributes-resources.md 818da8ab0c672144b02f526b2b805cf0505d2c7e 
>   docs/upgrades.md 4f30d725c6ed28c09a1c5528fd4193c3f06b2d93 
>   include/mesos/mesos.proto e24d3e03a7dc7c6bfd07f34531cb593fe4925646 
>   include/mesos/v1/mesos.proto d909e60ddfd8e3ba2075f82c372edde04cd99d54 
>   src/common/resources.cpp 5d731870542166cec11f9956ccdf16207b2d22cc 
>   src/common/values.cpp c64407bc97ad858300f4661d616e0480920fc541 
>   src/master/allocator/mesos/hierarchical.cpp 
> a9d2c23162892e22220f97d89a076d2311091d91 
>   src/tests/resources_tests.cpp 96864c3945729fe33be8b243b9c826fb12e90eff 
>   src/v1/resources.cpp 207eb61d6a6d03d314539d42751cac65fcffa9af 
>   src/v1/values.cpp 58ea9875804bf0287855a1e9855855e5e54de4c4 
> Diff: https://reviews.apache.org/r/43635/diff/
> Testing
> -------
> make check
> Manually verified that some of the floating point oddities in 
> https://issues.apache.org/jira/browse/MESOS-4071 do not occur when this patch 
> is applied, although I wasn't able to reproduce the crash described in that 
> ticket.
> * We don't currently emit a warning when discarding additional digits of 
> precision from input scalar resource values. Should we? That would require 
> identifying all the points where a resource value is seemed to be 
> "user-provided", and also runs the risk of generating a ton of log messages 
> when an old framework is used.
> * Similarly, if the user gives us a resource value and we don't do anything 
> to it, we won't discard any additional precision that appears in the value -- 
> the precision only gets discarded when we apply an operator like `+` or `-`. 
> Unclear if we should trim additional precision from all scalar resource 
> values more aggressively.
> Thanks,
> Neil Conway

Reply via email to