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 Mon, 22 Feb 2016 04:20:38 GMT


> On Feb. 19, 2016, 2:24 a.m., Alexander Rukletsov wrote:
> > src/master/master.hpp, lines 1530-1531
> > <https://reviews.apache.org/r/41681/diff/32/?file=1241338#file1241338line1530>
> >
> >     Let's pull at least the implementation out of the header. You may look at maintenance
and quota for inspiration : ).
> >     
> >     Going further, how about extracting registry part into a separate patch? It
will make reviewing easier. Also, it would be great to see some registry tests as well.
> 
> Yongqiao Wang wrote:
>     Thanks Alex. Personally, I find it more difficult to review a chain of commits than
a single, a single patch can give a whole picture of a feature at a time. In addition, the
main reason is Adam has completed to review this patch, if I split it into some small patches,
then maybe Adam needs to review them again before commiting them. Can we keep this patch except
make the smaller chagnes for addressing comments? can you just review the changes to each
file one at a time in the this commit? Based on your comments, I will post another patch for
registery test later.
>     
>     For moving the implementation out of the header, I will update this patches later.

I agree with Yongqioa. While patches with <100 lines are quicker/easier to review, this
patch is only about 300 new/changed lines, and I like to think I've reviewed it pretty thoroughly
already. Please review the patch as is, and let us know if there's anything for which we should
block the commit.
We can pull the implementation out of the header in a separate, follow-up patch.
We can also add additional registry tests and/or authz tests in a follow-up patch, if they
aren't already covered in https://reviews.apache.org/r/41790/
In the future, we can encourage Yongqiao to break up larger patches earlier in the review
process.


- Adam


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


On Feb. 14, 2016, 4:02 a.m., Yongqiao Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41681/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2016, 4:02 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 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7

>   src/CMakeLists.txt 9ab84c0898b3adce6063cc50b04ee74cf1471609 
>   src/Makefile.am 5813ab2c33a7de6b612064e894e5f15b5a474e2b 
>   src/authorizer/local/authorizer.hpp c7321c276d566eca6a91f45c546468bea1b0da15 
>   src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
>   src/master/http.cpp f92212bf69f9db51d729347fb553e74e28e105fd 
>   src/master/master.hpp 2f2ad2ada508e1923bf995ab124367a3b082b572 
>   src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 
>   src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
>   src/master/weights_handler.cpp PRE-CREATION 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> 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 "[{\"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