Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Bernd Mathiske]

> On Oct. 6, 2015, 5:42 p.m., Klaus Ma wrote:
> > src/common/resources.cpp, line 879
> > 
> >
> > This fix is ok for this ticket; but how to handle other part about 
> > cpu()? Here's some question from me:
> > 1. if `epsilon` is 0.01 here, does it mean the min cpu is 0.01?
> > 2. is this the only code that need `epsilon`? It seems not
> > 
> > @jieyu/@mcypark, show we start a EPIC to include all precision related 
> > ticket? so, we can 
> > 1. unify the min cpu/men/disk to user
> > 2. unify the operator/code within allocator
> > 3. unify the precision between backend and UI
> > 4. clarify the requirement to cpu/mem, e.g. whether accept empty cpu/mem
> > 
> > Any comments?

I suggest (based on input from @benjaminhindman) we don't use a specific 
constant here, but try using CHECK_DOUBLE_EQ, first. Only if this does not 
work, we should switch to CHECK_NEAR, which takes epsilon as an argument. (In 
that case, we'd have to elevate the constant to a more prominent place and 
describe it in the docs as well.) With either of this, we'll have a viable 
short-term fix. The long-term solution will be pursued by MESOS-3997.


- Bernd


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101727
---


On Oct. 6, 2015, 3:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 3:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Greg Mann]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review107420
---



src/common/resources.cpp (lines 879 - 881)


What about the resource types besides `cpu`? The JIRA was filed because of 
a `cpu` error, but this same problem is just as likely for `mem` and `disk`, 
can we apply the fix to them as well?


- Greg Mann


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mandeep Chadha]

> On Oct. 7, 2015, 1:03 a.m., Guangya Liu wrote:
> > src/common/resources.cpp, line 879
> > 
> >
> > The meos is now using 0.01 as the MIN_CPUS
> 
> Guangya Liu wrote:
> As the mesos is using 0.01 as the MIN_CPUS, I think it is OK using 0.01 
> as the epsilon

Are we ok with changes ? Friendly ping :)


- Mandeep


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101729
---


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Neil Conway]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101674
---



src/common/resources.cpp (line 870)


This change should also be applied to Resources::apply() in 
src/v1/resources.cpp



src/common/resources.cpp (line 871)


Capitalize "check"; also, maybe include a brief explanation of why we might 
see some error in the floating point calculation.



src/common/resources.cpp (line 872)


Should be "constexpr", maybe "static" too.



src/tests/reservation_tests.cpp (line 175)


I think the comment here should explain why using a double value is 
relevant.



src/tests/reservation_tests.cpp (line 245)


Whitespace/capitalization.


- Neil Conway


On Oct. 6, 2015, 5:59 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 5:59 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101684
---


Bad review!

Reviews applied: []

Error:
 No reviewers specified. Please find a reviewer by asking on JIRA or the 
mailing list.

- Mesos ReviewBot


On Oct. 6, 2015, 5:59 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 5:59 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mandeep Chadha]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/
---

(Updated Oct. 6, 2015, 9:33 p.m.)


Review request for mesos and Neil Conway.


Bugs: MESOS-3552
https://issues.apache.org/jira/browse/MESOS-3552


Repository: mesos


Description
---

Check failed due to double comparison : MESOS-3552.


Diffs (updated)
-

  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
  src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 

Diff: https://reviews.apache.org/r/39056/diff/


Testing
---

Added unit test.
make check successful.


Thanks,

Mandeep Chadha

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Anand Mazumdar]

> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote:
> > src/v1/resources.cpp, lines 880-886
> > 
> >
> > Not yours, but too bad we need to duplicate the logic here. I am now 
> > sure what will be the long term plan here. If we have 10 versions, do we 
> > need to copy the code 10 times... That's not good :(
> 
> Mandeep Chadha wrote:
> I agree !! I already missed src/v1/resources.cpp in my first review 
> request :) 
> Thanks Neil.

Jie, this is a tech-debt that should not hopefully exist across 10 version 
iterations :)

On a more serious note, this is the related JIRA :  
https://issues.apache.org/jira/browse/MESOS-3404 
We intend to work on it as part of the HTTP API epic.


- Anand


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101691
---


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Neil Conway]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101690
---

Ship it!


Ship It!


src/tests/reservation_tests.cpp (line 173)


"a Reserved"


- Neil Conway


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101691
---



src/common/resources.cpp (line 876)


Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to

https://code.google.com/p/googletest/wiki/AdvancedGuide#Floating-Point_Macros

To choose a default epsilon, you probably want to check the above link. 
Gtest uses "Units in the Last Place (ULPs)" as the default.



src/v1/resources.cpp (lines 876 - 882)


Not yours, but too bad we need to duplicate the logic here. I am now sure 
what will be the long term plan here. If we have 10 versions, do we need to 
copy the code 10 times... That's not good :(


- Jie Yu


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mandeep Chadha]

> On Oct. 6, 2015, 6:34 p.m., Neil Conway wrote:
> > src/common/resources.cpp, line 874
> > 
> >
> > This change should also be applied to Resources::apply() in 
> > src/v1/resources.cpp

Thanks Neil.


- Mandeep


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101674
---


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mandeep Chadha]

> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote:
> > src/common/resources.cpp, line 880
> > 
> >
> > Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to
> > 
> > https://code.google.com/p/googletest/wiki/AdvancedGuide#Floating-Point_Macros
> > 
> > To choose a default epsilon, you probably want to check the above link. 
> > Gtest uses "Units in the Last Place (ULPs)" as the default.
> 
> Mandeep Chadha wrote:
> CHECK_DOUBLE_EQ will also fail. 
> 
> F0930 18:15:38.169140 26984 resources.cpp:874] Check failed: 
> (result.cpus().get()) >= (cpus().get())-0.001L (24 vs. 24) *** 
> Check failure stack trace: *** F0930 18:15:38.169322 26991 resources.cpp:874] 
> Check failed: (result.cpus().get()) >= (cpus().get())-0.001L (24 
> vs. 24) *** Check failure stack trace: ***
> 
> CHECK_NEAR() is the right Macro to use. 
> 
> 
> But we also need to check (isNone() and isNone()) equality and hence the 
> present implementation. I think the long term fix is to wrap double into 
> Double and have the opeartor== to the right thing.
> 
> Jie Yu wrote:
> How do you implemente CHECK_DOUBLE_EQ? You copied from gtest code?

you can just call it:

CHECK_DOUBLE_EQ(result.cpus.get(), cpus.get());

src/common/resources.cpp ( includes the glog/logging.h and all the Macros are 
available).

#include 


- Mandeep


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101691
---


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mandeep Chadha]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/
---

(Updated Oct. 6, 2015, 10:07 p.m.)


Review request for mesos and Neil Conway.


Bugs: MESOS-3552
https://issues.apache.org/jira/browse/MESOS-3552


Repository: mesos


Description
---

Check failed due to double comparison : MESOS-3552.


Diffs (updated)
-

  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
  src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 

Diff: https://reviews.apache.org/r/39056/diff/


Testing
---

Added unit test.
make check successful.


Thanks,

Mandeep Chadha

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Jie Yu]

> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote:
> > src/common/resources.cpp, line 880
> > 
> >
> > Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to
> > 
> > https://code.google.com/p/googletest/wiki/AdvancedGuide#Floating-Point_Macros
> > 
> > To choose a default epsilon, you probably want to check the above link. 
> > Gtest uses "Units in the Last Place (ULPs)" as the default.
> 
> Mandeep Chadha wrote:
> CHECK_DOUBLE_EQ will also fail. 
> 
> F0930 18:15:38.169140 26984 resources.cpp:874] Check failed: 
> (result.cpus().get()) >= (cpus().get())-0.001L (24 vs. 24) *** 
> Check failure stack trace: *** F0930 18:15:38.169322 26991 resources.cpp:874] 
> Check failed: (result.cpus().get()) >= (cpus().get())-0.001L (24 
> vs. 24) *** Check failure stack trace: ***
> 
> CHECK_NEAR() is the right Macro to use. 
> 
> 
> But we also need to check (isNone() and isNone()) equality and hence the 
> present implementation. I think the long term fix is to wrap double into 
> Double and have the opeartor== to the right thing.

How do you implemente CHECK_DOUBLE_EQ? You copied from gtest code?


- Jie


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101691
---


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101726
---


Patch looks great!

Reviews applied: [39056]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Klaus Ma]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101727
---



src/common/resources.cpp (line 875)


This fix is ok for this ticket; but how to handle other part about cpu()? 
Here's some question from me:
1. if `epsilon` is 0.01 here, does it mean the min cpu is 0.01?
2. is this the only code that need `epsilon`? It seems not

@jieyu/@mcypark, show we start a EPIC to include all precision related 
ticket? so, we can 
1. unify the min cpu/men/disk to user
2. unify the operator/code within allocator
3. unify the precision between backend and UI
4. clarify the requirement to cpu/mem, e.g. whether accept empty cpu/mem

Any comments?


- Klaus Ma


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101729
---



src/common/resources.cpp (line 875)


The meos is now using 0.01 as the MIN_CPUS


- Guangya Liu


On 十月 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated 十月 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>

Re: Review Request 39056: Fix for Mesos master crash due to check failure.

[Guangya Liu]

> On 十月 7, 2015, 1:03 a.m., Guangya Liu wrote:
> > src/common/resources.cpp, line 879
> > 
> >
> > The meos is now using 0.01 as the MIN_CPUS

As the mesos is using 0.01 as the MIN_CPUS, I think it is OK using 0.01 as the 
epsilon


- Guangya


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39056/#review101729
---


On 十月 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated 十月 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>