> 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
> 
>

Reply via email to