mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam B" <a...@mesosphere.io>
Subject Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.
Date Fri, 22 Jan 2016 09:40:30 GMT


> On Jan. 20, 2016, 1:51 a.m., Adam B wrote:
> > src/master/master.hpp, line 1577
> > <https://reviews.apache.org/r/41681/diff/28/?file=1199825#file1199825line1577>
> >
> >     Can it ever return anything but `true`? Are there any error conditions at all?
> 
> Yongqiao Wang wrote:
>     I have double checked that there are no any error conditions in our case.

Ok, looks like this function has to return a Try<bool> because that's what Operation
defines for its interface. Makes sense for your to always return 'true' in such a simple Operation.
Dropping the review issue.


> On Jan. 20, 2016, 1:51 a.m., Adam B wrote:
> > src/master/weights_handler.cpp, lines 113-115
> > <https://reviews.apache.org/r/41681/diff/28/?file=1199828#file1199828line113>
> >
> >     Why are you reconstructing a roles `string` out of the weightInfos instead of
just using a `vector`? Does `authorize()` really require a comma-delimited string?
> >     
> >     What I was proposing before was that you call `authorize(principal, role)` for
each weightInfo.role, and fail if any of those fails.
> 
> Yongqiao Wang wrote:
>     Reconstructing a roles string is only for print all roles in Master::WeightsHandler::authorize
function.
>     
>     In addition, authorize() is an asynchronous call and will return a Future<bool>,
in our logic, weights can only be updated in master, registry and allocator if authorization
passed for all specified roles. So it is complex to block the authorization for each roles,
then call _update() to updates weights in registry and registry. So I authorize all roles
together at one time.

Printing doesn't seem like a good enough reason to have to translate roles back and forth.
How about you pass a vector of roles to `authorize()` and just use `stringify` for the print
string.?

And you wouldn't need to block sequentially for the calls, just wait for all of the futures
to return. But I can see what that's unnecessary for WeightsHandler::authorize. I'll drop
that idea.


- Adam


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


On Jan. 20, 2016, 6:16 a.m., Yongqiao Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41681/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2016, 6:16 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4214
>     https://issues.apache.org/jira/browse/MESOS-4214
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduce HTTP endpoint /weights for updating weight.
> 
> 
> Diffs
> -----
> 
>   include/mesos/authorizer/authorizer.hpp 5ee3c7afadd131802c93febbb6b4dbad069c2d81 
>   include/mesos/authorizer/authorizer.proto 84727e66dd14be9a25705ab1141e92eee72fae50

>   src/CMakeLists.txt a52203ab9aa47315e6e5c58cc283a7b5df597c76 
>   src/Makefile.am 4fabe600d4ba38c95a777d622b0b675dd5811a53 
>   src/authorizer/local/authorizer.hpp c7321c276d566eca6a91f45c546468bea1b0da15 
>   src/authorizer/local/authorizer.cpp c1db9c2131ea8fbf835278203a240f108c6372c5 
>   src/master/http.cpp 34a70ee50553492fc8c3947497ab5922f4379d72 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
>   src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
>   src/master/weights_handler.cpp PRE-CREATION 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
>   src/tests/mesos.cpp 365ebe8335c37bfdb983a5424d4c995fa9b76a22 
> 
> Diff: https://reviews.apache.org/r/41681/diff/
> 
> 
> Testing
> -------
> 
> Make & Make check successfully!
> 
> $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  --weights="role1=4.2,role2=3.1"
--authenticate_http --credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1
&)
> $ curl http://localhost:5050/roles | python -mjson.tool
> {
>     "roles": [
>         {
>             "frameworks": [ ], 
>             "name": "*", 
>             "resources": {
>                 "cpus": 0, 
>                 "disk": 0, 
>                 "mem": 0
>             }, 
>             "weight": 1
>         }, 
>         {
>             "frameworks": [ ], 
>             "name": "role1", 
>             "resources": {
>                 "cpus": 0, 
>                 "disk": 0, 
>                 "mem": 0
>             }, 
>             "weight": 4.2
>         }, 
>         {
>             "frameworks": [ ], 
>             "name": "role2", 
>             "resources": {
>                 "cpus": 0, 
>                 "disk": 0, 
>                 "mem": 0
>             }, 
>             "weight": 3.1
>         }
>     ]
> }
> 
> Test update:
> $ curl --user framework1:secret_string1 --data weights="[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]"
-X PUT http://127.0.0.1:5050/weights
> $ curl http://localhost:5050/roles | python -mjson.tool
> {
>     "roles": [
>         {
>             "frameworks": [],
>             "name": "*",
>             "resources": {
>                 "cpus": 0,
>                 "disk": 0,
>                 "mem": 0
>             },
>             "weight": 1.0
>         },
>         {
>             "frameworks": [],
>             "name": "role1",
>             "resources": {
>                 "cpus": 0,
>                 "disk": 0,
>                 "mem": 0
>             },
>             "weight": 1.8
>         },
>         {
>             "frameworks": [],
>             "name": "role2",
>             "resources": {
>                 "cpus": 0,
>                 "disk": 0,
>                 "mem": 0
>             },
>             "weight": 1.0
>         },
>         {
>             "frameworks": [],
>             "name": "role3",
>             "resources": {
>                 "cpus": 0,
>                 "disk": 0,
>                 "mem": 0
>             },
>             "weight": 3.4
>         }
>     ]
> }
> 
> Test recovuery:
> $ ps -ef | grep mesos-master
> 501 56292     1   0  6:18PM ttys001    0:00.31 /Users/yqwyq/Desktop/mesos/build/src/.libs/mesos-master
--ip=127.0.0.1 --work_dir=/tmp/mesos-master --weights=role1=4.2,role2=3.1 --authenticate_http
--credentials=/opt/credentials.json
> $ kill -9 56292
> 
> $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  --weights="role1=4.2,role2=3.1,role6=9.0"
--authenticate_http --credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1
&)
> $ curl http://localhost:5050/roles | python -mjson.tool
> {
>     "roles": [
>         {
>             "frameworks": [],
>             "name": "*",
>             "resources": {
>                 "cpus": 0,
>                 "disk": 0,
>                 "mem": 0
>             },
>             "weight": 1.0
>         },
>         {
>             "frameworks": [],
>             "name": "role1",
>             "resources": {
>                 "cpus": 0,
>                 "disk": 0,
>                 "mem": 0
>             },
>             "weight": 1.8
>         },
>         {
>             "frameworks": [],
>             "name": "role2",
>             "resources": {
>                 "cpus": 0,
>                 "disk": 0,
>                 "mem": 0
>             },
>             "weight": 1.0
>         },
>         {
>             "frameworks": [],
>             "name": "role3",
>             "resources": {
>                 "cpus": 0,
>                 "disk": 0,
>                 "mem": 0
>             },
>             "weight": 3.4
>         }
>     ]
> }
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>


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