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



Looks pretty good to me, but I'd like to get AlexR to take a look over it.
Also, do we need to rescind inverse offers here too? I'm guessing not, but I'd 
like somebody to confirm.


src/master/master.hpp (line 1047)
<https://reviews.apache.org/r/44450/#comment185597>

    "Rescind all outstanding offers if any of the weightInfos' roles has an 
active framework."
    This comment should describe what the function does, not why a particular 
caller is using it.



src/master/master.hpp (line 1049)
<https://reviews.apache.org/r/44450/#comment185599>

    s/updateWeightInfos/weightInfos/



src/master/weights_handler.cpp (lines 130 - 131)
<https://reviews.apache.org/r/44450/#comment185600>

    "If any active role is updated, we rescind all outstanding offers, to 
facilitate satisfying the updated weights."



src/master/weights_handler.cpp (line 136)
<https://reviews.apache.org/r/44450/#comment185601>

    s/in case/if/?
    "In case" tells me that updateWeights will always trigger an allocation, 
assuming that one of the roles might have had a registered framework.
    I was under the impression that updateWeights could exit without triggering 
an allocation, if no roles had registered frameworks.
    
    P.S. "In case" is not the same as "in the case of"



src/master/weights_handler.cpp (line 150)
<https://reviews.apache.org/r/44450/#comment185602>

    s/updateWeightInfos/weightInfos/



src/master/weights_handler.cpp (line 160)
<https://reviews.apache.org/r/44450/#comment185603>

    s/in case/if/



src/master/weights_handler.cpp (line 161)
<https://reviews.apache.org/r/44450/#comment185604>

    s/registered frameworks/a registered framework/


- Adam B


On March 8, 2016, 9:19 p.m., Yongqiao Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44450/
> -----------------------------------------------------------
> 
> (Updated March 8, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4881
>     https://issues.apache.org/jira/browse/MESOS-4881
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Rescind all outstanding offers to satisfy weights update.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/weights_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44450/diff/
> 
> 
> Testing
> -------
> 
> Make && Make check.
> 
> Manual test steps:
> 
> - Start Mesos master:
> $ ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master
> 
> - Start Mesos slave:
> $ ./bin/mesos-slave.sh --master=127.0.0.1:5050
> 
> - Register a framwork with `curl`
> $ curl -v http://127.0.0.1:5050/api/v1/scheduler -H "Content-type: 
> application/json" -X POST -d @subscribe.json
> $  cat subscribe.json
> {
>    "type"            : "SUBSCRIBE",
> 
>    "subscribe" : {
>       "framework_info"    : {
>           "user" :  "root",
>           "name" :  "comsumer c1 HTTP Framework",
>         "role" :  "mesos",
>         "principal":"wyq"
>       },
> 
>       "force" : true
>   }
> }
> 
> And this framework will receive an offer:
> {"subscribed":{"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-0000"}},"type":"SUBSCRIBED"}20
> {"type":"HEARTBEAT"}680
> {"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-0000"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
> {"type":"HEARTBEAT"}20
> 
> - Update the weight of role `mesos`
> $ curl --data "[{\"weight\":1.8,\"role\":\"mesos\"}]" -X PUT 
> http://127.0.0.1:5050/weights
> 
> Receive an rescind offer, and a new offer received:
> {"rescind":{"offer_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O0"}},"type":"RESCIND"}680
> {"offers":{"offers":[{"agent_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-S0"},"framework_id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-0000"},"hostname":"192.168.1.5","id":{"value":"7de42f40-2ddd-44a1-a4ff-4af932d25e02-O1"},"resources":[{"name":"cpus","role":"*","scalar":{"value":8.0},"type":"SCALAR"},{"name":"mem","role":"*","scalar":{"value":15360.0},"type":"SCALAR"},{"name":"disk","role":"*","scalar":{"value":470832.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"role":"*","type":"RANGES"}],"url":{"address":{"hostname":"192.168.1.5","ip":"192.168.1.5","port":5051},"path":"\/slave(1)","scheme":"http"}}]},"type":"OFFERS"}20
> {"type":"HEARTBEAT"}20
> 
> - Update the weight of role `mesos1`
> $ curl --data "[{\"weight\":2.8,\"role\":\"mesos1\"}]" -X PUT 
> http://127.0.0.1:5050/weights
> 
> - The outstanding offer will not be rescinded.
> 
> 
> (TODO) I will add couple of tests for this patch in another JIRA.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Reply via email to