mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zhitao Li <zhitaoli...@gmail.com>
Subject Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.
Date Tue, 17 Apr 2018 17:52:02 GMT


> On April 12, 2018, 5:27 p.m., Greg Mann wrote:
> > include/mesos/mesos.proto
> > Lines 1975 (patched)
> > <https://reviews.apache.org/r/66049/diff/6/?file=1994606#file1994606line1975>
> >
> >     Hmm I wonder if we should just use a `double` here? Does the `Value.Scalar`
type provide some benefit?
> 
> Chun-Hung Hsiao wrote:
>     From the API perspective, `double` is easier to use, but `Value.Scalar` is more consistent
with how we represesnt a disk resource.
>     
>     From the implementation perspective, I prefer `Value.Scalar` because it's arithmetic
operators will do fixed-point conversions for me so I won't forget that.
> 
> Greg Mann wrote:
>     My recommendation would be to prioritize easy-of-use in the API over ease-of-implementation
when possible :)
>     
>     In the implementation, as long as we toss the provided double into a `Scalar::Value`
before we do anything with it, I think we could get the same benefit?
> 
> Zhitao Li wrote:
>     I feel that `Scalar` is actually more consistent than `double` here. I do not see
either API is much easier to use.
> 
> Greg Mann wrote:
>     Why is `Scalar` more consistent? I think that `double` would be simpler because it
eliminates the nested `value` field, which is not intuitive.
>     
>     Is there any reason to make this a `Scalar` other than the fact that it makes our
implementation simpler? Do we have other APIs which accept a `Value.Scalar` for similar purposes?
> 
> Chun-Hung Hsiao wrote:
>     Actually my point is not about the ease of implementation, but to prevent a careless
developer like me ;) to forget to do the fixed-point conversion when doing resource arithmetic.
But still, this is a pure implementation concern, not an API concern.

Capturing offline conversation, we will keep `Scalar` but add a comment asking reader to see
Scalar's comment about precision.


- Zhitao


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


On April 13, 2018, 10:20 a.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66049/
> -----------------------------------------------------------
> 
> (Updated April 13, 2018, 10:20 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
>     https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added offer operation to grow and shrink persistent volumes.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 
>   include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
>   src/tests/mesos.hpp 6f4e0c5567b99891f6d0eceb7e6917d25083db0e 
>   src/tests/persistent_volume_tests.cpp 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66049/diff/8/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message