Re: Review Request 44450: Rescind all outstanding offers to satisfy weights update.

[Adam B]

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


Fix it, then Ship it!




Let's agree to remove the below check (or not?), and then I'll commit this.


src/master/weights_handler.cpp (lines 176 - 182)


Why do we need this check here? You refer to the "capacity heuristic" which 
is all the way in the quota_handler, unrelated to weights.
If a slave is disconnected or inactive, I wouldn't expect it to have any 
offers, so it'll early exit out of the foreach(offer) loop anyway.
I don't think it hurts, but I would prefer to remove it, since the logic is 
unnecessary to what you're doing here.


- Adam B


On March 22, 2016, 6:59 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44450/
> ---
> 
> (Updated March 22, 2016, 6:59 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> 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 124d43931a5c8a00ee0aaa604feb1761795209f2 
>   src/master/weights_handler.cpp c9a1b0d9adbeb1e165999cdbb4f295b24f10b18f 
> 
> 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-"}},"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-"},"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-"},"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
> 
>

Re: Review Request 45202: Add test for rescinding offer trriggered by updating weights.

[Adam B]

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



Great test for the rescind patch! Just a few cleanup suggestions


src/tests/master_allocator_tests.cpp (lines 1559 - 1561)


You could just check
```
EXPECT_EQ(Resources(framework1offers1.get()[i].resources()),
  Resources::parse(slaveFlags.resources.get()).get());
```
Then you're not planting magic numbers (2, 1024) all over the code.



src/tests/master_allocator_tests.cpp (line 1591)


Why "AtMost" instead of just `3`?



src/tests/master_allocator_tests.cpp (lines 1616 - 1618)


same as above



src/tests/master_allocator_tests.cpp (lines 1623 - 1625)


same as above



src/tests/mesos.hpp (lines 669 - 673)


Couldn't this just be:
`return strings::format("%s", JSON::protobuf(infos)).get();`?

And for that matter, is
`createUpdateRequestBody(infos)`
so much shorter than 
`strings::format("%s", JSON::protobuf(infos)).get()`
that it really needs to be its own function? I don't think think so. Just 
inline this everywhere. Example:

```
  RepeatedPtrField& infos = createWeightInfos("role1=0");
  Future response = process::http::request(
  process::http::createRequest(
  master.get()->pid,
  "PUT",
  false,
  "weights",
  createBasicAuthHeaders(DEFAULT_CREDENTIAL),
  strings::format("%s", JSON::protobuf(infos)).get()));
```


- Adam B


On March 23, 2016, 1:32 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45202/
> ---
> 
> (Updated March 23, 2016, 1:32 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4881
> https://issues.apache.org/jira/browse/MESOS-4881
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add test for rescinding offer trriggered by updating weights.
> 
> 
> Diffs
> -
> 
>   src/tests/dynamic_weights_tests.cpp 
> ee0c4b1cc20e76a35a8e4e445f6827a1fc33e6c6 
>   src/tests/master_allocator_tests.cpp 
> b41ba2bda4d680f6fc42f525719973d56c11fe31 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45202/diff/
> 
> 
> Testing
> ---
> 
> make && make check.
> 
> $ ./src/mesos-tests 
> --gtest_filter=MasterAllocatorTest/0.RebalancedForUpdatedWeights
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from MasterAllocatorTest/0, where TypeParam = 
> mesos::internal::master::allocator::MesosAllocator  mesos::internal::master::allocator::DRFSorter> >
> [ RUN  ] MasterAllocatorTest/0.RebalancedForUpdatedWeights
> [   OK ] MasterAllocatorTest/0.RebalancedForUpdatedWeights (1059 ms)
> [--] 1 test from MasterAllocatorTest/0 (1059 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (1070 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 45202: Add test for rescinding offer trriggered by updating weights.

[Yongqiao Wang]

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

(Updated March 25, 2016, 9:13 a.m.)


Review request for mesos and Adam B.


Changes
---

Addressed the comments of adam.


Bugs: MESOS-4881
https://issues.apache.org/jira/browse/MESOS-4881


Repository: mesos


Description
---

Add test for rescinding offer trriggered by updating weights.


Diffs (updated)
-

  src/tests/dynamic_weights_tests.cpp 6357cf42ec59c1388e95d6d808978918d5cd4a78 
  src/tests/master_allocator_tests.cpp b41ba2bda4d680f6fc42f525719973d56c11fe31 
  src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 

Diff: https://reviews.apache.org/r/45202/diff/


Testing
---

make && make check.

$ ./src/mesos-tests 
--gtest_filter=MasterAllocatorTest/0.RebalancedForUpdatedWeights
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from MasterAllocatorTest/0, where TypeParam = 
mesos::internal::master::allocator::MesosAllocator >
[ RUN  ] MasterAllocatorTest/0.RebalancedForUpdatedWeights
[   OK ] MasterAllocatorTest/0.RebalancedForUpdatedWeights (1059 ms)
[--] 1 test from MasterAllocatorTest/0 (1059 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (1070 ms total)
[  PASSED  ] 1 test.


Thanks,

Yongqiao Wang

Re: Review Request 44846: Deprecated the plain text credential format.

[Adam B]

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


Fix it, then Ship it!




Looks great. I'll fix up these last bits and commit it.


CHANGELOG (lines 5 - 9)


We like to put these in numerical order.



docs/configuration.md (lines 990 - 991)


The strings in configuration.md should match the strings in flags.cpp. 
Looks like this one didn't get updated with the most recent rewording.


- Adam B


On March 24, 2016, 6:22 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44846/
> ---
> 
> (Updated March 24, 2016, 6:22 a.m.)
> 
> 
> Review request for mesos, Adam B and Joerg Schad.
> 
> 
> Bugs: MESOS-2281
> https://issues.apache.org/jira/browse/MESOS-2281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated the plain text credential format.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 1fadaa1819c9b1a801752b1c0ddd572b82e6c170 
>   docs/authentication.md 9f64f657d1a01f85f0132cb7264e2b85d0d3dc59 
>   docs/configuration.md 70686adeca239b5962652a17661c8b0d44ca6708 
>   docs/upgrades.md 85d02929b58b465f22d0e9991f6cf66f52b6d9a6 
>   src/credentials/credentials.hpp aad17c21bb0ce98907bbfa22b890b66130e081e4 
>   src/master/flags.cpp 9b65dee02e3505f22aaa967899903f80fe0e7c57 
>   src/slave/flags.cpp 71685cee58322e608139f7c344c2e954f7e14322 
> 
> Diff: https://reviews.apache.org/r/44846/diff/
> 
> 
> Testing
> ---
> 
> make check
> Created the documentation using the Dockerfile in `support/site-docker` to 
> make sure that the markdown files renders correctly.
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 45317: Change Call and Event Type enums in scheduler.proto optional.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45317]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 24, 2016, 9:18 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45317/
> ---
> 
> (Updated March 24, 2016, 9:18 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5014
> https://issues.apache.org/jira/browse/MESOS-5014
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in scheduler.proto
> optional for the purpose of backwards compatibility. (MESOS-5014)
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> 0049e1383f50574c3dad6a29b91811001694e82c 
>   include/mesos/v1/scheduler/scheduler.proto 
> 09fafedd06837f2307fedc6fa0e7b4460b21e5b0 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45317/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44450: Rescind all outstanding offers to satisfy weights update.

[Yongqiao Wang]

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

(Updated March 25, 2016, 8:20 a.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Addressed comments of Adam.


Bugs: MESOS-4881
https://issues.apache.org/jira/browse/MESOS-4881


Repository: mesos


Description
---

Rescind all outstanding offers to satisfy weights update.


Diffs (updated)
-

  src/master/master.hpp 124d43931a5c8a00ee0aaa604feb1761795209f2 
  src/master/weights_handler.cpp c9a1b0d9adbeb1e165999cdbb4f295b24f10b18f 

Diff: https://reviews.apache.org/r/44450/diff/


Testing (updated)
---

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-"}},"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-"},"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-"},"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.


Thanks,

Yongqiao Wang

Re: Review Request 45014: Add /containers endpoint to return ResourceUsage.

[Jay Guo]

> On March 22, 2016, 5:39 a.m., Jie Yu wrote:
> > Instead of handling /containers endpoint in Slave, I would suggest we 
> > dispatch the request to ResourceMonitor. I will rename ResourceMonitor to 
> > ContainerMonitor, and we will be deprecating the /monitor/statistics 
> > endpoints in the future.
> 
> Jay Guo wrote:
> Thanks for reviewing! If I understand correctly, the /containers endpoint 
> itself is still `installed in slave`, although the action is `dispatched to` 
> ResourceMonitorProcess (aka ContainerMonitor) `via` ResourceMonitor (aka 
> ContainerMonitor). We will make corresponding changes if this is what you 
> have in mind.
> 
> Jay Guo wrote:
> *typo: ResourceMonitorProcess (aka ContainerMonitorProcess)
> 
> Jie Yu wrote:
> Yes, this is what I was suggesting! Thanks!

Another question: Can we refactor current `ResourceMonitorProcess` to use 
`containerizer` to collect `ResourceUsage/ContainerStatus`?

At present, collection of `ContainerStatus` would delegated back and forth 
between MonitorProcess and Slave (Slave -> ResourceMonitor -> 
ResourceMonitorProcess -> Slave), which, IMHO, is neither intuitive nor 
efficient. If ResourceMonitorProcess could possess a `containerizer`, slave 
could simply dispatch the job to ResourceMonitorProcess with a list of 
ContainerID.

I'm thinking to refactor ResourceMonitorProcess constructor to something like:
```cpp
  explicit ResourceMonitorProcess(
  Containerizer* _containerizer,
  const lambda::function& _containerIds)
: ProcessBase("monitor"),
  containerizer(_containerizer),
  containerIds(_containerIds),
  limiter(2, Seconds(1)) {} // 2 permits per second.
```

In this case, `ResourceMonitorProcess::statistics` consults slave for 
`list
`ResourceMonitorProcess::containerStatus` is invoked by slave with 
`list

This requires modification of several test cases. If you agree, we will submit 
a seperate patch of refactor.

Thanks!!


- Jay


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


On March 18, 2016, 5:27 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45014/
> ---
> 
> (Updated March 18, 2016, 5:27 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add /containers endpoint to return ResourceUsage.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/slave_tests.cpp ea1d776077bf638885db8421194aa4427c772169 
> 
> Diff: https://reviews.apache.org/r/45014/diff/
> 
> 
> Testing
> ---
> 
> make check
> `curl agent_ip:port/containers` returns same json content as `curl 
> agent_ip:port/monitor/statistics.json`
> 
> This is a draft patch of adding /containers endpoint to agent 
> [MESOS-4891](https://issues.apache.org/jira/browse/MESOS-4891)
> 
> ContainerStatus will be added to response based on this patch.
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 44500: Implemented passing image and user env var to command task.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44498, 44499, 44500]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 24, 2016, 10:01 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44500/
> ---
> 
> (Updated March 24, 2016, 10:01 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented passing image and user env var to command task.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> f97a9a92895387a9d504810a2ae971cfb5d3dbb4 
> 
> Diff: https://reviews.apache.org/r/44500/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45214: Updated protobuf to support external storage.

[Guangya Liu]

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

(Updated 三月 25, 2016, 2:07 p.m.)


Review request for mesos and Jie Yu.


Bugs: MESOS-5012
https://issues.apache.org/jira/browse/MESOS-5012


Repository: mesos


Description
---

Updated protobuf to support external storage.


Diffs (updated)
-

  include/mesos/mesos.proto cb68e2c13409620fa4836c12d877488f4333ace7 
  include/mesos/v1/mesos.proto af1dc9e11a26b52cfc348324b8dd796c1f72323f 

Diff: https://reviews.apache.org/r/45214/diff/


Testing
---


Thanks,

Guangya Liu

Re: Review Request 43935: Allow setting role in mesos-execute.

[Greg Mann]

> On March 24, 2016, 7:57 p.m., Greg Mann wrote:
> > src/cli/execute.cpp, line 215
> > 
> >
> > s/sepcified/specified/
> 
> Jian Qiu wrote:
> Sorry for the typo... please review again.

No worries :-) The patch looks good to me, perhaps mcypark can have a look when 
he has some cycles. You could contact him directly or ping him on the JIRA 
ticket to see when he'll have some time to review.


- Greg


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


On March 25, 2016, 1:54 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 25, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Neil Conway]

> On March 23, 2016, 8:21 a.m., Deshi Xiao wrote:
> > can we ignore .md scan? the .md possible container non-ascii characters.
> 
> Neil Conway wrote:
> Are non-ASCII characters in .md files actually required? We should be 
> able to use HTML character entities instead (e.g., ``) instead.
> 
> Deshi Xiao wrote:
> Neil, see this case:
> 
> quoted from haosdent's comments:
>  If disable no-ascii characters, we could not use emoji and some 
> languages which contains no-ascii characters. For the powered-by-mesos.md and 
> user-groups.html.md, this hard limit seems inconvenient.
> 
> haosdent huang wrote:
> Still could convert them to unicode code and represented by html 
> character entities. Just looks confuse and not straightforward.
> 
> Deshi Xiao wrote:
> same here same concerns

Personally I would opt for allowing only ASCII characters in .md files and 
using character entities for the (rare) cases where non-ASCII characters are 
required.


- Neil


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


On March 23, 2016, 2:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated March 23, 2016, 2:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, haosdent 
> huang, Neil Conway, and Deshi Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45327: Drop HTTP Status `404` and `307` in the scheduler library.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45327]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 25, 2016, 2:28 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45327/
> ---
> 
> (Updated March 25, 2016, 2:28 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5020
> https://issues.apache.org/jira/browse/MESOS-5020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix drops `404 Not Found` and `307 Temporary Redirect` in
> the scheduler library, instead of treating them as Event::ERROR.
> (MESOS-5020)
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 1f52a423c8c83c58d38ba6731ab2625136a271bb 
> 
> Diff: https://reviews.apache.org/r/45327/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45317: Change Call and Event Type enums in scheduler.proto optional.

[Anand Mazumdar]

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



Thanks for working on this. Added some comments.

Also, one more additional change would be needed:

Since now that the `Type` field is optional we would like to add validation on 
master to ignore the request if the `Type` field does not exist.

Here: https://github.com/apache/mesos/blob/master/src/master/validation.cpp#L61

Let's add a check like this:

```cpp
if (!call.has_type()) {
  return Error("Expecting 'type' to be present");
}
```


include/mesos/v1/scheduler/scheduler.proto (lines 36 - 37)


Let's add an identical comment to the one that exists in `mesos.proto` with 
a link to the epic. Also, In the near future it would be good to have this 
comment once per file and not being redundant.

```cpp
// This must be the first enum value in this list, to ensure that if 'type' 
is not set, the default value is UNKNOWN. This enables enum values to be added 
in a backwards-compatible way. See: MESOS-4997.
```



include/mesos/v1/scheduler/scheduler.proto (line 140)


Let's modify this line to make it identical to `mesos.proto`:

```cpp
// Enum fields should be optional, see: MESOS-4997.
```



include/mesos/v1/scheduler/scheduler.proto (lines 163 - 164)


Let's remove this redundant comment since we already added it for the 
`Event` type.

Just this comment should suffice:
```cpp
// See comments above on `Event::Type` for more details on this enum value.
```



include/mesos/v1/scheduler/scheduler.proto (line 314)


Let's modify this to:

```cpp
// See comments on `Event::Type` above on the reasoning behind this field 
being optional.
```



src/tests/mesos.hpp (lines 909 - 910)


I would like to see much more then a `break` since receiving an `UNKNOWN` 
from the master means something _bad_ has happened.

Let's add a `default` case to this `switch` i.e. something like this after 
L934.

```cpp
default:
  UNREACHABLE();
```


- Anand Mazumdar


On March 24, 2016, 9:18 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45317/
> ---
> 
> (Updated March 24, 2016, 9:18 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5014
> https://issues.apache.org/jira/browse/MESOS-5014
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in scheduler.proto
> optional for the purpose of backwards compatibility. (MESOS-5014)
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> 0049e1383f50574c3dad6a29b91811001694e82c 
>   include/mesos/v1/scheduler/scheduler.proto 
> 09fafedd06837f2307fedc6fa0e7b4460b21e5b0 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45317/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44372: Upgrade http-parser to 2.6.1 to support PowerPC LE platform.

[Vinod Kone]

> On March 24, 2016, 7:16 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [44372]
> > 
> > Failed command: ./support/apply-review.sh -n -r 44372
> > 
> > Error:
> > 2016-03-24 07:16:51 URL:https://reviews.apache.org/r/44372/diff/raw/ 
> > [104639/104639] -> "44372.patch" [1]
> > Total errors found: 0
> > Checking 1 files
> > ERROR: Commit spanning multiple projects.
> > 
> > Please use separate commits for mesos, libprocess and stout.
> > 
> > Paths grouped by project:
> > libprocess:
> >   3rdparty/libprocess/3rdparty/CMakeLists.txt
> >   3rdparty/libprocess/3rdparty/Makefile.am
> >   3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz
> >   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.patch
> >   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.tar.gz
> >   3rdparty/libprocess/3rdparty/versions.am
> >   3rdparty/libprocess/Makefile.am
> >   3rdparty/libprocess/src/decoder.hpp
> > mesos:
> >   3rdparty/cmake/Versions.cmake
> >   LICENSE
> > 
> > Full log: https://builds.apache.org/job/mesos-reviewbot/12121/console

Can you split this review as suggested by the reviewbot?


- Vinod


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


On March 24, 2016, 1:13 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44372/
> ---
> 
> (Updated March 24, 2016, 1:13 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4805
> https://issues.apache.org/jira/browse/MESOS-4805
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade http-parser to 2.6.1 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 6b07aefc58a1daa235b35e83832e47d1878e2e94 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> ddf7e3d9bf76d4a03c33f02d52ec29812aef8509 
>   3rdparty/libprocess/3rdparty/http-parser-2.6.1.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.patch 
> f9fac12437a6bedc66353fda1ce9c0d7a383225a 
>   3rdparty/libprocess/3rdparty/ry-http-parser-1c3624a.tar.gz 
> b811b63ce0ad6d71d9d296fed76656c023c76fc5 
>   3rdparty/libprocess/3rdparty/versions.am 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
>   3rdparty/libprocess/Makefile.am ac8cc8d29baccf6e3a17367540ddd1f28585ef6d 
>   3rdparty/libprocess/src/decoder.hpp 
> a20b5ba8fc50d834573d253948645cc863f030dd 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
> 
> Diff: https://reviews.apache.org/r/44372/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Review Request 45299: Stout:[1/2] transitioned codebase to use `os::pagesize`.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Stout:[1/2] transitioned codebase to use `os::pagesize`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
7bd4bfbc2ec5922879dcefddc12137336b11be52 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
6b81bf1ea3d571e8f5fb170076b85223b02efe2c 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
182ac9cfbacd4535fb355aac6ec38559d786e63a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
692fa483ee8a75dedae5ad0e7d2a4aa70dafa8a3 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/pagesize.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/pagesize.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pagesize.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
9ee233b988c08d953e70345c55bcdd5c2f7c101b 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
6a391ff198ab724f689bcef79d4e2e05a786cbc2 

Diff: https://reviews.apache.org/r/45299/diff/


Testing
---

make check


Thanks,

Alex Clemmer

Review Request 45300: Containerizer:[2/2] Transitioned codebase to use `os::pagesize`.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Containerizer:[2/2] Transitioned codebase to use `os::pagesize`.


Diffs
-

  src/examples/balloon_executor.cpp 399218478a596387814d123290eeeb33102fad7a 
  src/tests/containerizer/cgroups_tests.cpp 
acaed9b3f8a04964092cef413133834d0cf5a145 
  src/tests/containerizer/memory_test_helper.cpp 
92579a28336b3c15ba7fdbb4a9f769ac7182aeb6 

Diff: https://reviews.apache.org/r/45300/diff/


Testing
---

make check


Thanks,

Alex Clemmer

Re: Review Request 45317: Change Call and Event Type enums in scheduler.proto optional.

[Yong Tang]

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

(Updated March 25, 2016, 8:55 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Updated the CHANGELOG to reference this change for 0.29.0.


Bugs: MESOS-5014
https://issues.apache.org/jira/browse/MESOS-5014


Repository: mesos


Description
---

This fix changes Call and Event Type enums in scheduler.proto
optional for the purpose of backwards compatibility. (MESOS-5014)


Diffs (updated)
-

  CHANGELOG fef0cbfc3ae282707569429d38e52c19d4eb9aba 
  include/mesos/scheduler/scheduler.proto 
0049e1383f50574c3dad6a29b91811001694e82c 
  include/mesos/v1/scheduler/scheduler.proto 
09fafedd06837f2307fedc6fa0e7b4460b21e5b0 
  src/master/validation.cpp 701a5c4b279f319dde15bd8f2e97b5fd8608e578 
  src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 

Diff: https://reviews.apache.org/r/45317/diff/


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang

Re: Review Request 45317: Change Call and Event Type enums in scheduler.proto optional.

[Yong Tang]

> On March 25, 2016, 8:20 p.m., Vinod Kone wrote:
> > LGTM.
> > 
> > Can you update the CHANGELOG and call out this change in "Additonal API 
> > Changes" section for 0.29.0?

Thanks Vinod. Just updated the CHANGELOG.


- Yong


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


On March 25, 2016, 8:55 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45317/
> ---
> 
> (Updated March 25, 2016, 8:55 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5014
> https://issues.apache.org/jira/browse/MESOS-5014
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in scheduler.proto
> optional for the purpose of backwards compatibility. (MESOS-5014)
> 
> 
> Diffs
> -
> 
>   CHANGELOG fef0cbfc3ae282707569429d38e52c19d4eb9aba 
>   include/mesos/scheduler/scheduler.proto 
> 0049e1383f50574c3dad6a29b91811001694e82c 
>   include/mesos/v1/scheduler/scheduler.proto 
> 09fafedd06837f2307fedc6fa0e7b4460b21e5b0 
>   src/master/validation.cpp 701a5c4b279f319dde15bd8f2e97b5fd8608e578 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45317/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45000: MESOS-3902: [Updated] Fix location header in redirect from non-leader.

[Ashwin Murthy]

Thanks!

On Fri, Mar 25, 2016 at 10:18 AM, Vinod Kone  wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45000/
>
> Ship it!
>
> Thanks for following up with a real test!
>
> Btw, no need for that NOTE as the bug is attached to this review and the 
> commit will be pasted to that ticket. I'll remove it while committing.
>
>
> - Vinod Kone
>
> On March 24th, 2016, 2:24 a.m. UTC, Ashwin Murthy wrote:
> Review request for mesos and Vinod Kone.
> By Ashwin Murthy.
>
> *Updated March 24, 2016, 2:24 a.m.*
> *Repository: * mesos
> Description
>
> MESOS-3902: [Final] Fix location header in redirect from non-leader. Fixed to 
> append the path from the original URL.
>
> Testing
>
> This has been tested end to end with 3 local mesos masters and zk. Valided 
> the location header is now correct.
>
> Diffs
>
>- src/master/http.cpp (97e4b0ce3286540788e3e6c5b484687b83e000f6)
>
> View Diff 
>

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Anand Mazumdar]

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



Can you also make a similar change as  https://reviews.apache.org/r/45317/ to 
`src/slave/validation.cpp` if `Call::Type` is not present.

- Anand Mazumdar


On March 25, 2016, 6:58 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45304/
> ---
> 
> (Updated March 25, 2016, 6:58 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5015
> https://issues.apache.org/jira/browse/MESOS-5015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in executor.proto
> optional for the purpose of backwards compatibility. (MESOS-5015)
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 
> 6b4141f0aac27be5a4998f16e986fa87a7047834 
>   include/mesos/v1/executor/executor.proto 
> d7b1dd5eec9bcd424c7859d89471148bce8fe16e 
>   src/examples/test_http_executor.cpp 
> 562b0acfd8555b9b773175f53defe0e7e2744641 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45304/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Yong Tang]

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

(Updated March 25, 2016, 9:19 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Updated src/slave/validation.cpp and CHANGELOG based on the feedback from Anand 
and Vinod.


Bugs: MESOS-5015
https://issues.apache.org/jira/browse/MESOS-5015


Repository: mesos


Description
---

This fix changes Call and Event Type enums in executor.proto
optional for the purpose of backwards compatibility. (MESOS-5015)


Diffs (updated)
-

  CHANGELOG fef0cbfc3ae282707569429d38e52c19d4eb9aba 
  include/mesos/executor/executor.proto 
6b4141f0aac27be5a4998f16e986fa87a7047834 
  include/mesos/v1/executor/executor.proto 
d7b1dd5eec9bcd424c7859d89471148bce8fe16e 
  src/examples/test_http_executor.cpp 562b0acfd8555b9b773175f53defe0e7e2744641 
  src/slave/validation.cpp bc8d6717eac103c41f8cc8720e8482589210ea72 
  src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 

Diff: https://reviews.apache.org/r/45304/diff/


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang

Re: Review Request 45317: Change Call and Event Type enums in scheduler.proto optional.

[Anand Mazumdar]

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


Ship it!




LGTM

- Anand Mazumdar


On March 25, 2016, 5:56 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45317/
> ---
> 
> (Updated March 25, 2016, 5:56 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5014
> https://issues.apache.org/jira/browse/MESOS-5014
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in scheduler.proto
> optional for the purpose of backwards compatibility. (MESOS-5014)
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> 0049e1383f50574c3dad6a29b91811001694e82c 
>   include/mesos/v1/scheduler/scheduler.proto 
> 09fafedd06837f2307fedc6fa0e7b4460b21e5b0 
>   src/master/validation.cpp 701a5c4b279f319dde15bd8f2e97b5fd8608e578 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45317/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Yong Tang]

> On March 25, 2016, 9:23 p.m., Vinod Kone wrote:
> > Ship It!
> 
> Vinod Kone wrote:
> Can you rebase?

Just rebased the review request. Thanks a lot!


- Yong


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


On March 25, 2016, 9:25 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45304/
> ---
> 
> (Updated March 25, 2016, 9:25 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5015
> https://issues.apache.org/jira/browse/MESOS-5015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in executor.proto
> optional for the purpose of backwards compatibility. (MESOS-5015)
> 
> 
> Diffs
> -
> 
>   CHANGELOG cd551d896e8ba3ec9df17cc477f0abdd87917ab3 
>   include/mesos/executor/executor.proto 
> 6b4141f0aac27be5a4998f16e986fa87a7047834 
>   include/mesos/v1/executor/executor.proto 
> d7b1dd5eec9bcd424c7859d89471148bce8fe16e 
>   src/examples/test_http_executor.cpp 
> 562b0acfd8555b9b773175f53defe0e7e2744641 
>   src/slave/validation.cpp bc8d6717eac103c41f8cc8720e8482589210ea72 
>   src/tests/mesos.hpp 23694885a69ddcbc7039de1186093ce0ad5eed22 
> 
> Diff: https://reviews.apache.org/r/45304/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45073: Restructured authentication.md to group common flags.

[Greg Mann]

> On March 20, 2016, 5:44 p.m., Greg Mann wrote:
> > Personally, I'm not convinced that this is a useful way to format our flag 
> > documentation. I've been meaning to propose eliminating it from 
> > 'configuration.md'. I think it's more confusing to have the lists of flags 
> > broken up into multiple pieces. I'm guessing that most users come to the 
> > documentation either looking for a specific flag of either the master or 
> > agent, or they want to look over the entire list of flags for either the 
> > master or agent. In either case, I think that splitting up the flags makes 
> > it a little harder for them to find what they want. Thoughts?
> 
> Joerg Schad wrote:
> From I software engineering perspective I hate to duplicate information 
> which I then have to keep in sync :-). Secondly, from a user perspective 
> (imagine me writing some chef/puppet scripts to setupmy cluster) it is nice 
> to know which flags are applicable for both.

I do see your point about duplicated information. However, I guess I don't see 
the benefit from the user perspective: if I'm writing scripts to set up my 
cluster, at a given moment I'm probably looking for flags for _either_ the 
master _or_ the agent, so I would expect that it's a bit easier to just have 
all the flags for the relevant binary in one place?


- Greg


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


On March 19, 2016, 11:51 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45073/
> ---
> 
> (Updated March 19, 2016, 11:51 a.m.)
> 
> 
> Review request for mesos, Adam B and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Restructured authentication.md to group common flags.
> 
> 
> Diffs
> -
> 
>   docs/authentication.md e7c0bf3ed331411f607e7622419f14853006a480 
> 
> Diff: https://reviews.apache.org/r/45073/diff/
> 
> 
> Testing
> ---
> 
> viewed via gist: https://gist.github.com/joerg84/63e5919fef724b5c3508 and 
> docker website container
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Yong Tang]

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

(Updated March 25, 2016, 9:25 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Rebase the review request.


Bugs: MESOS-5015
https://issues.apache.org/jira/browse/MESOS-5015


Repository: mesos


Description
---

This fix changes Call and Event Type enums in executor.proto
optional for the purpose of backwards compatibility. (MESOS-5015)


Diffs (updated)
-

  CHANGELOG cd551d896e8ba3ec9df17cc477f0abdd87917ab3 
  include/mesos/executor/executor.proto 
6b4141f0aac27be5a4998f16e986fa87a7047834 
  include/mesos/v1/executor/executor.proto 
d7b1dd5eec9bcd424c7859d89471148bce8fe16e 
  src/examples/test_http_executor.cpp 562b0acfd8555b9b773175f53defe0e7e2744641 
  src/slave/validation.cpp bc8d6717eac103c41f8cc8720e8482589210ea72 
  src/tests/mesos.hpp 23694885a69ddcbc7039de1186093ce0ad5eed22 

Diff: https://reviews.apache.org/r/45304/diff/


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang

Re: Review Request 45290: Added a test for the '/files/debug' endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44678, 44703, 44515, 44523, 44553, 44554, 45088, 45248, 
45249, 45290]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 25, 2016, 4:42 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45290/
> ---
> 
> (Updated March 25, 2016, 4:42 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This endpoint previously had no test coverage, so a test was added that hits 
> the endpoint and checks for the expected output.
> 
> 
> Diffs
> -
> 
>   src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e 
> 
> Diff: https://reviews.apache.org/r/45290/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="FilesTest.AuthenticationTest" bin/mesos-tests.sh 
> --gtest_repeat=1000 --gtest_break_on_failure=1` was used to test on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Yong Tang]

> On March 25, 2016, 8:18 p.m., Anand Mazumdar wrote:
> > Can you also make a similar change as  https://reviews.apache.org/r/45317/ 
> > to `src/slave/validation.cpp` if `Call::Type` is not present.

Hi Anand, the review request has been updated. Let me know if there are other 
issues. Again, thanks a lot for the review.


- Yong


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


On March 25, 2016, 9:19 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45304/
> ---
> 
> (Updated March 25, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5015
> https://issues.apache.org/jira/browse/MESOS-5015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in executor.proto
> optional for the purpose of backwards compatibility. (MESOS-5015)
> 
> 
> Diffs
> -
> 
>   CHANGELOG fef0cbfc3ae282707569429d38e52c19d4eb9aba 
>   include/mesos/executor/executor.proto 
> 6b4141f0aac27be5a4998f16e986fa87a7047834 
>   include/mesos/v1/executor/executor.proto 
> d7b1dd5eec9bcd424c7859d89471148bce8fe16e 
>   src/examples/test_http_executor.cpp 
> 562b0acfd8555b9b773175f53defe0e7e2744641 
>   src/slave/validation.cpp bc8d6717eac103c41f8cc8720e8482589210ea72 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45304/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Yong Tang]

> On March 25, 2016, 8:22 p.m., Vinod Kone wrote:
> > LGTM modulo Anand's comments. Please also update the CHANGELOG.

Hi Vinod, the CHANGELOG has been updated. Let me know if there are other issues 
and thanks a lot for the review.


- Yong


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


On March 25, 2016, 9:19 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45304/
> ---
> 
> (Updated March 25, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5015
> https://issues.apache.org/jira/browse/MESOS-5015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in executor.proto
> optional for the purpose of backwards compatibility. (MESOS-5015)
> 
> 
> Diffs
> -
> 
>   CHANGELOG fef0cbfc3ae282707569429d38e52c19d4eb9aba 
>   include/mesos/executor/executor.proto 
> 6b4141f0aac27be5a4998f16e986fa87a7047834 
>   include/mesos/v1/executor/executor.proto 
> d7b1dd5eec9bcd424c7859d89471148bce8fe16e 
>   src/examples/test_http_executor.cpp 
> 562b0acfd8555b9b773175f53defe0e7e2744641 
>   src/slave/validation.cpp bc8d6717eac103c41f8cc8720e8482589210ea72 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45304/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45342: Make the Action enum optional to support upgrades (MESOS-5031).

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45342]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 25, 2016, 8:13 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45342/
> ---
> 
> (Updated March 25, 2016, 8:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, and Vinod Kone.
> 
> 
> Bugs: MESOS-5031
> https://issues.apache.org/jira/browse/MESOS-5031
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix tries to make the Action enum in Authorization optional
> to support upgrades. See MESOS-4997 for details.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 944a493e0979c7ffbd99f3a67785a10425fd9040 
>   src/authorizer/local/authorizer.cpp 
> 0f0d9276337858984f0b19a82ffca74ee84dc650 
> 
> Diff: https://reviews.apache.org/r/45342/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Vinod Kone]

> On March 25, 2016, 9:23 p.m., Vinod Kone wrote:
> > Ship It!

Can you rebase?


- Vinod


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


On March 25, 2016, 9:19 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45304/
> ---
> 
> (Updated March 25, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5015
> https://issues.apache.org/jira/browse/MESOS-5015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in executor.proto
> optional for the purpose of backwards compatibility. (MESOS-5015)
> 
> 
> Diffs
> -
> 
>   CHANGELOG fef0cbfc3ae282707569429d38e52c19d4eb9aba 
>   include/mesos/executor/executor.proto 
> 6b4141f0aac27be5a4998f16e986fa87a7047834 
>   include/mesos/v1/executor/executor.proto 
> d7b1dd5eec9bcd424c7859d89471148bce8fe16e 
>   src/examples/test_http_executor.cpp 
> 562b0acfd8555b9b773175f53defe0e7e2744641 
>   src/slave/validation.cpp bc8d6717eac103c41f8cc8720e8482589210ea72 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45304/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 25, 2016, 9:19 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45304/
> ---
> 
> (Updated March 25, 2016, 9:19 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5015
> https://issues.apache.org/jira/browse/MESOS-5015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in executor.proto
> optional for the purpose of backwards compatibility. (MESOS-5015)
> 
> 
> Diffs
> -
> 
>   CHANGELOG fef0cbfc3ae282707569429d38e52c19d4eb9aba 
>   include/mesos/executor/executor.proto 
> 6b4141f0aac27be5a4998f16e986fa87a7047834 
>   include/mesos/v1/executor/executor.proto 
> d7b1dd5eec9bcd424c7859d89471148bce8fe16e 
>   src/examples/test_http_executor.cpp 
> 562b0acfd8555b9b773175f53defe0e7e2744641 
>   src/slave/validation.cpp bc8d6717eac103c41f8cc8720e8482589210ea72 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45304/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45317: Change Call and Event Type enums in scheduler.proto optional.

[Vinod Kone]

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



LGTM.

Can you update the CHANGELOG and call out this change in "Additonal API 
Changes" section for 0.29.0?

- Vinod Kone


On March 25, 2016, 5:56 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45317/
> ---
> 
> (Updated March 25, 2016, 5:56 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5014
> https://issues.apache.org/jira/browse/MESOS-5014
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in scheduler.proto
> optional for the purpose of backwards compatibility. (MESOS-5014)
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> 0049e1383f50574c3dad6a29b91811001694e82c 
>   include/mesos/v1/scheduler/scheduler.proto 
> 09fafedd06837f2307fedc6fa0e7b4460b21e5b0 
>   src/master/validation.cpp 701a5c4b279f319dde15bd8f2e97b5fd8608e578 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45317/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45120: Extended `os::rmdir` in stout to support preserving the root directory.

[Jie Yu]

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




3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp (lines 36 - 
37)


Can you put each parameter in its own line? I find it more readable:)



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp (line 65)


Instead of using path comparison, can we use fts_level instead?



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 114)


Ditto. Can you put each parameter in its own line?: )


- Jie Yu


On March 21, 2016, 6:43 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45120/
> ---
> 
> (Updated March 21, 2016, 6:43 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows `os::rmdir("/x")` to optionally remove all the child
> files and directories of the root directory "/x", but preserve "/x"
> itself.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cbc97596cd8ed1e6d4261568fd0086c86e063232 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> 4e3dd0ff8cbfe470b14c7cacbd861e7dc8384ba1 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> daa46e05d113fd62ea9dad042ec41aaab28ad003 
> 
> Diff: https://reviews.apache.org/r/45120/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43883: Added a metric for querying the number offer filters for a role.

[Ben Mahler]

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



The code looks pretty clean, thanks! However, there were two bugs here:

(1) The value of the metric only counts how many slaves have offer filters, 
rather than the total number of filters (note the value type of `offerFilters` 
is a set).
(2) removeRole was not removing the role from the map of gauges

I made some minor adjustments based on the comments here and fixed these two 
bugs, will commit shortly!


docs/monitoring.md (line 955)


In the interest of consistency and avoiding repeating ourselves in the 
name, perhaps the following?

```
allocator/mesos/filters/roles//active
```



src/master/allocator/mesos/hierarchical.hpp (line 298)


Hm.. why the "role" naming prefix here but no "role" naming prefix on 
`_quota_allocated`?



src/master/allocator/mesos/hierarchical.cpp (line 1742)


2 spaces here



src/master/allocator/mesos/hierarchical.cpp (line 1743)


How about wrapping the argument on the next line here for consistency?



src/master/allocator/mesos/hierarchical.cpp (line 1747)


whoops, extra newline?



src/master/allocator/mesos/hierarchical.cpp (line 1753)


Whoops, this looks like a bug. `offerFilters` is a `hashmap>` and so it looks like we need to loop over the values 
and use the sum of the sizes, since there may be more than one filter per slave:

```
foreachkey (const SlaveID& slaveId, framework.offerFilters) {
  result += framework.offerFilters[slaveId].size();
}
```



src/master/allocator/mesos/metrics.hpp (lines 51 - 52)


Very nice!



src/master/allocator/mesos/metrics.cpp (line 113)


s/(/ (/



src/master/allocator/mesos/metrics.cpp (line 175)


`offer_filter` here but `gauge` in removeRole?



src/master/allocator/mesos/metrics.cpp (line 185)


2 spaces here



src/master/allocator/mesos/metrics.cpp (lines 188 - 192)


This looks like a bug: we also have to remove the role from the map here.



src/tests/hierarchical_allocator_tests.cpp (line 568)


Since the metric is declared higher up it's a bit tricker to follow what is 
being checked here, how about s/metric/filters_active/ ?



src/tests/hierarchical_allocator_tests.cpp (lines 2655 - 2657)


Thanks for the test!



src/tests/hierarchical_allocator_tests.cpp (lines 2700 - 2707)


Some newlines here would be nice:

```
  Future allocation = allocations.get();

  AWAIT_READY(allocation);
  ASSERT_EQ(framework1.id(), allocation->frameworkId);

  allocator->recoverResources(
  allocation->frameworkId,
  agent.id(),
  allocation->resources.get(agent.id()).get(),
  offerFilter);
```


- Ben Mahler


On March 24, 2016, 1:36 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> ---
> 
> (Updated March 24, 2016, 1:36 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
> https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a metric for querying the number offer filters for a role.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md dcf19e6ce06b02373a2bd1af81a451a35743fa76 
>   src/master/allocator/mesos/hierarchical.hpp 
> e4604f4da59166da24709a68b8cd4e56bf55f97f 
>   src/master/allocator/mesos/hierarchical.cpp 
> 39a290d0db2c22e179a8f933b1a78e3a2dcefdc3 
>   src/master/allocator/mesos/metrics.hpp 
> b5f2806cff99ee2ee46c4ac8a13174ef699969aa 
>   src/master/allocator/mesos/metrics.cpp 
> 2f3012a5bd40a4100e790e1d373832015c80b993 
>   src/tests/hierarchical_allocator_tests.cpp 
> e9cfcfc0ad8b0b89bbac459b7db39183f6c189be 
> 
> Diff: https://reviews.apache.org/r/43883/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 

Re: Review Request 45122: Updated tests for deletion of persistent volumes.

[Jie Yu]

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




src/tests/persistent_volume_tests.cpp (line 243)


Can we create a new test, instead of piggyback on this test? In the new 
test, you can test the 'creation' of persistent volume as well for PATH type 
disk.



src/tests/persistent_volume_tests.cpp (line 387)


Do you want to check if MOUNT disk is empty?



src/tests/persistent_volume_tests.cpp (line 809)


What the purpose of adding another file here?



src/tests/persistent_volume_tests.cpp (line 848)


Ditto. Check if MOUTN disk is empty.


- Jie Yu


On March 21, 2016, 6:44 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45122/
> ---
> 
> (Updated March 21, 2016, 6:44 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated tests for deletion of persistent volumes.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 26fff19daa8b175fdcc06fd9467224d5920a1967 
> 
> Diff: https://reviews.apache.org/r/45122/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44376: [WIP]Upgrade zookeeper to 3.4.8 to support PowerPC LE platform.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44376]

Failed command: ./support/apply-review.sh -n -r 44376

Error:
2016-03-25 23:32:22 URL:https://reviews.apache.org/r/44376/diff/raw/ 
[9344/9344] -> "44376.patch" [1]
error: unrecognized binary patch at line 152
Total errors found: 0
Checking 1 files
Error: Commit message summary (the first line) must start with a capital letter.

Full log: https://builds.apache.org/job/mesos-reviewbot/12145/console

- Mesos ReviewBot


On March 24, 2016, 12:33 p.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44376/
> ---
> 
> (Updated March 24, 2016, 12:33 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4612
> https://issues.apache.org/jira/browse/MESOS-4612
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade zookeeper to 3.4.8 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 49aa55741d76aa88c8fbb526f18908312bb0c717 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   3rdparty/versions.am c2dae2fb521b12344b93bf771dd5497ba8d446c3 
>   3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 
>   3rdparty/zookeeper-3.4.5.tar.gz 1a547fe17a6fad86012f855d3c4cc38fed4899fc 
>   3rdparty/zookeeper-3.4.8.patch PRE-CREATION 
>   src/examples/java/test-log.in 4c8547aaa115779ae7cec58edde01ab85feeb1b1 
>   src/python/native_common/ext_modules.py.in 
> c335bd83024bc07b6243dd59d775e7f29adc7520 
>   src/tests/zookeeper_test_server.cpp 
> 0dc041fef8973d35114b9f76a6a4002853884670 
> 
> Diff: https://reviews.apache.org/r/44376/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45290: Added a test for the '/files/debug' endpoint.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44678, 44703, 44515, 44523, 44553, 44554, 45088, 45248, 
45249, 45290]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:14.04' 
CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker_build.sh 2>&1 | tee build_45290"]

Error:
+ : ubuntu:14.04
+ : gcc
+ : --verbose
+ : GLOG_v=1 MESOS_VERBOSE=1
+++ dirname ./support/docker_build.sh
++ cd ./support/..
++ pwd
+ MESOS_DIRECTORY=/home/jenkins/jenkins-slave/workspace/mesos-reviewbot
+ cd /home/jenkins/jenkins-slave/workspace/mesos-reviewbot
+ DOCKERFILE=Dockerfile
+ rm -f Dockerfile
+ case $OS in
+ append_dockerfile 'FROM ubuntu:14.04'
+ echo FROM ubuntu:14.04
+ append_dockerfile 'RUN rm -rf /var/lib/apt/lists/*'
+ echo RUN rm -rf 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_main_binary-amd64_Packages 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_main_binary-i386_Packages 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_Release 
/var/lib/apt/lists/get.docker.io_ubuntu_dists_docker_Release.gpg 
/var/lib/apt/lists/lock 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_main_binary-amd64_Packages
 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_main_binary-i386_Packages
 /var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_Release 
/var/lib/apt/lists/packages.apache.org_asf%5finternal_dists_trusty_Release.gpg 
/var/lib/apt/lists/partial 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_binary-amd64_Packages
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_binary-i386_Packages
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_i18n_Translation-en
 /va
 
r/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_main_source_Sources
 
/var/lib/apt/lists/ppa.launchpad.net_webupd8team_java_ubuntu_dists_trusty_Release
 /var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_InRelease 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_main_source_Sources
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiverse_i18n_Translation-en
 /var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_multiver
 se_source_Sources 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_restricted_source_Sources
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_binary-amd64_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_binary-i386_Packages
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_i18n_Translation-en
 
/var/lib/apt/lists/security.ubuntu.com_ubuntu_dists_trusty-security_universe_source_Sources
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_InRelease
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_binary-amd64_Packages
 /var/lib/apt/lists/u
 s.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_binary-i386_Packages 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_i18n_Translation-en
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_main_source_Sources
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_binary-amd64_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_binary-i386_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_i18n_Translation-en
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_multiverse_source_Sources
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_restricted_binary-amd64_Packages
 
/var/lib/apt/lists/us.archive.ubuntu.com_ubuntu_dists_trusty-backports_restricted_binary-i386_Packages
 

Re: Review Request 45290: Added a test for the '/files/debug' endpoint.

[Greg Mann]

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

(Updated March 26, 2016, 12:07 a.m.)


Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.


Changes
---

Rebase.


Repository: mesos


Description
---

This endpoint previously had no test coverage, so a test was added that hits 
the endpoint and checks for the expected output.


Diffs (updated)
-

  src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e 

Diff: https://reviews.apache.org/r/45290/diff/


Testing
---

`GTEST_FILTER="FilesTest.AuthenticationTest" bin/mesos-tests.sh 
--gtest_repeat=1000 --gtest_break_on_failure=1` was used to test on OSX.


Thanks,

Greg Mann

Re: Review Request 44606: Returning `202` (Accepted) for /reserve and related endpoints.

[Kevin Klues]

> On March 18, 2016, 6:51 p.m., Vinod Kone wrote:
> > Looks good to me. Couple of things before this can get committed.
> > 
> > --> Have you sent an email to dev/user list about this backwards 
> > incompatible change? If not, you should.
> >  
> > --> If users are depending on the return code (need to ask on the above 
> > email) we need to do a proper deprecation warning in 0.29.0 CHANGELOG 
> > (Deprecations section) and change the behavior in 0.30.0.
> > 
> > --> If no one is depending on the return code, we might just do the change 
> > in 0.29.0. This should go into the 0.29.0 CHANGELOG though (API Changes 
> > section).
> 
> zhou xing wrote:
> Thanks for the review, we have sent a message to the community to see 
> wether anyone are using the return code. will update you the latest feedbacks 
> we collect, thx

I Zhao, I'm curious if you got any feedback on this.  Also, did you get a 
chance to address Neil's comments about updating the documentation? I think 
that would need to be included as part of this work, though I'd probably submit 
it as a separate patch that "Depends On" this one.


- Kevin


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


On March 10, 2016, 7:11 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44606/
> ---
> 
> (Updated March 10, 2016, 7:11 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Neil Conway, Qian Zhang, and Vinod 
> Kone.
> 
> 
> Bugs: mesos-4580
> https://issues.apache.org/jira/browse/mesos-4580
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modify the return code of the following endpoints to 202:
> 1. /reserve
> 2. /unreserve
> 3. /create-volumes
> 4. /destroy-volumes
> 
> [#MESOS-4580]
> 
> Signed-off-by: Guo Jiannan 
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/create-volumes.md 
> 542f555d9b6f07bde58d49ab1bc408b4d0aa7b9f 
>   docs/endpoints/master/destroy-volumes.md 
> d5d98198e70fae4f6ea6791511f3b26e792f66d1 
>   docs/endpoints/master/reserve.md 3e2a857ce784496d4de872bd00b1560c058b667d 
>   docs/endpoints/master/unreserve.md d26ae7cb7ec7a2c75ac45b792213fe8d82e8929d 
>   docs/persistent-volume.md 4b9c59daf6fdcee4a102e19d6eb4df9b5eddfa54 
>   docs/reservation.md 55924adb94028702e15db7e191915157552981d0 
>   docs/upgrades.md e888b233351b2da05a5e5c63138de5f60708afea 
>   src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 81185a161498394020a27f1f5bf747bac5425f43 
>   src/tests/reservation_endpoints_tests.cpp 
> f95ae7a32c3809d150adf1e9e515a3b527e61699 
> 
> Diff: https://reviews.apache.org/r/44606/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 44499: Added task_environment flag/used execvpe for command executor.

[Guangya Liu]

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




src/launcher/executor.cpp (line 311)


Just a question here: Does the os::raw::environment() will be overwritten 
by taskEnvironment? What is the use of `os::raw::environment()` here?


- Guangya Liu


On 三月 24, 2016, 9:53 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44499/
> ---
> 
> (Updated 三月 24, 2016, 9:53 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4781
> https://issues.apache.org/jira/browse/MESOS-4781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added task_environment flag/used execvpe for command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 4e9b4d9820f7c2f4cb1b3e16e2f4c8c13500693f 
> 
> Diff: https://reviews.apache.org/r/44499/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45033: Add a commit hook for checking non-ascii characters (MESOS-4033).

[Deshi Xiao]

> On 三月 23, 2016, 8:21 a.m., Deshi Xiao wrote:
> > can we ignore .md scan? the .md possible container non-ascii characters.
> 
> Neil Conway wrote:
> Are non-ASCII characters in .md files actually required? We should be 
> able to use HTML character entities instead (e.g., ``) instead.
> 
> Deshi Xiao wrote:
> Neil, see this case:
> 
> quoted from haosdent's comments:
>  If disable no-ascii characters, we could not use emoji and some 
> languages which contains no-ascii characters. For the powered-by-mesos.md and 
> user-groups.html.md, this hard limit seems inconvenient.
> 
> haosdent huang wrote:
> Still could convert them to unicode code and represented by html 
> character entities. Just looks confuse and not straightforward.
> 
> Deshi Xiao wrote:
> same here same concerns
> 
> Neil Conway wrote:
> Personally I would opt for allowing only ASCII characters in .md files 
> and using character entities for the (rare) cases where non-ASCII characters 
> are required.

if we can strict the md only contain ASCII characters, the opt for is fine. so 
here we need a shepherd to help us guideline the way.


- Deshi


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


On 三月 23, 2016, 2:46 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45033/
> ---
> 
> (Updated 三月 23, 2016, 2:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, haosdent 
> huang, Neil Conway, and Deshi Xiao.
> 
> 
> Bugs: MESOS-4033
> https://issues.apache.org/jira/browse/MESOS-4033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review request tries to add a commit hook for checking non-ascii
> characters. It scans .cpp, .hpp, .cc, .h files and report
> error if non-ascii characters exists.
> 
> As part of this review request, two non-ascii characters are identified
> in versioning.md (one in Ln 85 and another in Ln 96) and are corrected
> accordingly.
> 
> Note: .md scan is skipped based on feedback from review request.
> 
> 
> Diffs
> -
> 
>   docs/versioning.md ecacd8433f0fa1643827b36d03154042538c1c6b 
>   support/hooks/post-rewrite e3747a320fef0b71c06bcf0f2c5532958c416646 
>   support/hooks/pre-commit 10838a4c99db2a8318d64f95d90d2c2c90150453 
>   support/non-ascii.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45033/diff/
> 
> 
> Testing
> ---
> 
> Tested manually and found two non ascii characters in docs/versioning.md 
> (fixed as part of this review request).
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44257: Upgrade protobuf to 2.6.1 to support PowerPC LE platform.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [44257]

Failed command: ./support/apply-review.sh -n -r 44257

Error:
2016-03-26 00:00:57 URL:https://reviews.apache.org/r/44257/diff/raw/ 
[6587179/6587179] -> "44257.patch" [1]
No files to lint

ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
stout:
  3rdparty/libprocess/3rdparty/stout/README.md
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h
libprocess:
  3rdparty/libprocess/3rdparty/protobuf-2.5.0.tar.gz
  3rdparty/libprocess/3rdparty/protobuf-2.6.1.tar.gz
  3rdparty/libprocess/3rdparty/versions.am
mesos:
  3rdparty/cmake/Versions.cmake
  LICENSE
  configure.ac
  src/java/mesos.pom.in
  src/python/interface/setup.py.in
  src/python/native_common/ext_modules.py.in
  src/python/protocol/setup.py.in

Full log: https://builds.apache.org/job/mesos-reviewbot/12146/console

- Mesos ReviewBot


On March 24, 2016, 1:28 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44257/
> ---
> 
> (Updated March 24, 2016, 1:28 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4678
> https://issues.apache.org/jira/browse/MESOS-4678
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade protobuf to 2.6.1 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 24490d399f20b31b6336e92d1bd5d9a7230f31f7 
>   3rdparty/libprocess/3rdparty/protobuf-2.5.0.tar.gz 
> e600ac57be4c88efb5f146e4b3ec226d8f685033 
>   3rdparty/libprocess/3rdparty/protobuf-2.6.1.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/README.md 
> c534835db7baca1138791f2c700e95ff73052d85 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 
> 3d1f13082a65f9b1694ee7c65ba0cec131c18c5a 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 
> fb11b1147b3a1872f60e90d0691723f9b2985427 
>   3rdparty/libprocess/3rdparty/versions.am 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
>   configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1f2d19ffb8 
>   src/java/mesos.pom.in 7615d61eb6fedfa0ead785cd360946c56ccf80af 
>   src/python/interface/setup.py.in d73996734c3a3c70c3a6c0c697bb6733c241c091 
>   src/python/native_common/ext_modules.py.in 
> c335bd83024bc07b6243dd59d775e7f29adc7520 
>   src/python/protocol/setup.py.in 4c50fbbf1ce11c4c42c848364523225ee7ea5a3b 
> 
> Diff: https://reviews.apache.org/r/44257/diff/
> 
> 
> Testing
> ---
> 
> sudo make dist check
> 
> sudo ./src/mesos-tests --benchmark
> 
> sudo ./support/run-upgrade.py --prev=../mesos-0.27.0/build --next=./build
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 45120: Extended `os::rmdir` in stout to support preserving the root directory.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 26, 2016, 12:25 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45120/
> ---
> 
> (Updated March 26, 2016, 12:25 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows `os::rmdir("/x")` to optionally remove all the child
> files and directories of the root directory "/x", but preserve "/x"
> itself.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> 44a59b830f4a09c9a19d55d3e99154ca8e4d2593 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> 4e3dd0ff8cbfe470b14c7cacbd861e7dc8384ba1 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> 5466991675778c59d8040946717b492cdb1f9b14 
> 
> Diff: https://reviews.apache.org/r/45120/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45120: Extended `os::rmdir` in stout to support preserving the root directory.

[Neil Conway]

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

(Updated March 26, 2016, 12:25 a.m.)


Review request for mesos, Jie Yu and Joris Van Remoortere.


Changes
---

Address review comments, rebase.


Bugs: MESOS-2408
https://issues.apache.org/jira/browse/MESOS-2408


Repository: mesos


Description
---

This allows `os::rmdir("/x")` to optionally remove all the child
files and directories of the root directory "/x", but preserve "/x"
itself.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
44a59b830f4a09c9a19d55d3e99154ca8e4d2593 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
4e3dd0ff8cbfe470b14c7cacbd861e7dc8384ba1 
  3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
5466991675778c59d8040946717b492cdb1f9b14 

Diff: https://reviews.apache.org/r/45120/diff/


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 45123: Updated docs for deletion of persistent volumes.

[Neil Conway]

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

(Updated March 26, 2016, 12:25 a.m.)


Review request for mesos, Jie Yu and Joris Van Remoortere.


Changes
---

Rebase.


Bugs: MESOS-2408
https://issues.apache.org/jira/browse/MESOS-2408


Repository: mesos


Description
---

Updated docs for deletion of persistent volumes.


Diffs (updated)
-

  docs/multiple-disk.md 5cdce26721212597052515a9cf2fd9277d24e42c 
  docs/persistent-volume.md 4b9c59daf6fdcee4a102e19d6eb4df9b5eddfa54 
  docs/reservation.md 55924adb94028702e15db7e191915157552981d0 
  docs/upgrades.md 1b683b59b981ec7e4664c751cb57ab848c7506dc 

Diff: https://reviews.apache.org/r/45123/diff/


Testing
---


Thanks,

Neil Conway

Re: Review Request 45121: Implemented deletion for persistent volumes.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/slave.cpp (line 2333)


insert a space before 'at'



src/slave/slave.cpp (lines 2344 - 2347)


This is not a problem for PATH type disk. But it's definitely a problem for 
MOUNT type disk since it'll be offered to the framework, and the framework 
might create a new volume on top of that. We might want to do a cleanup when we 
create a volume from MOUNT type disk. Can you make sure the TODO captures that 
as well?



src/slave/slave.cpp (line 2373)


I would suggest we don't use CHECK here. We can just LOG(ERROR) if the 
deletion fails. Given the TODO above, we could leak disk space anyway.


- Jie Yu


On March 21, 2016, 6:44 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45121/
> ---
> 
> (Updated March 21, 2016, 6:44 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
> https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prior to this commit, destroying a persistent volume would remove
> the Mesos-level metadata about the volume, but wouldn't destroy
> any of the volume's filesystem content. We now remove the volume
> from the slave's filesystem, essentially via "rm -r".
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
> 
> Diff: https://reviews.apache.org/r/45121/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Vinod Kone]

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



LGTM modulo Anand's comments. Please also update the CHANGELOG.

- Vinod Kone


On March 25, 2016, 6:58 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45304/
> ---
> 
> (Updated March 25, 2016, 6:58 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5015
> https://issues.apache.org/jira/browse/MESOS-5015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes Call and Event Type enums in executor.proto
> optional for the purpose of backwards compatibility. (MESOS-5015)
> 
> 
> Diffs
> -
> 
>   include/mesos/executor/executor.proto 
> 6b4141f0aac27be5a4998f16e986fa87a7047834 
>   include/mesos/v1/executor/executor.proto 
> d7b1dd5eec9bcd424c7859d89471148bce8fe16e 
>   src/examples/test_http_executor.cpp 
> 562b0acfd8555b9b773175f53defe0e7e2744641 
>   src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 
> 
> Diff: https://reviews.apache.org/r/45304/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45015: Windows: Fixed bug causing `os::exists` to report invalid paths exist.

[Yi Sun]

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


Fix it, then Ship it!




Ship It!


3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp (line 149)


Nit: expect.


- Yi Sun


On March 24, 2016, 7:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45015/
> ---
> 
> (Updated March 24, 2016, 7:58 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently on Windows, `os::exists` will return true if a component of a
> path does not exist. For example if you have `a/fancy/path`, and you ask
> `os::exists("a/fake/path")`, the result currently reports `true`. In
> other words, the Windows code path only checks for the error that a file
> does not exist, and ignores the error that says the path is not valid.
> 
> This commit will fix this, and also add a test that will verify we don't
> regress.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp 
> 9211851e4562e04045276421b359c3c78cdae7f1 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> 4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 
> 
> Diff: https://reviews.apache.org/r/45015/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43935: Allow setting role in mesos-execute.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43935]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 25, 2016, 1:54 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43935/
> ---
> 
> (Updated March 25, 2016, 1:54 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Shuai Lin, and Michael Park.
> 
> 
> Bugs: MESOS-4744
> https://issues.apache.org/jira/browse/MESOS-4744
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow setting role in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/43935/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> start master.
> ./bin/mesos-master.sh --work_dir=/tmp/mesos
> 
> start slave.
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos --master=192.168.99.1:5050 
> --resources="cpus:1;cpus(test):1;mem:7500;mem(test):7500"
> 
> running mesos-execute without specifying role succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --resources="cpus:1;mem:512"
> 
> running mesos-execute with role test succeeds.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test" --resources="cpus:2;mem:512"
> 
> running mesos-execute with role test1 fails.
> ./src/mesos-execute --master=192.168.99.1:5050 --name="test" --command="sleep 
> 10" --role="test1" --resources="cpus:2;mem:512"
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 45009: Transition `filesystem_tests.cpp` to use `path::join`.

[Yi Sun]

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


Ship it!




Ship It!

- Yi Sun


On March 18, 2016, 12:51 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45009/
> ---
> 
> (Updated March 18, 2016, 12:51 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Transition `filesystem_tests.cpp` to use `path::join`.
> 
> This commit will fix a test that fails due to inconsistent usage of path
> delimiting characters.
> 
> Historically in Mesos, it has been common to join paths by adding
> strings. For example: `somePath + "/yourFile"`. The '/' character in
> particular is contentious, and can cause problems if you're not
> consistent in your usage. For example, if `somePath` already uses '\' as
> a path separator, then joining the string `"/yourFile"` introduces
> inconsistency in the path delimiter usage, resulting in undesirable
> behavior.
> 
> Review: https://reviews.apache.org/r/45009
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> 4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 
> 
> Diff: https://reviews.apache.org/r/45009/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44084: Windows: Forked os::rename().

[Yi Sun]

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


Ship it!




Ship It!

- Yi Sun


On March 24, 2016, 2:59 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44084/
> ---
> 
> (Updated March 24, 2016, 2:59 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Forked os::rename().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 7bd4bfbc2ec5922879dcefddc12137336b11be52 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rename.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rename.hpp 
> 7b2215208bcaa5613426d8ede9cab21d1a084631 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rename.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> 4c30189bb8261ccfc699da0f31b8b1fd3e9b3c83 
> 
> Diff: https://reviews.apache.org/r/44084/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 45299: Stout:[1/2] transitioned codebase to use `os::pagesize`.

[Yi Sun]

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


Fix it, then Ship it!




Ship It!


3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp (line 87)


nit: getpagesize() is irrelevant now.


- Yi Sun


On March 25, 2016, 6:12 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45299/
> ---
> 
> (Updated March 25, 2016, 6:12 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout:[1/2] transitioned codebase to use `os::pagesize`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 7bd4bfbc2ec5922879dcefddc12137336b11be52 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
> 6b81bf1ea3d571e8f5fb170076b85223b02efe2c 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> 182ac9cfbacd4535fb355aac6ec38559d786e63a 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
> 692fa483ee8a75dedae5ad0e7d2a4aa70dafa8a3 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/pagesize.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/pagesize.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pagesize.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> 9ee233b988c08d953e70345c55bcdd5c2f7c101b 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 6a391ff198ab724f689bcef79d4e2e05a786cbc2 
> 
> Diff: https://reviews.apache.org/r/45299/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 45300: Containerizer:[2/2] Transitioned codebase to use `os::pagesize`.

[Yi Sun]

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


Ship it!




Ship It!

- Yi Sun


On March 25, 2016, 6:12 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45300/
> ---
> 
> (Updated March 25, 2016, 6:12 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Containerizer:[2/2] Transitioned codebase to use `os::pagesize`.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_executor.cpp 399218478a596387814d123290eeeb33102fad7a 
>   src/tests/containerizer/cgroups_tests.cpp 
> acaed9b3f8a04964092cef413133834d0cf5a145 
>   src/tests/containerizer/memory_test_helper.cpp 
> 92579a28336b3c15ba7fdbb4a9f769ac7182aeb6 
> 
> Diff: https://reviews.apache.org/r/45300/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

[Yong Tang]

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

(Updated March 25, 2016, 6:58 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Updated the review request based on feedback from MESOS-5014 (related). Add 
Anand to reviewers list.


Bugs: MESOS-5015
https://issues.apache.org/jira/browse/MESOS-5015


Repository: mesos


Description
---

This fix changes Call and Event Type enums in executor.proto
optional for the purpose of backwards compatibility. (MESOS-5015)


Diffs (updated)
-

  include/mesos/executor/executor.proto 
6b4141f0aac27be5a4998f16e986fa87a7047834 
  include/mesos/v1/executor/executor.proto 
d7b1dd5eec9bcd424c7859d89471148bce8fe16e 
  src/examples/test_http_executor.cpp 562b0acfd8555b9b773175f53defe0e7e2744641 
  src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 

Diff: https://reviews.apache.org/r/45304/diff/


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang

Re: Review Request 45249: Added a new '/files' endpoints test using authentication.

[Greg Mann]

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

(Updated March 25, 2016, 4:42 p.m.)


Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.


Bugs: MESOS-4956
https://issues.apache.org/jira/browse/MESOS-4956


Repository: mesos


Description (updated)
---

A new test was added (FilesTest.AuthenticationTest) to probe the `/files/*` 
endpoints' behavior when HTTP authentication is enabled.


Diffs (updated)
-

  src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e 

Diff: https://reviews.apache.org/r/45249/diff/


Testing (updated)
---

`sudo make check` and `GTEST_FILTER="FilesTest.AuthenticationTest" 
build/bin/mesos-tests.sh --gtest_repeat=1000 --gtest_break_on_failure=1` were 
used to test on OSX and CentOS 7.


Thanks,

Greg Mann

Re: Review Request 45290: Added a test for the '/files/debug' endpoint.

[Greg Mann]

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

(Updated March 25, 2016, 4:42 p.m.)


Review request for mesos, Adam B, Ben Mahler, and Joerg Schad.


Changes
---

Rebase.


Repository: mesos


Description
---

This endpoint previously had no test coverage, so a test was added that hits 
the endpoint and checks for the expected output.


Diffs (updated)
-

  src/tests/files_tests.cpp b3894954e32b14f879f24fac17869fc32ad2ce0e 

Diff: https://reviews.apache.org/r/45290/diff/


Testing
---

`GTEST_FILTER="FilesTest.AuthenticationTest" bin/mesos-tests.sh 
--gtest_repeat=1000 --gtest_break_on_failure=1` was used to test on OSX.


Thanks,

Greg Mann

Re: Review Request 45317: Change Call and Event Type enums in scheduler.proto optional.

[Yong Tang]

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

(Updated March 25, 2016, 5:56 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Add Anand to the reviewers list.


Bugs: MESOS-5014
https://issues.apache.org/jira/browse/MESOS-5014


Repository: mesos


Description
---

This fix changes Call and Event Type enums in scheduler.proto
optional for the purpose of backwards compatibility. (MESOS-5014)


Diffs (updated)
-

  include/mesos/scheduler/scheduler.proto 
0049e1383f50574c3dad6a29b91811001694e82c 
  include/mesos/v1/scheduler/scheduler.proto 
09fafedd06837f2307fedc6fa0e7b4460b21e5b0 
  src/master/validation.cpp 701a5c4b279f319dde15bd8f2e97b5fd8608e578 
  src/tests/mesos.hpp aaef158e5784ce077ef60996ebbeb77b356b7c57 

Diff: https://reviews.apache.org/r/45317/diff/


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang