> On Jan. 4, 2016, 2:23 a.m., Adam B wrote:
> > include/mesos/mesos.proto, line 1669
> > <https://reviews.apache.org/r/41597/diff/23/?file=1180669#file1180669line1669>
> >
> >     Let's make it `optional`, in case we later want to add weights per 
> > principal, frameworkId, etc. Experienced protobuffers say "`required` is 
> > forever" and usually only recommend one required field per protobuf 
> > message. Since this is a WeightInfo, only `weight` needs to be required.
> 
> Yongqiao Wang wrote:
>     Agree. But how to handle the case that end user does not specify role 
> (only specify weight) in the JSON when update weight? two proposal as below:
>     1. Return an error.
>     2. Update the weight of * role.
>     
>     I prefer #1 due to it can avoid the careless of end user. @adam, what is 
> your comments?

Agreed with #1. Updates must be explicit about what role they're updating, and 
a missing role is an error.


> On Jan. 4, 2016, 2:23 a.m., Adam B wrote:
> > include/mesos/mesos.proto, lines 1676-1678
> > <https://reviews.apache.org/r/41597/diff/23/?file=1180669#file1180669line1676>
> >
> >     Do we really need this message? Can't we just use `repeated WeightInfo 
> > weightInfos` in any other protobuf that needs this, or use 
> > `vector<WeightInfo>` in the allocator API?
> 
> Yongqiao Wang wrote:
>     I refered the other message difination in mesos.proto, such as Labels, 
> Ports, Credentials, Parameters, etc. in addition, this message is also used 
> in weights persist and authrization interface (Refer patch #41681 for the 
> details), and I think define this message is simple that vector<WeightInfo>.

We've started to realize that these collection wrapper messages make 
json/protobuf serialization unnecessarily more verbose, without adding any 
valuable flexibility for extending the WeightInfos message later.
Looking at patch #41681, you're actually persisting `repeated Weight weights;` 
(not sure why it isn't `repeated WeightInfo weights;`) in registry.proto. And 
the authorizer uses `ACL::UpdateWeights` instead, so neither registry nor 
authorizer use WeightInfos. I can't see any reason why you can't use 
`vector<WeightInfo>` or `RepeatedPtrField<WeightInfo>` instead.


- Adam


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


On Jan. 4, 2016, 5:52 a.m., Yongqiao Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41597/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2016, 5:52 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-3943
>     https://issues.apache.org/jira/browse/MESOS-3943
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add the interface in allocator to support updating weight
> at runtime, and the allocator is invoked to allocate the
> resources based on the updated weights later.
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp f7ada68d7111486d264284990996413bb33333d6 
>   include/mesos/mesos.proto 158e08774c4a4fa5ec667388c61e55dbdafc7f67 
>   include/mesos/v1/mesos.proto c6c5a81eb9282d188d90fe395e1c16633a2a64cf 
>   src/master/allocator/mesos/allocator.hpp 
> 50ef3b20f34bc6d87cbeccabcebec9a5031a6554 
>   src/master/allocator/mesos/hierarchical.hpp 
> 86ea5a402ed67f8f22f11d5730147cd907d66a08 
>   src/master/allocator/mesos/hierarchical.cpp 
> 7f900c4e024485704d79e57ae22407557598fe6c 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 050896e8b12cd4097ccd137d5284d6b39b0f06ab 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 3a442f121f3a1505513877a5c78458a4b8d0a824 
>   src/master/allocator/sorter/sorter.hpp 
> 7be6b44a762fd62c2cd7f28b4dc4865a4587ed26 
>   src/tests/allocator.hpp 9bdfaecf1a148f113ad52956b50ed7cabe0902ef 
> 
> Diff: https://reviews.apache.org/r/41597/diff/
> 
> 
> Testing
> -------
> 
> Make & Make check successfully!
> 
> Test case: https://reviews.apache.org/r/41672/
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Reply via email to