Re: Review Request 44314: Added authentication to http master endpoint tests.

2016-03-02 Thread Joerg Schad

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

(Updated March 3, 2016, 7:31 a.m.)


Review request for mesos, Adam B and Alexander Rojas.


Repository: mesos


Description
---

Added authentication to http master endpoint tests.


Diffs
-

  src/tests/fault_tolerance_tests.cpp 982468f851cd9d95eb6cde7c57f2d737d46a827c 
  src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
  src/tests/master_maintenance_tests.cpp 
3faa8136cf57276295553910319480028f433e4c 
  src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
  src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
  src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
  src/tests/status_update_manager_tests.cpp 
d64d3b8c96270478f6b681c038de77c3a9eb68fe 

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


Testing
---

make check


Thanks,

Joerg Schad



Re: Review Request 44200: Add agent flags for specifying CNI plugin and config directories.

2016-03-02 Thread Qian Zhang


> On March 3, 2016, 12:07 a.m., Avinash sridharan wrote:
> > src/slave/flags.cpp, line 690
> > 
> >
> > Maybe s/Directory path/Location ?

I just followed the existing convention, please see the help message of 
`Flags::launcher_dir` and `Flags::work_dir`, they all use the words "Directory 
path".


> On March 3, 2016, 12:07 a.m., Avinash sridharan wrote:
> > src/slave/flags.cpp, line 694
> > 
> >
> > s/directory/location
> > 
> > remove this line:
> > This flag is used for\n"
> >   "the `network/cni` isolator.\n",
> >   "/tmp/mesos/cni/plugins"

Did you mean removing the line: `This flag is used for the 'network/cni' 
isolator.`? I think this is the existing convention, please see the help 
message of `Flags::eth0_name`, `Flags::network_enable_snmp_statistics`, 
`Flags::container_disk_watch_interval`, etc. They all have the line like: `This 
flag is used for the 'xxx' isolator.`.


> On March 3, 2016, 12:07 a.m., Avinash sridharan wrote:
> > docs/configuration.md, line 1305
> > 
> >
> > Update the documentation based on the comments in `flags.cpp`

Thanks for your comments in `flags.cpp` :-) I have replied your comments there, 
please let me know if you have further comments.


> On March 3, 2016, 12:07 a.m., Avinash sridharan wrote:
> > docs/endpoints/slave/state.json.md, line 91
> > 
> >
> > Do we need to change some install scripts to create these paths? I 
> > think we should leave the paths as empty. Giving a default path implies 
> > that the path should exist during mesos installation. Leaving the path 
> > empty would force the operator to specify it while launching `network/cni` 
> > isolator.

I think operator will be forced to specify it if the default path 
`/tmp/mesos/cni/plugins` does not exist because there is a check in 
`NetworkCniIsolatorProcess::create()`, please see 
https://reviews.apache.org/r/44269/ for details, if the default path does not 
exist, the agent will eventually exit with a meaningful error message to 
operator.


- Qian


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


On March 1, 2016, 3:04 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44200/
> ---
> 
> (Updated March 1, 2016, 3:04 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4761
> https://issues.apache.org/jira/browse/MESOS-4761
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add agent flags for specifying CNI plugin and config directories.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md c4d094aac4acb95fd6648071413a3d5160dbf381 
>   docs/endpoints/slave/state.json.md a6403eea3426bdec81d170d0829ffe7174862e55 
>   docs/endpoints/slave/state.md bdd555e0231935c668dea1ba63582348db2b3629 
>   src/slave/flags.hpp c07932157ffe36ea36f1def4c7d92b79d2219c48 
>   src/slave/flags.cpp 7f139e99e8392fe95bb30070896ed9d95dfe5701 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
> 
> Diff: https://reviews.apache.org/r/44200/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 44186: Added authentication to master endpoints.

2016-03-02 Thread Alexander Rojas

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


Ship it!




Ship It!

- Alexander Rojas


On March 3, 2016, 8:22 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44186/
> ---
> 
> (Updated March 3, 2016, 8:22 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4844
> https://issues.apache.org/jira/browse/MESOS-4844
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Authentication to master http endpoints (except version, health, 
> redirect, scheduler).
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
>   src/tests/fault_tolerance_tests.cpp 
> 982468f851cd9d95eb6cde7c57f2d737d46a827c 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/master_maintenance_tests.cpp 
> 3faa8136cf57276295553910319480028f433e4c 
>   src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
>   src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
>   src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
>   src/tests/status_update_manager_tests.cpp 
> d64d3b8c96270478f6b681c038de77c3a9eb68fe 
> 
> Diff: https://reviews.apache.org/r/44186/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 44186: Added authentication to master endpoints.

2016-03-02 Thread Joerg Schad

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

(Updated March 3, 2016, 7:22 a.m.)


Review request for mesos, Adam B and Alexander Rojas.


Changes
---

Addressed Review.


Summary (updated)
-

Added authentication to master endpoints.


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


Repository: mesos


Description
---

Added Authentication to master http endpoints (except version, health, 
redirect, scheduler).


Diffs (updated)
-

  src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
  src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
  src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
  src/tests/fault_tolerance_tests.cpp 982468f851cd9d95eb6cde7c57f2d737d46a827c 
  src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
  src/tests/master_maintenance_tests.cpp 
3faa8136cf57276295553910319480028f433e4c 
  src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
  src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
  src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
  src/tests/status_update_manager_tests.cpp 
d64d3b8c96270478f6b681c038de77c3a9eb68fe 

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


Testing
---

make check


Thanks,

Joerg Schad



Re: Review Request 44315: Do not traverse offer list if there is only one offer.

2016-03-02 Thread Guangya Liu

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

(Updated 三月 3, 2016, 7:01 a.m.)


Review request for mesos, Anand Mazumdar, Joris Van Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

Do not traverse offer list if there is only one offer.


Diffs
-

  src/tests/master_maintenance_tests.cpp 
3faa8136cf57276295553910319480028f433e4c 

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


Testing
---

make
make check
./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*"


Thanks,

Guangya Liu



Review Request 44315: Do not traverse offer list if there is only one offer.

2016-03-02 Thread Guangya Liu

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

Review request for mesos, Anand Mazumdar, Joris Van Remoortere, and Joseph Wu.


Repository: mesos


Description
---

Do not traverse offer list if there is only one offer.


Diffs
-

  src/tests/master_maintenance_tests.cpp 
3faa8136cf57276295553910319480028f433e4c 

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


Testing
---

make
make check
./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*"


Thanks,

Guangya Liu



Review Request 44314: Added authentication to http master endpoint tests.

2016-03-02 Thread Joerg Schad

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

Review request for mesos, Adam B and Alexander Rojas.


Repository: mesos


Description
---

Added authentication to http master endpoint tests.


Diffs
-

  src/tests/fault_tolerance_tests.cpp 982468f851cd9d95eb6cde7c57f2d737d46a827c 
  src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
  src/tests/master_maintenance_tests.cpp 
3faa8136cf57276295553910319480028f433e4c 
  src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
  src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
  src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
  src/tests/status_update_manager_tests.cpp 
d64d3b8c96270478f6b681c038de77c3a9eb68fe 

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


Testing
---

make check


Thanks,

Joerg Schad



Re: Review Request 44186: Added authenticated to master endpoints.

2016-03-02 Thread Joerg Schad

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

(Updated March 3, 2016, 6:52 a.m.)


Review request for mesos, Adam B and Alexander Rojas.


Changes
---

Split Patch


Summary (updated)
-

Added authenticated to master endpoints.


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


Repository: mesos


Description
---

Added Authentication to master http endpoints (except version, health, 
redirect, scheduler).


Diffs (updated)
-

  src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
  src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
  src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 

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


Testing
---

make check


Thanks,

Joerg Schad



Re: Review Request 44266: Rename event_call_framework.cpp to test_http_framework.cpp.

2016-03-02 Thread Yong Tang

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

(Updated March 3, 2016, 6:49 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Change s/EventCall/TestHTTP/ in example_tests.cpp, change s/C++ low level 
scheduler/C++ HTTP scheduler/ in test_http_framework_test.sh.


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


Repository: mesos


Description
---

This change rename event_call_framework to test_http_framework
in order to correctly reflect that it's an example for HTTP based
framework. (MESOS-4583)


Diffs (updated)
-

  src/Makefile.am b30cc25f61856d6417437547baaa0bb338a30d63 
  src/examples/event_call_framework.cpp 
9bac8021394675d416967f507fbc2556e08d 
  src/tests/event_call_framework_test.sh 
cddb5208bce29cf84c40ff76aff0163d110a98d1 
  src/tests/examples_tests.cpp 9b8b30f3718aa0fa9d02a3aa344634f0c03e61e6 

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


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang



Re: Review Request 44200: Add agent flags for specifying CNI plugin and config directories.

2016-03-02 Thread Qian Zhang


> On March 2, 2016, 1:03 a.m., Gilbert Song wrote:
> > src/slave/flags.hpp, lines 132-133
> > 
> >
> > Thinking about a proper naming. Consider remove `_dir`?

I see there are already a couple of flags named with `_dir` as suffix, e.g., 
`appc_store_dir`, `launcher_dir`, etc., so I just followed them :-)


> On March 2, 2016, 1:03 a.m., Gilbert Song wrote:
> > src/slave/flags.cpp, lines 698-705
> > 
> >
> > While users can config the CNI network form a JSON file, is it possible 
> > to provide an option to define the CNI config from cli?

Yes, I think it is possible and there are already a couple of flags defined in 
this way, e.g., `--resources`, `--credential`. @Jie, do you think we should 
switch to this way?


- Qian


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


On March 1, 2016, 3:04 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44200/
> ---
> 
> (Updated March 1, 2016, 3:04 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4761
> https://issues.apache.org/jira/browse/MESOS-4761
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add agent flags for specifying CNI plugin and config directories.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md c4d094aac4acb95fd6648071413a3d5160dbf381 
>   docs/endpoints/slave/state.json.md a6403eea3426bdec81d170d0829ffe7174862e55 
>   docs/endpoints/slave/state.md bdd555e0231935c668dea1ba63582348db2b3629 
>   src/slave/flags.hpp c07932157ffe36ea36f1def4c7d92b79d2219c48 
>   src/slave/flags.cpp 7f139e99e8392fe95bb30070896ed9d95dfe5701 
>   src/slave/http.cpp 4eb1fafdfa72094511b0b2684a3c2705bd8c7c5e 
> 
> Diff: https://reviews.apache.org/r/44200/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 44186: Added authentication to master endpoints.

2016-03-02 Thread Joerg Schad

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

(Updated March 2, 2016, 10:22 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


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


Repository: mesos


Description
---

Added Authentication to master http endpoints (except version, health, 
redirect, scheduler).


Diffs
-

  src/master/http.cpp d6e1f22620dfc4271244a2983195cffc36da6e8e 
  src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
  src/master/master.cpp 7c62f2a882a1c89d73f328b2ae665422fd84d7a1 
  src/tests/fault_tolerance_tests.cpp 982468f851cd9d95eb6cde7c57f2d737d46a827c 
  src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
  src/tests/master_maintenance_tests.cpp 
356015ebd8d5d55b656e9116eafdb04f01f99039 
  src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
  src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
  src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
  src/tests/status_update_manager_tests.cpp 
d64d3b8c96270478f6b681c038de77c3a9eb68fe 

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


Testing
---

make check


Thanks,

Joerg Schad



Re: Review Request 44258: Fixed http endpoint trigger two inverse offer calls.

2016-03-02 Thread Klaus Ma


> On March 2, 2016, 10:43 p.m., Klaus Ma wrote:
> > src/master/http.cpp, lines 1999-2000
> > 
> >
> > I think we can just remove `master->updateUnavailability(id, 
> > updated[id]);` here, so other machine will `UP` and scheduler will be 
> > updated in next loop.
> > 
> > 
> > Futhur more, we can avoid the loop for update. The draft code will be:
> > 
> > ```
> > hashmap updated;
> > 
> > // delete the loop for "updated[id] = window.unavailability();"
> > 
> > foreach (const mesos::maintenance::Window& window, schedule.windows()) {
> >   foreach (const MachineID& id, window.machine_ids()) {
> > ...
> > master->updateUnavailability(id, window.unavailability());
> > updated[id] = window.unavailability();
> >   }
> > }
> > 
> > foreachkey (const MachineID& id, utils::copy(master->machines)) {
> >   if (updated.contains(id)) {
> > continue;
> >   }
> > 
> >   // Transition each removed machine back to the `UP` mode and remove 
> > the
> >   // unavailability.
> >   master->machines[id].info.set_mode(MachineInfo::UP);
> >   master->updateUnavailability(id, None());
> > }
> > ```
> 
> Joseph Wu wrote:
> (Wish I could comment inline on a comment...)
> 
> Your two loops won't update the `Unavailability` for machines that have 
> been brought DOWN previously.  That's why we had 
> `master->updateUnavailability(id, updated[id]);` in two places.

@Joseph, I think `schedule.windows` will also include DOWN machines. We did 
check here: 
https://github.com/apache/mesos/blob/master/src/master/maintenance.cpp#L241 ; 
if the machine is DOWN but not in updated list, master'll reject the request.

I have filed a JIRA (MESOS-4837) on whether this check is necessary.


- Klaus


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


On March 3, 2016, 2:05 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44258/
> ---
> 
> (Updated March 3, 2016, 2:05 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-4831
> https://issues.apache.org/jira/browse/MESOS-4831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There is a bug when setting host maintain with http endpoint: 
> https://github.com/apache/mesos/blob/master/src/master/http.cpp#L1987-L2021
> The logic is as this:
> 1) Get all host list from maintain window and put it to updated hashmap.
> 2) If the machine in was in updated was also in master->machines, call master 
> updateUnavailability to trigger recoverResources, updateUnavailability etc in 
> allocator
> 3) Otherwise, clear the unavailabity time window for the machine.
> 4) Update each new machines in updated to call master updateUnavailability
> 
> But the logic in step 4) is getting all machines from the schedule windows 
> but not the machines that is new to the cluster, this caused master get two 
> updateUnavailability calls for a machine in the updated hashmap.
> 
> The fix is filter machines in updated hashmap when handling new machines.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
>   src/tests/master_maintenance_tests.cpp 
> 3faa8136cf57276295553910319480028f433e4c 
> 
> Diff: https://reviews.apache.org/r/44258/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
>  ./bin/mesos-tests.sh --gtest_filter="MasterMaintenanceTest.*" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 44258: Fixed http endpoint trigger two inverse offer calls.

2016-03-02 Thread Guangya Liu

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

(Updated 三月 3, 2016, 6:05 a.m.)


Review request for mesos, Anand Mazumdar, Joris Van Remoortere, and Joseph Wu.


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


Repository: mesos


Description
---

There is a bug when setting host maintain with http endpoint: 
https://github.com/apache/mesos/blob/master/src/master/http.cpp#L1987-L2021
The logic is as this:
1) Get all host list from maintain window and put it to updated hashmap.
2) If the machine in was in updated was also in master->machines, call master 
updateUnavailability to trigger recoverResources, updateUnavailability etc in 
allocator
3) Otherwise, clear the unavailabity time window for the machine.
4) Update each new machines in updated to call master updateUnavailability

But the logic in step 4) is getting all machines from the schedule windows but 
not the machines that is new to the cluster, this caused master get two 
updateUnavailability calls for a machine in the updated hashmap.

The fix is filter machines in updated hashmap when handling new machines.


Diffs (updated)
-

  src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
  src/tests/master_maintenance_tests.cpp 
3faa8136cf57276295553910319480028f433e4c 

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


Testing
---

make
make check
 ./bin/mesos-tests.sh --gtest_filter="MasterMaintenanceTest.*" --verbose


Thanks,

Guangya Liu



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

2016-03-02 Thread Mesos ReviewBot

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On March 3, 2016, 5:08 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44257/
> ---
> 
> (Updated March 3, 2016, 5:08 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-4804
> https://issues.apache.org/jira/browse/MESOS-4804
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade protobuf to 2.6.1 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 932f2f66b04e5ca3d2ed04da1e7019d2ff7488e4 
>   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 c3aaa437af10533132698df3348114195d338965 
>   configure.ac b045d3c68a2d440bed4d1b3e6ab21a1bbe063517 
>   src/java/mesos.pom.in 7615d61eb6fedfa0ead785cd360946c56ccf80af 
>   src/python/interface/setup.py.in d73996734c3a3c70c3a6c0c697bb6733c241c091 
>   src/python/native/ext_modules.py.in 
> 4682e5eed0f7be23fb48ef628e1bebc7741431d7 
>   src/python/protocol/setup.py.in 4c50fbbf1ce11c4c42c848364523225ee7ea5a3b 
> 
> Diff: https://reviews.apache.org/r/44257/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>



Re: Review Request 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-02 Thread fan du


> On 三月 2, 2016, 7:40 p.m., Greg Mann wrote:
> > Thanks Fan! I'm having a look at this now. Regarding the problems with 
> > review board not wanting to update your previous RR, it's probably because 
> > it didn't find the review URL in the commit message. In order to determine 
> > which RR it should update, the script looks for a string like "Review: 
> > https://reviews.apache.org/r/44255/; in the commit message of the commit 
> > you're pushing to review board. If you make sure that string (with the 
> > correct URL) is in your commit message locally, review board should be able 
> > to find your previous review and update it.

yes, it seems I recreated the commit message when rebasing without adding 
"Review: https://reviews.apache.org/r/44255;.
squash the commits together should also works ok :) Thanks for your kindness.


- fan


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


On 三月 3, 2016, 5:40 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated 三月 3, 2016, 5:40 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4492
> https://issues.apache.org/jira/browse/MESOS-4492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
> 
> Diff: https://reviews.apache.org/r/44255/diff/
> 
> 
> Testing
> ---
> 
> ChangLog:
> v2:
>   - Documenting those metrics
>   - Add test code for MetricsTest as suggested by Guangya
>   - post-review.py does not update original 
> RR(https://reviews.apache.org/r/44058/), but only create a new one even if I 
> rebased.
> 
> 
> Tests:
> 1. make check GTEST_FILTER="MetricsTest.Master" on Centos-7 
> (3.10.0-123.el7.x86_640)
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from MetricsTest
> [ RUN  ] MetricsTest.Master
> [   OK ] MetricsTest.Master (211 ms)
> [--] 1 test from MetricsTest (211 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (244 ms total)
> [  PASSED  ] 1 test
> 
> 2. Verify its functionality with 'reserve' http endpoint as an test case
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 0.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> # curl -i -d slaveId=6250553a-2f39-4a92-9073-4618d130f433-S1  -d resources='[ 
> { "name": "cpus", "type": "SCALAR","scalar": { "value": 1 
> },"reservation":{"principal": "XiaoHaHa"}}  ]' -X POST  
> ipdc02-kvm-guest2:5050/master/reserve
> HTTP/1.1 200 OK
> Date: Fri, 26 Feb 2016 19:59:01 GMT
> Content-Length: 0
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot  | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 1.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> Thanks,
> 
> fan du
> 
>



Review Request 44313: Described "/maintanence/schedule" GET/POST in the two paragraph.

2016-03-02 Thread Klaus Ma

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

Review request for mesos, Joris Van Remoortere and Joseph Wu.


Repository: mesos


Description
---

Described /maintanence/schedule GET/POST in the two paragraph.


Diffs
-

  docs/endpoints/master/maintenance/schedule.md 
81c34f598ccdc74af74d2a96f02a252a239d9b21 
  src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 

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


Testing
---


Thanks,

Klaus Ma



Re: Review Request 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-03-02 Thread fan du

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

(Updated 三月 3, 2016, 5:40 a.m.)


Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.


Changes
---

Incorperated comments from Greg Mann
1. Fix few typos
2. Drop signed-off-by


Summary (updated)
-

Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.


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


Repository: mesos


Description (updated)
---

Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.


Diffs (updated)
-

  docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
  src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
  src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
  src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
  src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 

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


Testing
---

ChangLog:
v2:
  - Documenting those metrics
  - Add test code for MetricsTest as suggested by Guangya
  - post-review.py does not update original 
RR(https://reviews.apache.org/r/44058/), but only create a new one even if I 
rebased.


Tests:
1. make check GTEST_FILTER="MetricsTest.Master" on Centos-7 
(3.10.0-123.el7.x86_640)

[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from MetricsTest
[ RUN  ] MetricsTest.Master
[   OK ] MetricsTest.Master (211 ms)
[--] 1 test from MetricsTest (211 ms total)

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

2. Verify its functionality with 'reserve' http endpoint as an test case

# curl http://ipdc02-kvm-guest2:5050/metrics/snapshot | python -mjson.tool | 
grep reserve
"master/messages_reserve_resource": 0.0,
"master/messages_unreserve_resource": 0.0,


# curl -i -d slaveId=6250553a-2f39-4a92-9073-4618d130f433-S1  -d resources='[ { 
"name": "cpus", "type": "SCALAR","scalar": { "value": 1 
},"reservation":{"principal": "XiaoHaHa"}}  ]' -X POST  
ipdc02-kvm-guest2:5050/master/reserve
HTTP/1.1 200 OK
Date: Fri, 26 Feb 2016 19:59:01 GMT
Content-Length: 0

# curl http://ipdc02-kvm-guest2:5050/metrics/snapshot  | python -mjson.tool | 
grep reserve
"master/messages_reserve_resource": 1.0,
"master/messages_unreserve_resource": 0.0,


Thanks,

fan du



Re: Review Request 44071: Added documentation for new libprocess environment variable.

2016-03-02 Thread Ben Mahler

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


Ship it!





docs/configuration.md (lines 1694 - 1700)


The comments above are indented by 2 more than this block..?

How about:

```
  

  LIBPROCESS_METRICS_SNAPSHOT_ENDPOINT_RATE_LIMIT


  If set, this variable can be used to configure the rate limit
  applied to the /metrics/snapshot endpoint. The format is
  `/`.
  Examples: `10/1secs`, `100/10secs`, etc.

  
```


- Ben Mahler


On March 1, 2016, 9:04 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44071/
> ---
> 
> (Updated March 1, 2016, 9:04 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, and Ben 
> Mahler.
> 
> 
> Bugs: MESOS-4776
> https://issues.apache.org/jira/browse/MESOS-4776
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The environment variable LIBPROCESS_METRICS_ENDPOINT_RATE_LIMIT was
> added in a previous commit; add the corresponding documentation.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md c4d094aac4acb95fd6648071413a3d5160dbf381 
> 
> Diff: https://reviews.apache.org/r/44071/diff/
> 
> 
> Testing
> ---
> 
> Site rendered with packaged docker container.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 44073: Disabled metrics endpoint rate limiting in mesos tests.

2016-03-02 Thread Ben Mahler

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



This patch should also remove any remnants of advancing the clock to deal with 
metrics rate limiting, I'll go ahead and do that for you before committing.


src/tests/main.cpp (line 28)


We need to include the os agnostic version here, otherwise this doesn't 
work for windows.



src/tests/main.cpp (lines 75 - 77)


This means we need to update the tests to remove any clock advances to cope 
with the rate limiting.


- Ben Mahler


On March 1, 2016, 4:28 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44073/
> ---
> 
> (Updated March 1, 2016, 4:28 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, and Ben 
> Mahler.
> 
> 
> Bugs: MESOS-4783
> https://issues.apache.org/jira/browse/MESOS-4783
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Disabled metrics endpoint rate limiting in mesos tests.
> 
> 
> Diffs
> -
> 
>   src/tests/main.cpp 942488e57419ace8b7a821f53024aced0f43c7d9 
> 
> Diff: https://reviews.apache.org/r/44073/diff/
> 
> 
> Testing
> ---
> 
> make distcheck (ubuntu 12 & 14), make check (OS X)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 44070: Allowed disabling metrics endpoint rate limiting via the environment.

2016-03-02 Thread Ben Mahler

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



Thanks for the patience, I left some comments for some issues that remained. 
I'll get this committed shortly with adjustments based on the comments.


3rdparty/libprocess/include/process/metrics/metrics.hpp (lines 33 - 36)


This should be called from process::initialize, if we'd like this to be 
consistent with Clock::initialize, mime::initialize, EventLoop::initialize:

```
  // TODO(bmahler): Consider initializing this consistently with
  // the other global Processes.
  metrics::internal::MetricsProcess* metricsProcess =
metrics::internal::MetricsProcess::instance();

  CHECK_NOTNULL(metricsProcess);
```

Becomes:

```
  // Ensure metrics process is running.
  metrics::initialize();
```

Two newlines below but one above?



3rdparty/libprocess/include/process/metrics/metrics.hpp (line 79)


Hm.. it's hard to tell what "setting up global infrastructure" mean? How 
about:

```
// Needed to access the private constructor.
```



3rdparty/libprocess/src/metrics/metrics.cpp (lines 19 - 30)


Includes for Owned, numify, Duration?



3rdparty/libprocess/src/metrics/metrics.cpp (line 30)


Pulling in the posix header means it won't compile on windows, so this 
should be the agnostic wrapper: 

```
#include 
```



3rdparty/libprocess/src/metrics/metrics.cpp (line 33)


stale?



3rdparty/libprocess/src/metrics/metrics.cpp (line 41)


Looking around at the globals in the code base, I can't seem to find an 
instance of us naming the global "singleton". How about "metrics_process" to be 
consistent with the naming in proces.cpp for now?



3rdparty/libprocess/src/metrics/metrics.cpp (line 196)


Since it's at the top of the header, can you move this to the top of the 
.cpp file?

Also, whoops, brace should be on the next line here.



3rdparty/libprocess/src/metrics/metrics.cpp (lines 197 - 260)


Hm.. any reason you introduced two ways to check that we only initialize 
once? The first check seems unreliable as Alexander mentioned, so is it a 
performance optimization?

Also, it seems like the rate limiting parsing and creation should only 
occur when the once determines this is the only execution, right? Was this 
intentionally moved outside the once for some reason? Because as it stands we 
may create some unnecessary rate limiters and throw them away, which seems odd.



3rdparty/libprocess/src/metrics/metrics.cpp (line 201)


How about s/rateLimiter/limiter/ here and elsewhere to be consistent with 
how it is named in the MetricsProcess?



3rdparty/libprocess/src/metrics/metrics.cpp (line 205)


We planned to add more endpoints in the future, so perhaps 
LIBPROCESS_METRICS_SNAPSHOT_ENDPOINT_RATE_LIMIT would be more appropriate here. 
For example, if we exposed an endoint for getting historical data, that may not 
need the same rate limiting since it doesn't hit components through gauges.



3rdparty/libprocess/src/metrics/metrics.cpp (lines 207 - 209)


A comment that we default to 2 per second would be great here, noting that 
we default to this for backwards compatibility. The default was originally in 
place to apply a reasonable limit, and now we have to keep it for backwards 
compatibility.



3rdparty/libprocess/src/metrics/metrics.cpp (lines 209 - 210)


else if?



3rdparty/libprocess/src/metrics/metrics.cpp (line 210)


You can use -> instead of .get()., can you do a sweep within your code?

```
value->empty()
// vs
value.get().empty()
```



3rdparty/libprocess/src/metrics/metrics.cpp (lines 215 - 238)


It would be nice to do error messaging once rather than 3 places.

I realize this was copied, but we tend to EXIT upon invalid user input. 
Since LOG(FATAL) prints a stack trace, users tend to think mesos has crashed 
because there is a bug in mesos.



3rdparty/libprocess/src/metrics/metrics.cpp (line 236)


 

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

2016-03-02 Thread Zhiwei Chen

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

Review request for mesos.


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


Repository: mesos


Description
---

Upgrade protobuf to 2.6.1 to support PowerPC LE platform.


Diffs
-

  3rdparty/cmake/Versions.cmake 932f2f66b04e5ca3d2ed04da1e7019d2ff7488e4 
  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 c3aaa437af10533132698df3348114195d338965 
  configure.ac b045d3c68a2d440bed4d1b3e6ab21a1bbe063517 
  src/java/mesos.pom.in 7615d61eb6fedfa0ead785cd360946c56ccf80af 
  src/python/interface/setup.py.in d73996734c3a3c70c3a6c0c697bb6733c241c091 
  src/python/native/ext_modules.py.in 4682e5eed0f7be23fb48ef628e1bebc7741431d7 
  src/python/protocol/setup.py.in 4c50fbbf1ce11c4c42c848364523225ee7ea5a3b 

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


Testing
---


Thanks,

Zhiwei Chen



Re: Review Request 44266: Rename event_call_framework.cpp to test_http_framework.cpp.

2016-03-02 Thread Vinod Kone

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




src/tests/event_call_framework_test.sh (line 42)


s/C++ low level scheduler/C++ HTTP scheduler/


- Vinod Kone


On March 3, 2016, 4:27 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44266/
> ---
> 
> (Updated March 3, 2016, 4:27 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-4583
> https://issues.apache.org/jira/browse/MESOS-4583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change rename event_call_framework to test_http_framework
> in order to correctly reflect that it's an example for HTTP based
> framework. (MESOS-4583)
> 
> 
> Diffs
> -
> 
>   src/Makefile.am fe549a881e778e094d3f957e5119eeb2f55e7915 
>   src/examples/event_call_framework.cpp 
> d07d05d4b1eae5d2286d3fc1fdc32247bc19cada 
>   src/tests/event_call_framework_test.sh 
> cddb5208bce29cf84c40ff76aff0163d110a98d1 
>   src/tests/examples_tests.cpp 9b8b30f3718aa0fa9d02a3aa344634f0c03e61e6 
> 
> Diff: https://reviews.apache.org/r/44266/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>



Re: Review Request 44266: Rename event_call_framework.cpp to test_http_framework.cpp.

2016-03-02 Thread Vinod Kone

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


Fix it, then Ship it!





src/tests/examples_tests.cpp (line 27)


s/EventCall/TestHTTP/


- Vinod Kone


On March 3, 2016, 4:27 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44266/
> ---
> 
> (Updated March 3, 2016, 4:27 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-4583
> https://issues.apache.org/jira/browse/MESOS-4583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change rename event_call_framework to test_http_framework
> in order to correctly reflect that it's an example for HTTP based
> framework. (MESOS-4583)
> 
> 
> Diffs
> -
> 
>   src/Makefile.am fe549a881e778e094d3f957e5119eeb2f55e7915 
>   src/examples/event_call_framework.cpp 
> d07d05d4b1eae5d2286d3fc1fdc32247bc19cada 
>   src/tests/event_call_framework_test.sh 
> cddb5208bce29cf84c40ff76aff0163d110a98d1 
>   src/tests/examples_tests.cpp 9b8b30f3718aa0fa9d02a3aa344634f0c03e61e6 
> 
> Diff: https://reviews.apache.org/r/44266/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>



Re: Review Request 44266: Rename event_call_framework.cpp to test_http_framework.cpp.

2016-03-02 Thread Yong Tang


> On March 3, 2016, 1 a.m., Vinod Kone wrote:
> > Looks great.
> > 
> > Can you also rename the class name in test_http_framework.cpp? 
> > s/EventCallScheduler/HTTPScheduler/
> 
> Yong Tang wrote:
> Thanks Vinod. Will update the review request shortly.

Just updated the review request. Let me know if there are any other issues.


- Yong


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


On March 3, 2016, 4:27 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44266/
> ---
> 
> (Updated March 3, 2016, 4:27 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-4583
> https://issues.apache.org/jira/browse/MESOS-4583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change rename event_call_framework to test_http_framework
> in order to correctly reflect that it's an example for HTTP based
> framework. (MESOS-4583)
> 
> 
> Diffs
> -
> 
>   src/Makefile.am fe549a881e778e094d3f957e5119eeb2f55e7915 
>   src/examples/event_call_framework.cpp 
> d07d05d4b1eae5d2286d3fc1fdc32247bc19cada 
>   src/tests/event_call_framework_test.sh 
> cddb5208bce29cf84c40ff76aff0163d110a98d1 
>   src/tests/examples_tests.cpp 9b8b30f3718aa0fa9d02a3aa344634f0c03e61e6 
> 
> Diff: https://reviews.apache.org/r/44266/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>



Re: Review Request 44266: Rename event_call_framework.cpp to test_http_framework.cpp.

2016-03-02 Thread Yong Tang

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

(Updated March 3, 2016, 4:27 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Change EventCallScheduler to HTTPScheduler in test_http_framework.cpp


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


Repository: mesos


Description
---

This change rename event_call_framework to test_http_framework
in order to correctly reflect that it's an example for HTTP based
framework. (MESOS-4583)


Diffs (updated)
-

  src/Makefile.am fe549a881e778e094d3f957e5119eeb2f55e7915 
  src/examples/event_call_framework.cpp 
d07d05d4b1eae5d2286d3fc1fdc32247bc19cada 
  src/tests/event_call_framework_test.sh 
cddb5208bce29cf84c40ff76aff0163d110a98d1 
  src/tests/examples_tests.cpp 9b8b30f3718aa0fa9d02a3aa344634f0c03e61e6 

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


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang



Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

2016-03-02 Thread Mesos ReviewBot

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



Bad patch!

Reviews applied: [43615, 43630, 43629, 43614, 43613]

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

Error:
2016-03-03 04:23:19 URL:https://reviews.apache.org/r/43615/diff/raw/ 
[678716/678716] -> "43615.patch" [1]
error: patch failed: src/tests/scheduler_http_api_tests.cpp:601
error: src/tests/scheduler_http_api_tests.cpp: patch does not apply

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

- Mesos ReviewBot


On March 3, 2016, 12:05 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43615/
> ---
> 
> (Updated March 3, 2016, 12:05 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Includes the following changes:
> 
> * Added the `` header where appropriate.
> * Added the namespace `using process::Owned;` where appropriate.
> * Generally replaced `Try` with `Owned`.  And 
> `Try` with `Owned`.
> * Added the (now required) `MasterDetector` argument to all slaves.  Before, 
> this was fetched from the first master in `Cluster`.
> * Removed `Shutdown();` from all tests.
> * Replaced `Stop(...)` with the appropriate master/slave destruction calls.
> * Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
> launchers, etc).
> * Replace `CHECK` in tests with `ASSERT`.
> 
> 
> Diffs
> -
> 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/command_executor_tests.cpp 
> 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
>   src/tests/container_logger_tests.cpp 
> 00f4129e46aa9268fbb66da25b34e61004fa87b2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6aecd912fc84b72d2b64f7a842891fddcbc469ac 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e72239a55724f1aeeec5362cc370c93dbeca7164 
>   src/tests/containerizer/isolator_tests.cpp 
> 342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 79f134996c4b80bf49cbb8bee28eab5e6b4f5822 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 983a6be160aefe5a32acb6111bb3c85230ec 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 5b685bfd842d0d98e8ea5ec5ddea8d2cd893dd81 
>   src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
>   src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
>   src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
>   src/tests/executor_http_api_tests.cpp 
> 2fc0893f5f5e80a783296fb31b30abe86d92df1b 
>   src/tests/fault_tolerance_tests.cpp 
> f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
>   src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/hook_tests.cpp 88ad7806bf9f3194e434743c35199a896edea92c 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_quota_tests.cpp 8357ec911b2a158632a708ae3adff6eabc536697 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp 36595772b34bcb8d37dbc74d247bdf4614f10150 
>   src/tests/master_validation_tests.cpp 
> c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
>   src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 
>   src/tests/oversubscription_tests.cpp 
> e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
>   src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 81185a161498394020a27f1f5bf747bac5425f43 
>   src/tests/persistent_volume_tests.cpp 
> bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
>   src/tests/rate_limiting_tests.cpp caced732ded05a334861a53488ef6391885b2263 
>   src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
>   src/tests/registrar_zookeeper_tests.cpp 
> 3df9779ee5d076e16f6a538326693a36f986b6d0 
>   src/tests/repair_tests.cpp 

Re: Review Request 44266: Rename event_call_framework.cpp to test_http_framework.cpp.

2016-03-02 Thread Yong Tang


> On March 3, 2016, 1 a.m., Vinod Kone wrote:
> > Looks great.
> > 
> > Can you also rename the class name in test_http_framework.cpp? 
> > s/EventCallScheduler/HTTPScheduler/

Thanks Vinod. Will update the review request shortly.


- Yong


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


On March 2, 2016, 3:11 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44266/
> ---
> 
> (Updated March 2, 2016, 3:11 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-4583
> https://issues.apache.org/jira/browse/MESOS-4583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change rename event_call_framework to test_http_framework
> in order to correctly reflect that it's an example for HTTP based
> framework. (MESOS-4583)
> 
> 
> Diffs
> -
> 
>   src/Makefile.am fe549a881e778e094d3f957e5119eeb2f55e7915 
>   src/examples/event_call_framework.cpp  
>   src/tests/event_call_framework_test.sh 
> cddb5208bce29cf84c40ff76aff0163d110a98d1 
>   src/tests/examples_tests.cpp 9b8b30f3718aa0fa9d02a3aa344634f0c03e61e6 
> 
> Diff: https://reviews.apache.org/r/44266/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>



Re: Review Request 44258: Fixed http endpoint trigger two inverse offer calls.

2016-03-02 Thread Joseph Wu

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




src/master/http.cpp (lines 1999 - 2000)


We originally did not set the mode here because this loop updates the 
"unavailability" regardless of the "mode".

If you'd like, you can add a note here.  i.e.
```
// NOTE: We do not update the `MachineInfo::MODE` here because the machine 
may be in any mode.
```

---

If we really want to prevent the double-inverse-offer, you'd need to add 
something like this at inside the existing `if`:
```
if (master->machines[id].info.mode() == MachineInfo::UP) {
  continue;
}
```

Although at this point, we might as well consider folding the body of this 
function into `Master::updateUnavailability`.



src/master/http.cpp (lines 2014 - 2016)


Nice catch.  This loop is in fact not representative of the comment above.

However, to perform the update correctly (including the issue above), you'd 
need to account for any existing machines that are DRAINING or DOWN.
```
if (master->machines.contains(id) &&
master->machines[id].info.mode() != MachineInfo::UP) {
  continue;
}
```

---

Nit: Newline after the `if`.



src/tests/master_maintenance_tests.cpp (lines 427 - 429)


Can you also change this to:
```
EXPECT_FALSE(event.get().offers().offers(0).has_unavailability());
```



src/tests/master_maintenance_tests.cpp (lines 469 - 478)


Can you also change this:

```
EXPECT_TRUE(event.get().offers().offers(0).has_unavailability());
EXPECT_EQ(
unavailability.start().nanoseconds(),
event.get().offers().offers(0).unavailability().start().nanoseconds());

EXPECT_EQ(
unavailability.duration().nanoseconds(),

event.get().offers().offers(0).unavailability().duration().nanoseconds());
```


- Joseph Wu


On March 1, 2016, 11:45 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44258/
> ---
> 
> (Updated March 1, 2016, 11:45 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-4831
> https://issues.apache.org/jira/browse/MESOS-4831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There is a bug when setting host maintain with http endpoint: 
> https://github.com/apache/mesos/blob/master/src/master/http.cpp#L1987-L2021
> The logic is as this:
> 1) Get all host list from maintain window and put it to updated hashmap.
> 2) If the machine in was in updated was also in master->machines, call master 
> updateUnavailability to trigger recoverResources, updateUnavailability etc in 
> allocator
> 3) Otherwise, clear the unavailabity time window for the machine.
> 4) Update each new machines in updated to call master updateUnavailability
> 
> But the logic in step 4) is getting all machines from the schedule windows 
> but not the machines that is new to the cluster, this caused master get two 
> updateUnavailability calls for a machine in the updated hashmap.
> 
> The fix is filter machines in updated hashmap when handling new machines.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
>   src/tests/master_maintenance_tests.cpp 
> 3faa8136cf57276295553910319480028f433e4c 
> 
> Diff: https://reviews.apache.org/r/44258/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
>  ./bin/mesos-tests.sh --gtest_filter="MasterMaintenanceTest.*" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 44258: Fixed http endpoint trigger two inverse offer calls.

2016-03-02 Thread Joseph Wu


> On March 2, 2016, 6:43 a.m., Klaus Ma wrote:
> > src/master/http.cpp, lines 1999-2000
> > 
> >
> > I think we can just remove `master->updateUnavailability(id, 
> > updated[id]);` here, so other machine will `UP` and scheduler will be 
> > updated in next loop.
> > 
> > 
> > Futhur more, we can avoid the loop for update. The draft code will be:
> > 
> > ```
> > hashmap updated;
> > 
> > // delete the loop for "updated[id] = window.unavailability();"
> > 
> > foreach (const mesos::maintenance::Window& window, schedule.windows()) {
> >   foreach (const MachineID& id, window.machine_ids()) {
> > ...
> > master->updateUnavailability(id, window.unavailability());
> > updated[id] = window.unavailability();
> >   }
> > }
> > 
> > foreachkey (const MachineID& id, utils::copy(master->machines)) {
> >   if (updated.contains(id)) {
> > continue;
> >   }
> > 
> >   // Transition each removed machine back to the `UP` mode and remove 
> > the
> >   // unavailability.
> >   master->machines[id].info.set_mode(MachineInfo::UP);
> >   master->updateUnavailability(id, None());
> > }
> > ```

(Wish I could comment inline on a comment...)

Your two loops won't update the `Unavailability` for machines that have been 
brought DOWN previously.  That's why we had `master->updateUnavailability(id, 
updated[id]);` in two places.


- Joseph


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


On March 1, 2016, 11:45 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44258/
> ---
> 
> (Updated March 1, 2016, 11:45 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-4831
> https://issues.apache.org/jira/browse/MESOS-4831
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There is a bug when setting host maintain with http endpoint: 
> https://github.com/apache/mesos/blob/master/src/master/http.cpp#L1987-L2021
> The logic is as this:
> 1) Get all host list from maintain window and put it to updated hashmap.
> 2) If the machine in was in updated was also in master->machines, call master 
> updateUnavailability to trigger recoverResources, updateUnavailability etc in 
> allocator
> 3) Otherwise, clear the unavailabity time window for the machine.
> 4) Update each new machines in updated to call master updateUnavailability
> 
> But the logic in step 4) is getting all machines from the schedule windows 
> but not the machines that is new to the cluster, this caused master get two 
> updateUnavailability calls for a machine in the updated hashmap.
> 
> The fix is filter machines in updated hashmap when handling new machines.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
>   src/tests/master_maintenance_tests.cpp 
> 3faa8136cf57276295553910319480028f433e4c 
> 
> Diff: https://reviews.apache.org/r/44258/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
>  ./bin/mesos-tests.sh --gtest_filter="MasterMaintenanceTest.*" --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 41632: Windows: Forked signal handling in `signalhandler.hpp`.

2016-03-02 Thread Daniel Pravat

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

(Updated March 3, 2016, 3 a.m.)


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


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


Repository: mesos


Description
---

Windows: Forked signal handling in `signalhandler.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
03eff5a831283f6d298e9a1feecfdc7369cacfe7 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/signalhandler.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/signalhandler.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/signalhandler.hpp 
PRE-CREATION 

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


Testing
---

OSX: make check
Windows: make


Thanks,

Daniel Pravat



Re: Review Request 44239: Introduced copy fetcher plugin for file URIs.

2016-03-02 Thread Jojy Varghese

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

(Updated March 3, 2016, 2:02 a.m.)


Review request for mesos and Jie Yu.


Changes
---

fixed patch.


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


Repository: mesos


Description
---

This change adds support for fetching `file` URIs using the fetcher plugin
framework.


Diffs (updated)
-

  src/CMakeLists.txt 0832f1e9a850cc0d2c1112a446f0daf0190f8d7f 
  src/Makefile.am 5d8fe8bf33acb352589b2b3e0341987f9a41bc17 
  src/tests/uri_fetcher_tests.cpp dcfba784d6ae49b8181b5a69dc00e82854ac4ff1 
  src/uri/fetcher.hpp 5bc9703b76457413b5841710e0313f8bfa9047a1 
  src/uri/fetcher.cpp 2d8d5b18b7a9b55be52288f17e85d6c406cfef88 
  src/uri/fetchers/copy.hpp PRE-CREATION 
  src/uri/fetchers/copy.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 43969: Added test for Appc image fetcher.

2016-03-02 Thread Jie Yu

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


Ship it!




Ship It!

- Jie Yu


On March 2, 2016, 9:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43969/
> ---
> 
> (Updated March 2, 2016, 9:47 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4748
> https://issues.apache.org/jira/browse/MESOS-4748
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added simple appc Fetcher test with mock HTTP image server.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 9d9779ac04e32348ce0e28da54fd7aa039a7fbcd 
> 
> Diff: https://reviews.apache.org/r/43969/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 44239: Introduced copy fetcher plugin for file URIs.

2016-03-02 Thread Jojy Varghese

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

(Updated March 3, 2016, 1:47 a.m.)


Review request for mesos and Jie Yu.


Changes
---

review addressed.


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


Repository: mesos


Description
---

This change adds support for fetching `file` URIs using the fetcher plugin
framework.


Diffs (updated)
-

  src/CMakeLists.txt 0832f1e9a850cc0d2c1112a446f0daf0190f8d7f 
  src/Makefile.am 5d8fe8bf33acb352589b2b3e0341987f9a41bc17 
  src/tests/uri_fetcher_tests.cpp dcfba784d6ae49b8181b5a69dc00e82854ac4ff1 
  src/uri/fetcher.hpp 5bc9703b76457413b5841710e0313f8bfa9047a1 
  src/uri/fetcher.cpp 2d8d5b18b7a9b55be52288f17e85d6c406cfef88 
  src/uri/fetchers/copy.hpp PRE-CREATION 
  src/uri/fetchers/copy.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 44275: Fixed flakiness in tests using the scheduler library.

2016-03-02 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [44273, 44274, 44275]

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 3, 2016, 1:31 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44275/
> ---
> 
> (Updated March 3, 2016, 1:31 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4029
> https://issues.apache.org/jira/browse/MESOS-4029
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change fixes the tests using the scheduler library by
> explicitly invoking `stop()` from the testing interface. This
> ensures that no further callbacks are delivered to the scheduler.
> 
> For one-off async callbacks that are already on the libprocess queue
> we need to do a `Clock::settle` to ensure they are executed before
> the mock object goes out of scope.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 1d4f075e470a60040e17b9f011aea6202310c437 
> 
> Diff: https://reviews.apache.org/r/44275/diff/
> 
> 
> Testing
> ---
> 
> make check + Induced a sleep in the async callbacks. Previously, the test 
> used to crash but with the fix it no longer crashes.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 44278: Added HTTP scheduler stream IDs.

2016-03-02 Thread Anand Mazumdar


> On March 3, 2016, 1:37 a.m., Vinod Kone wrote:
> > src/master/http.cpp, lines 458-462
> > 
> >
> > should these be BadRequest as well?

I think `Forbidden` is more apt here. _There is nothing wrong with the request 
object per se i.e. being malformed etc._

In both the cases, the framework is trying to perform an operation that it is 
not allowed to do i.e. trying to send a non-subscribe call without 
subscribing/trying to send a non-subscribe call when previously subscribed as a 
driver based framework.


- Anand


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


On March 2, 2016, 10:36 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44278/
> ---
> 
> (Updated March 2, 2016, 10:36 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3583
> https://issues.apache.org/jira/browse/MESOS-3583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HTTP scheduler stream IDs.
> 
> In some failure scenarios involving highly-available HTTP schedulers with 
> multiple instances, it's possible for a non-leading instance to successfully 
> make HTTP calls to the master. This patch enables the master to use HTTP 
> scheduler stream IDs to uniquely identify each HTTP subscription stream, 
> preventing any non-leading scheduler instance from making calls to the 
> master. The patch also adds stream ID support to the HTTP scheduler library.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
> 
> Diff: https://reviews.apache.org/r/44278/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on both OSX and CentOS 7.1.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 44279: Added stream IDs to the HTTP API docs.

2016-03-02 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On March 2, 2016, 8:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44279/
> ---
> 
> (Updated March 2, 2016, 8:59 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3583
> https://issues.apache.org/jira/browse/MESOS-3583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stream IDs to the HTTP API docs.
> 
> 
> Diffs
> -
> 
>   CHANGELOG e6cc39b4f45317f94145f0a4a1b64215b9a0cbff 
>   docs/scheduler-http-api.md 53b482a813fa3909e786791d5331ef11c8fcc662 
> 
> Diff: https://reviews.apache.org/r/44279/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 44290: Added tests involving HTTP scheduler stream IDs.

2016-03-02 Thread Vinod Kone

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


Fix it, then Ship it!





src/tests/scheduler_http_api_tests.cpp (line 899)


// Send a TEARDOWN call without stream ID.



src/tests/scheduler_http_api_tests.cpp (line 1039)


nice test!


- Vinod Kone


On March 2, 2016, 10:20 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44290/
> ---
> 
> (Updated March 2, 2016, 10:20 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3583
> https://issues.apache.org/jira/browse/MESOS-3583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests involving HTTP scheduler stream IDs.
> 
> Three new tests have been added in this patch: 
> SchedulerHttpApiTest.TeardownWithoutStreamId, 
> SchedulerHttpApiTest.TeardownWrongStreamId, and 
> SchedulerHttpApiTest.SubscribeWithStreamId.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_http_api_tests.cpp 
> 428e12646d80b45daec30cfe607b97f36170fdf5 
> 
> Diff: https://reviews.apache.org/r/44290/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on both OSX and CentOS 7.1
> 
> The new tests were run 1000 times to look for flakiness; no failures were 
> observed.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 44278: Added HTTP scheduler stream IDs.

2016-03-02 Thread Vinod Kone

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




src/master/http.cpp (line 440)


s/stream/streamId/



src/master/http.cpp (lines 458 - 462)


should these be BadRequest as well?



src/master/master.hpp (line 1605)


s/_stream/_streamId/



src/master/master.hpp (line 1608)


s/stream/streamId/



src/scheduler/scheduler.cpp (lines 501 - 502)


pull this down to #516



src/scheduler/scheduler.cpp (line 678)


s/stream/streamId/


- Vinod Kone


On March 2, 2016, 10:36 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44278/
> ---
> 
> (Updated March 2, 2016, 10:36 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3583
> https://issues.apache.org/jira/browse/MESOS-3583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HTTP scheduler stream IDs.
> 
> In some failure scenarios involving highly-available HTTP schedulers with 
> multiple instances, it's possible for a non-leading instance to successfully 
> make HTTP calls to the master. This patch enables the master to use HTTP 
> scheduler stream IDs to uniquely identify each HTTP subscription stream, 
> preventing any non-leading scheduler instance from making calls to the 
> master. The patch also adds stream ID support to the HTTP scheduler library.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
> 
> Diff: https://reviews.apache.org/r/44278/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on both OSX and CentOS 7.1.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 44275: Fixed flakiness in tests using the scheduler library.

2016-03-02 Thread Anand Mazumdar

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

(Updated March 3, 2016, 1:31 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments from Vinod


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


Repository: mesos


Description
---

This change fixes the tests using the scheduler library by
explicitly invoking `stop()` from the testing interface. This
ensures that no further callbacks are delivered to the scheduler.

For one-off async callbacks that are already on the libprocess queue
we need to do a `Clock::settle` to ensure they are executed before
the mock object goes out of scope.


Diffs (updated)
-

  src/tests/mesos.hpp 1d4f075e470a60040e17b9f011aea6202310c437 

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


Testing
---

make check + Induced a sleep in the async callbacks. Previously, the test used 
to crash but with the fix it no longer crashes.


Thanks,

Anand Mazumdar



Re: Review Request 44275: Fixed flakiness in tests using the scheduler library.

2016-03-02 Thread Anand Mazumdar


> On March 3, 2016, 1:10 a.m., Vinod Kone wrote:
> > src/tests/mesos.hpp, lines 992-994
> > 
> >
> > MESOS-4029 is talking about a bunch of issues. The final thing it says 
> > is to use a shared_ptr. There is nothing about why there needs to be a 
> > stop() and this settle().
> > 
> > Can you explain here with a little more detail for posterity? both the 
> > stop() and settle().

+1. I removed the link to the JIRA since it was doing more harm than good. I 
modified the comment slightly and it looks pretty self-explanatory to me now.


- Anand


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


On March 2, 2016, 9:08 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44275/
> ---
> 
> (Updated March 2, 2016, 9:08 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4029
> https://issues.apache.org/jira/browse/MESOS-4029
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change fixes the tests using the scheduler library by
> explicitly invoking `stop()` from the testing interface. This
> ensures that no further callbacks are delivered to the scheduler.
> 
> For one-off async callbacks that are already on the libprocess queue
> we need to do a `Clock::settle` to ensure they are executed before
> the mock object goes out of scope.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 1d4f075e470a60040e17b9f011aea6202310c437 
> 
> Diff: https://reviews.apache.org/r/44275/diff/
> 
> 
> Testing
> ---
> 
> make check + Induced a sleep in the async callbacks. Previously, the test 
> used to crash but with the fix it no longer crashes.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 44274: Added the ability to stop running the scheduler library process.

2016-03-02 Thread Anand Mazumdar

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

(Updated March 3, 2016, 1:30 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments from Vinod


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


Repository: mesos


Description
---

This change adds the ability to stop running the scheduler library
process so that no future callbacks are delivered to the scheduler.

This helps us during testing to ensure no further callbacks happen
to stack allocated mock objects.


Diffs (updated)
-

  include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
  src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 

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


Testing
---

make check. This is used later in the chain when we move the interface to use 
the `stop()` function.


Thanks,

Anand Mazumdar



Re: Review Request 44274: Added the ability to stop running the scheduler library process.

2016-03-02 Thread Anand Mazumdar


> On March 2, 2016, 11:59 p.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp, line 463
> > 
> >
> > don't you need to guard this with !running.load() check?

To be more precise, I moved all the `running.load()` calls to the `async` 
callbacks.


> On March 2, 2016, 11:59 p.m., Vinod Kone wrote:
> > include/mesos/v1/scheduler.hpp, line 86
> > 
> >
> > s/interface/library/
> > 
> > what about calls? can they make calls after calling stop? looks like no 
> > from the implementation below, but it needs to be commented here.
> > 
> > also here it says "no callbacks" but down below it says "atmost one 
> > callback" ?

+1. My bad, I had tried to follow the driver that punts on adding the "atmost 
one" detail in the header.


- Anand


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


On March 3, 2016, 1:30 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44274/
> ---
> 
> (Updated March 3, 2016, 1:30 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4029
> https://issues.apache.org/jira/browse/MESOS-4029
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the ability to stop running the scheduler library
> process so that no future callbacks are delivered to the scheduler.
> 
> This helps us during testing to ensure no further callbacks happen
> to stack allocated mock objects.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
> 
> Diff: https://reviews.apache.org/r/44274/diff/
> 
> 
> Testing
> ---
> 
> make check. This is used later in the chain when we move the interface to use 
> the `stop()` function.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 44273: Modified scheduler tests to use the callback interface.

2016-03-02 Thread Anand Mazumdar

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

(Updated March 3, 2016, 1:29 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change modifies the existing scheduler tests to use the new callback
interface introduced in MESOS-3339.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp 2b1693eaf1a6106f5e7d269e4e3f6c353dd6f017 

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


Testing
---

make check (gtest_repeat=100)

I did not yet modify the maintenance tests due to a bug I found while
modifying the tests. Filed MESOS-4831.


Thanks,

Anand Mazumdar



Re: Review Request 44298: Added support for file URI in Appc fetcher.

2016-03-02 Thread Jie Yu

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


Ship it!




Ship It!

- Jie Yu


On March 2, 2016, 9:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44298/
> ---
> 
> (Updated March 2, 2016, 9:47 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4822
> https://issues.apache.org/jira/browse/MESOS-4822
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for file URI in Appc fetcher.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp 
> e12a6f27866b6362191ea4dafe8bf818b33cd9e3 
> 
> Diff: https://reviews.apache.org/r/44298/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 44239: Introduced copy fetcher plugin for file URIs.

2016-03-02 Thread Jie Yu

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


Fix it, then Ship it!





src/uri/fetchers/copy.cpp (line 85)


Why do you need this?



src/uri/fetchers/copy.cpp (line 87)


Copying uri xxx to 'directory'



src/uri/fetchers/copy.cpp (lines 132 - 137)


Why do you need this?


- Jie Yu


On March 2, 2016, 9:44 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44239/
> ---
> 
> (Updated March 2, 2016, 9:44 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4834
> https://issues.apache.org/jira/browse/MESOS-4834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds support for fetching `file` URIs using the fetcher plugin
> framework.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0832f1e9a850cc0d2c1112a446f0daf0190f8d7f 
>   src/Makefile.am 5d8fe8bf33acb352589b2b3e0341987f9a41bc17 
>   src/tests/uri_fetcher_tests.cpp dcfba784d6ae49b8181b5a69dc00e82854ac4ff1 
>   src/uri/fetcher.hpp 5bc9703b76457413b5841710e0313f8bfa9047a1 
>   src/uri/fetcher.cpp 2d8d5b18b7a9b55be52288f17e85d6c406cfef88 
>   src/uri/fetchers/copy.hpp PRE-CREATION 
>   src/uri/fetchers/copy.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44239/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 44251: Improved allocator perf for labeled reservations and volumes.

2016-03-02 Thread Joris Van Remoortere

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 2, 2016, 7:17 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44251/
> ---
> 
> (Updated March 2, 2016, 7:17 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4833
> https://issues.apache.org/jira/browse/MESOS-4833
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the cluster contains many resources that have either labeled
> reservations or persistent volumes, allocator performance can decrease
> substantially because such metadata prevents `Resource` objects from
> being merged together inside the allocator. As a result, the allocator
> must manipulate `Resources` vectors that consist of many small
> individual `Resource` values; since many `Resources` operations take
> linear-time in the number of `Resource` values they contain, this can
> cause very significant slowdowns.
> 
> As a short-term solution, this commit strips dynamic reservation and
> persistent volume information from the `Resources` objects used
> internally by the allocator, because they are not needed when
> aggregating resource quantities together.
> 
> A long-term solution for this problem will be addressed as work on
> refactoring the allocator more generally.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 24fa50f62dec10ed43089297473cc386d6ba2f78 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 46b2a9caf13b028a3aee6c1590679f885be90fd6 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 9e863dd0ca5e2f2f0d517cb833687b757bed2c52 
>   src/master/allocator/sorter/sorter.hpp 
> ba91a38e47065718af87c9b3b7c5b74d25a258df 
> 
> Diff: https://reviews.apache.org/r/44251/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Perf:
> 
> ```
> [ RUN  ] HierarchicalAllocator_BENCHMARK_Test.ResourceLabels
> Using 2000 slaves and 200 frameworks
> round 0 allocate took 2.458462secs to make 200 offers
> round 1 allocate took 2.427941secs to make 200 offers
> round 2 allocate took 2.460724secs to make 200 offers
> round 3 allocate took 2.443408secs to make 200 offers
> round 4 allocate took 2.464784secs to make 200 offers
> round 5 allocate took 2.501429secs to make 200 offers
> round 6 allocate took 2.468777secs to make 200 offers
> round 7 allocate took 2.482268secs to make 200 offers
> round 8 allocate took 2.479014secs to make 200 offers
> round 9 allocate took 2.529951secs to make 200 offers
> round 10 allocate took 2.460059secs to make 200 offers
> ```
> 
> Performance of `DeclineOffers` without labels is about ~2.1 seconds.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 44275: Fixed flakiness in tests using the scheduler library.

2016-03-02 Thread Vinod Kone

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




src/tests/mesos.hpp (lines 992 - 994)


MESOS-4029 is talking about a bunch of issues. The final thing it says is 
to use a shared_ptr. There is nothing about why there needs to be a stop() and 
this settle().

Can you explain here with a little more detail for posterity? both the 
stop() and settle().


- Vinod Kone


On March 2, 2016, 9:08 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44275/
> ---
> 
> (Updated March 2, 2016, 9:08 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4029
> https://issues.apache.org/jira/browse/MESOS-4029
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change fixes the tests using the scheduler library by
> explicitly invoking `stop()` from the testing interface. This
> ensures that no further callbacks are delivered to the scheduler.
> 
> For one-off async callbacks that are already on the libprocess queue
> we need to do a `Clock::settle` to ensure they are executed before
> the mock object goes out of scope.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 1d4f075e470a60040e17b9f011aea6202310c437 
> 
> Diff: https://reviews.apache.org/r/44275/diff/
> 
> 
> Testing
> ---
> 
> make check + Induced a sleep in the async callbacks. Previously, the test 
> used to crash but with the fix it no longer crashes.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 44266: Rename event_call_framework.cpp to test_http_framework.cpp.

2016-03-02 Thread Vinod Kone

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



Looks great.

Can you also rename the class name in test_http_framework.cpp? 
s/EventCallScheduler/HTTPScheduler/

- Vinod Kone


On March 2, 2016, 3:11 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44266/
> ---
> 
> (Updated March 2, 2016, 3:11 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-4583
> https://issues.apache.org/jira/browse/MESOS-4583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change rename event_call_framework to test_http_framework
> in order to correctly reflect that it's an example for HTTP based
> framework. (MESOS-4583)
> 
> 
> Diffs
> -
> 
>   src/Makefile.am fe549a881e778e094d3f957e5119eeb2f55e7915 
>   src/examples/event_call_framework.cpp  
>   src/tests/event_call_framework_test.sh 
> cddb5208bce29cf84c40ff76aff0163d110a98d1 
>   src/tests/examples_tests.cpp 9b8b30f3718aa0fa9d02a3aa344634f0c03e61e6 
> 
> Diff: https://reviews.apache.org/r/44266/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>



Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

2016-03-02 Thread Joseph Wu

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

(Updated March 2, 2016, 4:05 p.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Propagating changes due to the reintroduction of the Try<> in the factory 
method's return type.


Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634


Repository: mesos


Description
---

Includes the following changes:

* Added the `` header where appropriate.
* Added the namespace `using process::Owned;` where appropriate.
* Generally replaced `Try` with `Owned`.  And 
`Try` with `Owned`.
* Added the (now required) `MasterDetector` argument to all slaves.  Before, 
this was fetched from the first master in `Cluster`.
* Removed `Shutdown();` from all tests.
* Replaced `Stop(...)` with the appropriate master/slave destruction calls.
* Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
launchers, etc).
* Replace `CHECK` in tests with `ASSERT`.


Diffs (updated)
-

  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/command_executor_tests.cpp 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
  src/tests/container_logger_tests.cpp 00f4129e46aa9268fbb66da25b34e61004fa87b2 
  src/tests/containerizer/docker_containerizer_tests.cpp 
6aecd912fc84b72d2b64f7a842891fddcbc469ac 
  src/tests/containerizer/external_containerizer_test.cpp 
8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
e72239a55724f1aeeec5362cc370c93dbeca7164 
  src/tests/containerizer/isolator_tests.cpp 
342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
  src/tests/containerizer/memory_pressure_tests.cpp 
79f134996c4b80bf49cbb8bee28eab5e6b4f5822 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
  src/tests/containerizer/port_mapping_tests.cpp 
983a6be160aefe5a32acb6111bb3c85230ec 
  src/tests/containerizer/provisioner_docker_tests.cpp 
5b685bfd842d0d98e8ea5ec5ddea8d2cd893dd81 
  src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
  src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
  src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
  src/tests/executor_http_api_tests.cpp 
2fc0893f5f5e80a783296fb31b30abe86d92df1b 
  src/tests/fault_tolerance_tests.cpp f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
  src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
  src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
  src/tests/hook_tests.cpp 88ad7806bf9f3194e434743c35199a896edea92c 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
29c89fb11da792c3e71eb880a19657ea225b3cc8 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_quota_tests.cpp 8357ec911b2a158632a708ae3adff6eabc536697 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp 36595772b34bcb8d37dbc74d247bdf4614f10150 
  src/tests/master_validation_tests.cpp 
c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 
  src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
  src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_endpoints_tests.cpp 
81185a161498394020a27f1f5bf747bac5425f43 
  src/tests/persistent_volume_tests.cpp 
bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
  src/tests/rate_limiting_tests.cpp caced732ded05a334861a53488ef6391885b2263 
  src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
  src/tests/registrar_zookeeper_tests.cpp 
3df9779ee5d076e16f6a538326693a36f986b6d0 
  src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da 
  src/tests/reservation_endpoints_tests.cpp 
f3a143812aa10bc445ac5d27c00318e91eb086aa 
  src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 
  src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 
  src/tests/scheduler_driver_tests.cpp f6dc25d82ae5f1e77fc6ede7ff2660ed0d9ea039 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
428e12646d80b45daec30cfe607b97f36170fdf5 
  src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 
  src/tests/status_update_manager_tests.cpp 
d64d3b8c96270478f6b681c038de77c3a9eb68fe 
  src/tests/teardown_tests.cpp 6e9e2e64f1666c2a81f5d859ee014ee14365b6b0 

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


Testing 

Re: Review Request 44274: Added the ability to stop running the scheduler library process.

2016-03-02 Thread Vinod Kone

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




include/mesos/v1/scheduler.hpp (line 86)


s/interface/library/

what about calls? can they make calls after calling stop? looks like no 
from the implementation below, but it needs to be commented here.

also here it says "no callbacks" but down below it says "atmost one 
callback" ?



src/scheduler/scheduler.cpp (line 314)


s/connection attempt/connected event/



src/scheduler/scheduler.cpp (line 409)


s/change\/disconnection/detected event/



src/scheduler/scheduler.cpp (line 463)


don't you need to guard this with !running.load() check?


- Vinod Kone


On March 2, 2016, 6:21 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44274/
> ---
> 
> (Updated March 2, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4029
> https://issues.apache.org/jira/browse/MESOS-4029
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the ability to stop running the scheduler library
> process so that no future callbacks are delivered to the scheduler.
> 
> This helps us during testing to ensure no further callbacks happen
> to stack allocated mock objects.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
> 
> Diff: https://reviews.apache.org/r/44274/diff/
> 
> 
> Testing
> ---
> 
> make check. This is used later in the chain when we move the interface to use 
> the `stop()` function.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 44273: Modified scheduler tests to use the callback interface.

2016-03-02 Thread Vinod Kone

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


Fix it, then Ship it!




Thanks for the cleanup.


src/tests/scheduler_tests.cpp (lines 655 - 659)


Split this expectation. The expectation for udpate2 should be set after 
#699.



src/tests/scheduler_tests.cpp (lines 1101 - 1105)


split this. set the expectation for offers2 after #1124.



src/tests/scheduler_tests.cpp (lines 1202 - 1206)


ditto. split please.


- Vinod Kone


On March 2, 2016, 8:28 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44273/
> ---
> 
> (Updated March 2, 2016, 8:28 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4798
> https://issues.apache.org/jira/browse/MESOS-4798
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change modifies the existing scheduler tests to use the new callback
> interface introduced in MESOS-3339.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 2b1693eaf1a6106f5e7d269e4e3f6c353dd6f017 
> 
> Diff: https://reviews.apache.org/r/44273/diff/
> 
> 
> Testing
> ---
> 
> make check (gtest_repeat=100)
> 
> I did not yet modify the maintenance tests due to a bug I found while
> modifying the tests. Filed MESOS-4831.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 44070: Allowed disabling metrics endpoint rate limiting via the environment.

2016-03-02 Thread Alexander Rojas

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




3rdparty/libprocess/src/metrics/metrics.cpp (lines 197 - 199)


This is definitely not enough to prevent a race here. Two `rateLimiters` 
may very well instantiated (though the once below will prevent the singleton 
from being initialized twice).

Can we move the `Once initialize` test right after this if?


- Alexander Rojas


On March 2, 2016, 6:48 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44070/
> ---
> 
> (Updated March 2, 2016, 6:48 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4776
> https://issues.apache.org/jira/browse/MESOS-4776
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed disabling metrics endpoint rate limiting via the environment.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 09b716be56eac38f75d79d917799c001aa0b205c 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> a9840083722dd6b7b6aab692ed449407ab125ac7 
> 
> Diff: https://reviews.apache.org/r/44070/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, not optimized)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 44286: Unified/Added Future checks for http methods in tests.

2016-03-02 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [44286]

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 2, 2016, 8:53 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44286/
> ---
> 
> (Updated March 2, 2016, 8:53 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified/Added Future checks for http methods in tests.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> 982468f851cd9d95eb6cde7c57f2d737d46a827c 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
> 
> Diff: https://reviews.apache.org/r/44286/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation

2016-03-02 Thread Greg Mann

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



Looks good! A couple small comments.


docs/monitoring.md (line 605)


s/distroy/destroy/



docs/monitoring.md (line 607)


s/distroy/destroy/



src/master/master.cpp (line 2942)


s/distroy/destroy/



src/master/metrics.hpp (line 131)


It's a bit more consistent with the other existing metrics here to make the 
names of these plural - i.e., `messages_reserve_resources`, 
`messages_unreserve_resources`, etc.



src/master/metrics.hpp (line 134)


s/distroy/destroy/, here and below


- Greg Mann


On March 2, 2016, 5:18 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 2, 2016, 5:18 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4492
> https://issues.apache.org/jira/browse/MESOS-4492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Fan Du 
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
> 
> Diff: https://reviews.apache.org/r/44255/diff/
> 
> 
> Testing
> ---
> 
> ChangLog:
> v2:
>   - Documenting those metrics
>   - Add test code for MetricsTest as suggested by Guangya
>   - post-review.py does not update original 
> RR(https://reviews.apache.org/r/44058/), but only create a new one even if I 
> rebased.
> 
> 
> Tests:
> 1. make check GTEST_FILTER="MetricsTest.Master" on Centos-7 
> (3.10.0-123.el7.x86_640)
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from MetricsTest
> [ RUN  ] MetricsTest.Master
> [   OK ] MetricsTest.Master (211 ms)
> [--] 1 test from MetricsTest (211 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (244 ms total)
> [  PASSED  ] 1 test
> 
> 2. Verify its functionality with 'reserve' http endpoint as an test case
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 0.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> # curl -i -d slaveId=6250553a-2f39-4a92-9073-4618d130f433-S1  -d resources='[ 
> { "name": "cpus", "type": "SCALAR","scalar": { "value": 1 
> },"reservation":{"principal": "XiaoHaHa"}}  ]' -X POST  
> ipdc02-kvm-guest2:5050/master/reserve
> HTTP/1.1 200 OK
> Date: Fri, 26 Feb 2016 19:59:01 GMT
> Content-Length: 0
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot  | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 1.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> Thanks,
> 
> fan du
> 
>



Re: Review Request 44278: Added HTTP scheduler stream IDs.

2016-03-02 Thread Greg Mann

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

(Updated March 2, 2016, 10:36 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added HTTP scheduler stream IDs.

In some failure scenarios involving highly-available HTTP schedulers with 
multiple instances, it's possible for a non-leading instance to successfully 
make HTTP calls to the master. This patch enables the master to use HTTP 
scheduler stream IDs to uniquely identify each HTTP subscription stream, 
preventing any non-leading scheduler instance from making calls to the master. 
The patch also adds stream ID support to the HTTP scheduler library.


Diffs (updated)
-

  src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
  src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
  src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 

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


Testing
---

`make check` was used to test on both OSX and CentOS 7.1.


Thanks,

Greg Mann



Re: Review Request 44290: Added tests involving HTTP scheduler stream IDs.

2016-03-02 Thread Greg Mann

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

(Updated March 2, 2016, 10:20 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added tests involving HTTP scheduler stream IDs.

Three new tests have been added in this patch: 
SchedulerHttpApiTest.TeardownWithoutStreamId, 
SchedulerHttpApiTest.TeardownWrongStreamId, and 
SchedulerHttpApiTest.SubscribeWithStreamId.


Diffs (updated)
-

  src/tests/scheduler_http_api_tests.cpp 
428e12646d80b45daec30cfe607b97f36170fdf5 

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


Testing
---

`make check` was used to test on both OSX and CentOS 7.1

The new tests were run 1000 times to look for flakiness; no failures were 
observed.


Thanks,

Greg Mann



Re: Review Request 44278: Added HTTP scheduler stream IDs.

2016-03-02 Thread Anand Mazumdar

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



Looks good. Mostly comments around having explicit `CHECK`'s if the stream id 
header is missing.


src/master/http.cpp (line 464)


Remove period at the end.



src/master/http.cpp (line 468)


hmm.. the `framework->http` access can crash the master.

Consider this scenario, if a driver based framework, now tries to send a 
non-subscribe call. In that case `framework->http` would be `None()`.

We should do the following after L459.

```cpp
if (framework->http.isNone()) {
  return Forbidden("Framework not connected via HTTP");
}
```



src/scheduler/scheduler.cpp (lines 253 - 256)


Let's make this an explicit check since we are sure that the master would 
set it:

```
CHECK_SOME(stream);
```



src/scheduler/scheduler.cpp (line 518)


Let's have this as an explicit `CHECK` along with the other checks on L504 
and remove this line.


- Anand Mazumdar


On March 2, 2016, 8:58 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44278/
> ---
> 
> (Updated March 2, 2016, 8:58 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3583
> https://issues.apache.org/jira/browse/MESOS-3583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added HTTP scheduler stream IDs.
> 
> In some failure scenarios involving highly-available HTTP schedulers with 
> multiple instances, it's possible for a non-leading instance to successfully 
> make HTTP calls to the master. This patch enables the master to use HTTP 
> scheduler stream IDs to uniquely identify each HTTP subscription stream, 
> preventing any non-leading scheduler instance from making calls to the 
> master. The patch also adds stream ID support to the HTTP scheduler library.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
> 
> Diff: https://reviews.apache.org/r/44278/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on both OSX and CentOS 7.1.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Review Request 44299: Added unit test for file URI fetcher.

2016-03-02 Thread Jojy Varghese

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added unit test for file URI fetcher.


Diffs
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
9d9779ac04e32348ce0e28da54fd7aa039a7fbcd 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 43969: Added test for Appc image fetcher.

2016-03-02 Thread Jojy Varghese

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

(Updated March 2, 2016, 9:47 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added simple appc Fetcher test with mock HTTP image server.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
9d9779ac04e32348ce0e28da54fd7aa039a7fbcd 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Review Request 44298: Added support for file URI in Appc fetcher.

2016-03-02 Thread Jojy Varghese

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added support for file URI in Appc fetcher.


Diffs
-

  src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp 
e12a6f27866b6362191ea4dafe8bf818b33cd9e3 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 43629: Especially updated tests to use the updated MesosTest helpers.

2016-03-02 Thread Joseph Wu

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

(Updated March 2, 2016, 1:45 p.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Propagating changes due to the reintroduction of the Try<> in the factory 
method's return type.


Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634


Repository: mesos


Description
---

Continuation of https://reviews.apache.org/r/43615/ with a slightly different 
pattern.


Diffs (updated)
-

  src/tests/fetcher_cache_tests.cpp f9c48f5d938c2601cb8f826029d6969d676ab98e 
  src/tests/resource_offers_tests.cpp 0bad45dd1dabecc88fef1ab46e8ea26718070b33 
  src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 

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


Testing
---

Tests are run at the end of this review chain.


Thanks,

Joseph Wu



Re: Review Request 43613: Refactor cluster test helpers into self-contained objects.

2016-03-02 Thread Joseph Wu


> On March 1, 2016, 5:56 a.m., Bernd Mathiske wrote:
> > src/tests/cluster.cpp, line 97
> > 
> >
> > The local assertions here do not stop the test outside the factory 
> > method from progressing. This hazard creates implicit dependencies between 
> > the implementation here and test code following call sites.
> > 
> > Not making the return type here a Try makes the whole method and code 
> > following its call sites more fragile. Everything may look rock-solid right 
> > now, but if anyone later makes any changes, she must keep in mind that none 
> > of the pointers in members of the resulting master object can become stale. 
> > Otherwise they can cause test crashes. Not saying there are crashes now, 
> > but the probability of introducing them just went up.
> > 
> > Worse: should every test writer read the implementation code here and 
> > consider this? This kind of dependency slows down development. I suggest we 
> > avoid it.
> > 
> > If there were still an ASSERT_SOME(master) in every test right behind 
> > the creation of the master, then the test would exit prematurely and no 
> > stale members could cause crashes.

Ok.  Changed all these asserts back into errors.


> On March 1, 2016, 5:56 a.m., Bernd Mathiske wrote:
> > src/tests/cluster.cpp, line 313
> > 
> >
> > FAIL() is commonly used to indicate a test failure. But such a failure 
> > should pertain to the subject matter of the individual test, which is not 
> > the case here.
> > 
> > gtest might simply mark this test as failed, but the situation is 
> > actually worse. Here we are looking at a malfunction of non-specific 
> > support code.
> > 
> > However, if we use a Try return type as discussed above, the 
> > ASSERT_SOME in the top level test code makes things right again.
> > 
> > Still, strictly speaking, it "look more "correct" if writing this here, 
> > assuming the Try:
> > 
> > LOG(FATAL) << "Failed to wait for _recover";
> > return;

I've changed this one to return an Error.

Note that `LOG(FATAL)` is roughly equivalent to `EXIT(EXIT_FAILURE)`.  (Rather 
than `ASSERT_TRUE(false)`.)


> On March 1, 2016, 5:56 a.m., Bernd Mathiske wrote:
> > src/tests/cluster.cpp, line 124
> > 
> >
> > In order to cause an Error to be returned as the overall result when 
> > such an assertion fires, you could capture a bool by reference that gets 
> > set at the very end of the inline closure. If the bool is false, we have an 
> > Error.

Ended up getting rid of the inner closure for the `start` factories.


- Joseph


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


On Feb. 26, 2016, 11:50 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43613/
> ---
> 
> (Updated Feb. 26, 2016, 11:50 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Artem 
> Harutyunyan, and Michael Park.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Major rewrite of the `tests/cluster` helpers.  This strongly ties the scope 
> of test objects to the test body.
> 
> Changes the `Cluster` class into two RAII objects (`Master` and `Slave`).  
> The `Slave` object performs cleanup originally found in 
> `cluster::Slave::stop`.  `cluster::Master::start` and `cluster::Slave::start` 
> were changed to factory methods.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
> 
> Diff: https://reviews.apache.org/r/43613/diff/
> 
> 
> Testing
> ---
> 
> Tests are run at the end of this review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 43630: Especially updated scheduler tests to use the updated MesosTest helpers.

2016-03-02 Thread Joseph Wu

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

(Updated March 2, 2016, 1:45 p.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Propagating changes due to the reintroduction of the Try<> in the factory 
method's return type.


Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634


Repository: mesos


Description
---

Continuation of https://reviews.apache.org/r/43615/ with re-ordering of some 
local variables due to the order of destruction.


Diffs (updated)
-

  src/tests/master_maintenance_tests.cpp 
3faa8136cf57276295553910319480028f433e4c 
  src/tests/scheduler_tests.cpp 2b1693eaf1a6106f5e7d269e4e3f6c353dd6f017 

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


Testing
---

Tests are run at the end of this review chain.


Thanks,

Joseph Wu



Re: Review Request 44239: Introduced copy fetcher plugin for file URIs.

2016-03-02 Thread Jojy Varghese


> On March 2, 2016, 3:26 p.m., Guangya Liu wrote:
> > src/uri/fetchers/copy.cpp, line 72
> > 
> >
> > Printing the `uri` in the failure may help improve the debug ability?

The pattern is same as all other fetchers. Will drop this issue.


- Jojy


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


On March 2, 2016, 12:13 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44239/
> ---
> 
> (Updated March 2, 2016, 12:13 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4834
> https://issues.apache.org/jira/browse/MESOS-4834
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds support for fetching `file` URIs using the fetcher plugin
> framework.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0832f1e9a850cc0d2c1112a446f0daf0190f8d7f 
>   src/Makefile.am 5d8fe8bf33acb352589b2b3e0341987f9a41bc17 
>   src/tests/uri_fetcher_tests.cpp dcfba784d6ae49b8181b5a69dc00e82854ac4ff1 
>   src/uri/fetcher.hpp 5bc9703b76457413b5841710e0313f8bfa9047a1 
>   src/uri/fetcher.cpp 2d8d5b18b7a9b55be52288f17e85d6c406cfef88 
>   src/uri/fetchers/copy.hpp PRE-CREATION 
>   src/uri/fetchers/copy.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44239/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 44280: Removed `FLAGS_v` assignment in test case.

2016-03-02 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43684, 43685, 43686, 44250, 44251, 44280]

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 2, 2016, 6:37 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44280/
> ---
> 
> (Updated March 2, 2016, 6:37 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Per discussion with James Peach, this doesn't serve a
> useful purpose.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/44280/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 44239: Introduced copy fetcher plugin for file URIs.

2016-03-02 Thread Jojy Varghese

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

(Updated March 2, 2016, 9:44 p.m.)


Review request for mesos and Jie Yu.


Changes
---

review addressed.


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


Repository: mesos


Description
---

This change adds support for fetching `file` URIs using the fetcher plugin
framework.


Diffs (updated)
-

  src/CMakeLists.txt 0832f1e9a850cc0d2c1112a446f0daf0190f8d7f 
  src/Makefile.am 5d8fe8bf33acb352589b2b3e0341987f9a41bc17 
  src/tests/uri_fetcher_tests.cpp dcfba784d6ae49b8181b5a69dc00e82854ac4ff1 
  src/uri/fetcher.hpp 5bc9703b76457413b5841710e0313f8bfa9047a1 
  src/uri/fetcher.cpp 2d8d5b18b7a9b55be52288f17e85d6c406cfef88 
  src/uri/fetchers/copy.hpp PRE-CREATION 
  src/uri/fetchers/copy.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 44290: Added tests involving HTTP scheduler stream IDs.

2016-03-02 Thread Anand Mazumdar

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



Looks good. Just some minor cleanup comments.


src/tests/scheduler_http_api_tests.cpp (line 290)


We should also explicitly check that the "Mesos-Stream-Id" header is not 
equal to "" in addition to it being present.



src/tests/scheduler_http_api_tests.cpp (line 329)


s/Bad Request/BadRequest

Here and the other 2 more occurences please.



src/tests/scheduler_http_api_tests.cpp (line 852)


s/subscribeCall/call

Also can you scope all the calls?

{
  Call call;
  call.set_type(Call::SUBSCRIBE);
  ...
}

{
  Call call;
  call.set_type(Call::TEARDOWN);
}



src/tests/scheduler_http_api_tests.cpp (line 894)


s/teardownCall/call

Can we also scope this?



src/tests/scheduler_http_api_tests.cpp (line 922)


Can we move this before L938 i.e. in the next scope?



src/tests/scheduler_http_api_tests.cpp (line 973)


Can we define another `Call` here explicitly and not re-use the old 
instance?



src/tests/scheduler_http_api_tests.cpp (line 1010)


s/teardownCall/call


- Anand Mazumdar


On March 2, 2016, 8:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44290/
> ---
> 
> (Updated March 2, 2016, 8:59 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-3583
> https://issues.apache.org/jira/browse/MESOS-3583
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests involving HTTP scheduler stream IDs.
> 
> Three new tests have been added in this patch: 
> SchedulerHttpApiTest.TeardownWithoutStreamId, 
> SchedulerHttpApiTest.TeardownWrongStreamId, and 
> SchedulerHttpApiTest.SubscribeWithStreamId.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_http_api_tests.cpp 
> 428e12646d80b45daec30cfe607b97f36170fdf5 
> 
> Diff: https://reviews.apache.org/r/44290/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on both OSX and CentOS 7.1
> 
> The new tests were run 1000 times to look for flakiness; no failures were 
> observed.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-02 Thread Anurag Singh

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

(Updated March 2, 2016, 9:30 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs (updated)
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh



Re: Review Request 44288: Changed MasterDetector/Contender namespace.

2016-03-02 Thread Anurag Singh

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

(Updated March 2, 2016, 9:30 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Also modified users of MasterContender and MasterDetector to use this
namespace.


Diffs (updated)
-

  include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
  include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
  src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
  src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
  src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
  src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
  src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
  src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
  src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
  src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
  src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
  src/tests/fault_tolerance_tests.cpp f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
29c89fb11da792c3e71eb880a19657ea225b3cc8 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp 36595772b34bcb8d37dbc74d247bdf4614f10150 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_tests.cpp 
bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
  src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
  src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
428e12646d80b45daec30cfe607b97f36170fdf5 
  src/tests/scheduler_tests.cpp 2b1693eaf1a6106f5e7d269e4e3f6c353dd6f017 
  src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
  src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh



Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-02 Thread Anurag Singh


> On March 2, 2016, 9:20 p.m., Joseph Wu wrote:
> > Are you working with @mcavage?  (See https://reviews.apache.org/r/43269/)
> > 
> > Also, you might want to add this ticket to your reviews: 
> > https://issues.apache.org/jira/browse/MESOS-4610

That's correct. I'll update this with the ticket.

Based on your suggestions, I've split the change into 3 parts (I couldn't split 
the namespace and refactoring changes because they are too tightly coupled). 
I've also added master_contender and master_detector flags 
(https://reviews.apache.org/r/44288/) so users can specify the modules to use.


- Anurag


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


On March 2, 2016, 9:13 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 2, 2016, 9:13 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>



Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-02 Thread Joseph Wu

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



Are you working with @mcavage?  (See https://reviews.apache.org/r/43269/)

Also, you might want to add this ticket to your reviews: 
https://issues.apache.org/jira/browse/MESOS-4610

- Joseph Wu


On March 2, 2016, 1:13 p.m., Anurag Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44287/
> ---
> 
> (Updated March 2, 2016, 1:13 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MasterContender and MasterDetector abstract classes can be subclassed by
> user-provided classes to allow for arbitrary contender/detector 
> implementations.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp PRE-CREATION 
>   include/mesos/master/detector.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44287/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>



Review Request 44289: Added support for contender and detector modules.

2016-03-02 Thread Anurag Singh

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

Review request for mesos.


Repository: mesos


Description
---

Added support for contender and detector modules.


Diffs
-

  include/mesos/module/contender.hpp PRE-CREATION 
  include/mesos/module/detector.hpp PRE-CREATION 
  src/Makefile.am fe549a881e778e094d3f957e5119eeb2f55e7915 
  src/examples/test_contender_module.cpp PRE-CREATION 
  src/examples/test_detector_module.cpp PRE-CREATION 
  src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
  src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/module/manager.cpp 6ae99504005581b22a44768949b1d305cec517d9 
  src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
  src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 

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


Testing
---

In addition to all unit tests passing, we are currently using this 
functionality in our environment with a custom consensus stack. In our world, 
we have a C++ plugin that calls out to an HTTP REST service (implemented in 
Java/Scala, not that it matters).


Thanks,

Anurag Singh



Review Request 44287: Added MasterContender and MasterDetector abstract classes.

2016-03-02 Thread Anurag Singh

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

Review request for mesos.


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh



Review Request 44288: Changed MasterDetector/Contender namespace.

2016-03-02 Thread Anurag Singh

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

Review request for mesos.


Repository: mesos


Description
---

Also modified users of MasterContender and MasterDetector to use this
namespace.


Diffs
-

  include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
  include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
  src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
  src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
  src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
  src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
  src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
  src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
  src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
  src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
  src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
  src/tests/fault_tolerance_tests.cpp f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
29c89fb11da792c3e71eb880a19657ea225b3cc8 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp 36595772b34bcb8d37dbc74d247bdf4614f10150 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_tests.cpp 
bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
  src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
  src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
428e12646d80b45daec30cfe607b97f36170fdf5 
  src/tests/scheduler_tests.cpp 2b1693eaf1a6106f5e7d269e4e3f6c353dd6f017 
  src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
  src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 

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


Testing
---

See https://reviews.apache.org/r/44289/.


Thanks,

Anurag Singh



Re: Review Request 44275: Fixed flakiness in tests using the scheduler library.

2016-03-02 Thread Anand Mazumdar

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

(Updated March 2, 2016, 9:08 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Make review bot run again.


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


Repository: mesos


Description
---

This change fixes the tests using the scheduler library by
explicitly invoking `stop()` from the testing interface. This
ensures that no further callbacks are delivered to the scheduler.

For one-off async callbacks that are already on the libprocess queue
we need to do a `Clock::settle` to ensure they are executed before
the mock object goes out of scope.


Diffs (updated)
-

  src/tests/mesos.hpp 1d4f075e470a60040e17b9f011aea6202310c437 

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


Testing
---

make check + Induced a sleep in the async callbacks. Previously, the test used 
to crash but with the fix it no longer crashes.


Thanks,

Anand Mazumdar



Review Request 44290: Added tests involving HTTP scheduler stream IDs.

2016-03-02 Thread Greg Mann

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Added tests involving HTTP scheduler stream IDs.

Three new tests have been added in this patch: 
SchedulerHttpApiTest.TeardownWithoutStreamId, 
SchedulerHttpApiTest.TeardownWrongStreamId, and 
SchedulerHttpApiTest.SubscribeWithStreamId.


Diffs
-

  src/tests/scheduler_http_api_tests.cpp 
428e12646d80b45daec30cfe607b97f36170fdf5 

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


Testing
---

`make check` was used to test on both OSX and CentOS 7.1

The new tests were run 1000 times to look for flakiness; no failures were 
observed.


Thanks,

Greg Mann



Re: Review Request 44279: Added stream IDs to the HTTP API docs.

2016-03-02 Thread Greg Mann

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

(Updated March 2, 2016, 8:59 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Added stream IDs to the HTTP API docs.


Diffs (updated)
-

  CHANGELOG e6cc39b4f45317f94145f0a4a1b64215b9a0cbff 
  docs/scheduler-http-api.md 53b482a813fa3909e786791d5331ef11c8fcc662 

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


Testing
---


Thanks,

Greg Mann



Re: Review Request 44278: Added HTTP scheduler stream IDs.

2016-03-02 Thread Greg Mann

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

(Updated March 2, 2016, 8:58 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Moved new tests into a different patch.


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


Repository: mesos


Description (updated)
---

Added HTTP scheduler stream IDs.

In some failure scenarios involving highly-available HTTP schedulers with 
multiple instances, it's possible for a non-leading instance to successfully 
make HTTP calls to the master. This patch enables the master to use HTTP 
scheduler stream IDs to uniquely identify each HTTP subscription stream, 
preventing any non-leading scheduler instance from making calls to the master. 
The patch also adds stream ID support to the HTTP scheduler library.


Diffs (updated)
-

  src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
  src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
  src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 

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


Testing (updated)
---

`make check` was used to test on both OSX and CentOS 7.1.


Thanks,

Greg Mann



Re: Review Request 44286: Unified/Added Future checks for http methods in tests.

2016-03-02 Thread Joerg Schad

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

(Updated March 2, 2016, 8:53 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


Repository: mesos


Description
---

Unified/Added Future checks for http methods in tests.


Diffs
-

  src/tests/fault_tolerance_tests.cpp 982468f851cd9d95eb6cde7c57f2d737d46a827c 
  src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
  src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 

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


Testing
---

sudo make check


Thanks,

Joerg Schad



Re: Review Request 44273: Modified scheduler tests to use the callback interface.

2016-03-02 Thread Anand Mazumdar

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

(Updated March 2, 2016, 8:28 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This change modifies the existing scheduler tests to use the new callback
interface introduced in MESOS-3339.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp 2b1693eaf1a6106f5e7d269e4e3f6c353dd6f017 

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


Testing
---

make check (gtest_repeat=100)

I did not yet modify the maintenance tests due to a bug I found while
modifying the tests. Filed MESOS-4831.


Thanks,

Anand Mazumdar



Re: Review Request 44269: Added the framework of 'network/cni' isolator.

2016-03-02 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [44200, 44269]

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 2, 2016, 3:25 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 2, 2016, 3:25 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 9d29fad396cdac0f263c5d52460030630fc2dfaf 
>   src/Makefile.am fe549a881e778e094d3f957e5119eeb2f55e7915 
>   src/slave/containerizer/mesos/containerizer.cpp 
> af3ff5750649497d8852b4761c78d4cae5455a02 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44269/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation

2016-03-02 Thread Greg Mann

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



Thanks Fan! I'm having a look at this now. Regarding the problems with review 
board not wanting to update your previous RR, it's probably because it didn't 
find the review URL in the commit message. In order to determine which RR it 
should update, the script looks for a string like "Review: 
https://reviews.apache.org/r/44255/; in the commit message of the commit you're 
pushing to review board. If you make sure that string (with the correct URL) is 
in your commit message locally, review board should be able to find your 
previous review and update it.

- Greg Mann


On March 2, 2016, 5:18 a.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44255/
> ---
> 
> (Updated March 2, 2016, 5:18 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Guangya Liu, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-4492
> https://issues.apache.org/jira/browse/MESOS-4492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Fan Du 
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/master.cpp 8d6d3c6468c6b85fe09c33cf9747cc3d1f515ab9 
>   src/master/metrics.hpp 9d201fcce1c46a890c86a889ab31029f9a061561 
>   src/master/metrics.cpp 30c091198a8fdd6d6a957a351dc37d3dae7788e4 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
> 
> Diff: https://reviews.apache.org/r/44255/diff/
> 
> 
> Testing
> ---
> 
> ChangLog:
> v2:
>   - Documenting those metrics
>   - Add test code for MetricsTest as suggested by Guangya
>   - post-review.py does not update original 
> RR(https://reviews.apache.org/r/44058/), but only create a new one even if I 
> rebased.
> 
> 
> Tests:
> 1. make check GTEST_FILTER="MetricsTest.Master" on Centos-7 
> (3.10.0-123.el7.x86_640)
> 
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from MetricsTest
> [ RUN  ] MetricsTest.Master
> [   OK ] MetricsTest.Master (211 ms)
> [--] 1 test from MetricsTest (211 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (244 ms total)
> [  PASSED  ] 1 test
> 
> 2. Verify its functionality with 'reserve' http endpoint as an test case
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 0.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> # curl -i -d slaveId=6250553a-2f39-4a92-9073-4618d130f433-S1  -d resources='[ 
> { "name": "cpus", "type": "SCALAR","scalar": { "value": 1 
> },"reservation":{"principal": "XiaoHaHa"}}  ]' -X POST  
> ipdc02-kvm-guest2:5050/master/reserve
> HTTP/1.1 200 OK
> Date: Fri, 26 Feb 2016 19:59:01 GMT
> Content-Length: 0
> 
> # curl http://ipdc02-kvm-guest2:5050/metrics/snapshot  | python -mjson.tool | 
> grep reserve
> "master/messages_reserve_resource": 1.0,
> "master/messages_unreserve_resource": 0.0,
> 
> 
> Thanks,
> 
> fan du
> 
>



Re: Review Request 44279: Added stream IDs to the HTTP API docs.

2016-03-02 Thread Greg Mann

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

(Updated March 2, 2016, 7:30 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Edited changelog.


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


Repository: mesos


Description
---

Added stream IDs to the HTTP API docs.


Diffs (updated)
-

  CHANGELOG e6cc39b4f45317f94145f0a4a1b64215b9a0cbff 
  docs/scheduler-http-api.md 53b482a813fa3909e786791d5331ef11c8fcc662 

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


Testing
---


Thanks,

Greg Mann



Re: Review Request 44220: Updated the CHANGELOG and doc about 'subscribe.force' field removal.

2016-03-02 Thread Greg Mann

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




docs/scheduler-http-api.md (line 117)


s/with `SUBSCRIBED` response/with a `SUBSCRIBED` response/



docs/scheduler-http-api.md (line 508)


s/sending `SUBSCRIBE` request/sending a `SUBSCRIBE` request/

s/framework id/framework ID/



docs/scheduler-http-api.md (line 511)


s/is that, only/is that only/

s/FrameworkID/framework ID/



docs/scheduler-http-api.md (line 515)


s/similar in vein/similar/

s/upto/up to/


- Greg Mann


On March 2, 2016, 1:11 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44220/
> ---
> 
> (Updated March 2, 2016, 1:11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-4712
> https://issues.apache.org/jira/browse/MESOS-4712
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Marked this as an API change in the CHANGELOG and removed references of this
> field in the user doc. Also, updated the semantics for schedulers that
> resubscribe.
> 
> 
> Diffs
> -
> 
>   CHANGELOG e6cc39b4f45317f94145f0a4a1b64215b9a0cbff 
>   docs/scheduler-http-api.md 53b482a813fa3909e786791d5331ef11c8fcc662 
> 
> Diff: https://reviews.apache.org/r/44220/diff/
> 
> 
> Testing
> ---
> 
> Rendered the markdown locally.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 44243: Updated master to ignore 'Subscribe.force' for HTTP framework.

2016-03-02 Thread Greg Mann

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


Ship it!




Ship It!

- Greg Mann


On March 2, 2016, 1:10 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44243/
> ---
> 
> (Updated March 2, 2016, 1:10 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Greg Mann.
> 
> 
> Bugs: MESOS-4712
> https://issues.apache.org/jira/browse/MESOS-4712
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master always closes any old active streaming connection when it gets a
> SUBSCRIBE request.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
> 
> Diff: https://reviews.apache.org/r/44243/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>



Re: Review Request 44251: Improved allocator perf for labeled reservations and volumes.

2016-03-02 Thread Neil Conway

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

(Updated March 2, 2016, 7:17 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Tweak variable name.


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


Repository: mesos


Description
---

When the cluster contains many resources that have either labeled
reservations or persistent volumes, allocator performance can decrease
substantially because such metadata prevents `Resource` objects from
being merged together inside the allocator. As a result, the allocator
must manipulate `Resources` vectors that consist of many small
individual `Resource` values; since many `Resources` operations take
linear-time in the number of `Resource` values they contain, this can
cause very significant slowdowns.

As a short-term solution, this commit strips dynamic reservation and
persistent volume information from the `Resources` objects used
internally by the allocator, because they are not needed when
aggregating resource quantities together.

A long-term solution for this problem will be addressed as work on
refactoring the allocator more generally.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
24fa50f62dec10ed43089297473cc386d6ba2f78 
  src/master/allocator/sorter/drf/sorter.hpp 
46b2a9caf13b028a3aee6c1590679f885be90fd6 
  src/master/allocator/sorter/drf/sorter.cpp 
9e863dd0ca5e2f2f0d517cb833687b757bed2c52 
  src/master/allocator/sorter/sorter.hpp 
ba91a38e47065718af87c9b3b7c5b74d25a258df 

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


Testing
---

make check

Perf:

```
[ RUN  ] HierarchicalAllocator_BENCHMARK_Test.ResourceLabels
Using 2000 slaves and 200 frameworks
round 0 allocate took 2.458462secs to make 200 offers
round 1 allocate took 2.427941secs to make 200 offers
round 2 allocate took 2.460724secs to make 200 offers
round 3 allocate took 2.443408secs to make 200 offers
round 4 allocate took 2.464784secs to make 200 offers
round 5 allocate took 2.501429secs to make 200 offers
round 6 allocate took 2.468777secs to make 200 offers
round 7 allocate took 2.482268secs to make 200 offers
round 8 allocate took 2.479014secs to make 200 offers
round 9 allocate took 2.529951secs to make 200 offers
round 10 allocate took 2.460059secs to make 200 offers
```

Performance of `DeclineOffers` without labels is about ~2.1 seconds.


Thanks,

Neil Conway



Re: Review Request 44251: Improved allocator perf for labeled reservations and volumes.

2016-03-02 Thread Neil Conway


> On March 2, 2016, 1:11 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1421
> > 
> >
> > One suggestion is that does it make sense to update all variables 
> > including `scalar` to `strippedScalar` which might be more accurate.

Thanks -- I decided to go with `scalarQuantity`.


- Neil


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


On March 2, 2016, 7:15 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44251/
> ---
> 
> (Updated March 2, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4833
> https://issues.apache.org/jira/browse/MESOS-4833
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the cluster contains many resources that have either labeled
> reservations or persistent volumes, allocator performance can decrease
> substantially because such metadata prevents `Resource` objects from
> being merged together inside the allocator. As a result, the allocator
> must manipulate `Resources` vectors that consist of many small
> individual `Resource` values; since many `Resources` operations take
> linear-time in the number of `Resource` values they contain, this can
> cause very significant slowdowns.
> 
> As a short-term solution, this commit strips dynamic reservation and
> persistent volume information from the `Resources` objects used
> internally by the allocator, because they are not needed when
> aggregating resource quantities together.
> 
> A long-term solution for this problem will be addressed as work on
> refactoring the allocator more generally.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 24fa50f62dec10ed43089297473cc386d6ba2f78 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 46b2a9caf13b028a3aee6c1590679f885be90fd6 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 9e863dd0ca5e2f2f0d517cb833687b757bed2c52 
>   src/master/allocator/sorter/sorter.hpp 
> ba91a38e47065718af87c9b3b7c5b74d25a258df 
> 
> Diff: https://reviews.apache.org/r/44251/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Perf:
> 
> ```
> [ RUN  ] HierarchicalAllocator_BENCHMARK_Test.ResourceLabels
> Using 2000 slaves and 200 frameworks
> round 0 allocate took 2.458462secs to make 200 offers
> round 1 allocate took 2.427941secs to make 200 offers
> round 2 allocate took 2.460724secs to make 200 offers
> round 3 allocate took 2.443408secs to make 200 offers
> round 4 allocate took 2.464784secs to make 200 offers
> round 5 allocate took 2.501429secs to make 200 offers
> round 6 allocate took 2.468777secs to make 200 offers
> round 7 allocate took 2.482268secs to make 200 offers
> round 8 allocate took 2.479014secs to make 200 offers
> round 9 allocate took 2.529951secs to make 200 offers
> round 10 allocate took 2.460059secs to make 200 offers
> ```
> 
> Performance of `DeclineOffers` without labels is about ~2.1 seconds.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 44251: Improved allocator perf for labeled reservations and volumes.

2016-03-02 Thread Neil Conway

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

(Updated March 2, 2016, 7:15 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Tweak variable name per Guangya.


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


Repository: mesos


Description
---

When the cluster contains many resources that have either labeled
reservations or persistent volumes, allocator performance can decrease
substantially because such metadata prevents `Resource` objects from
being merged together inside the allocator. As a result, the allocator
must manipulate `Resources` vectors that consist of many small
individual `Resource` values; since many `Resources` operations take
linear-time in the number of `Resource` values they contain, this can
cause very significant slowdowns.

As a short-term solution, this commit strips dynamic reservation and
persistent volume information from the `Resources` objects used
internally by the allocator, because they are not needed when
aggregating resource quantities together.

A long-term solution for this problem will be addressed as work on
refactoring the allocator more generally.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
24fa50f62dec10ed43089297473cc386d6ba2f78 
  src/master/allocator/sorter/drf/sorter.hpp 
46b2a9caf13b028a3aee6c1590679f885be90fd6 
  src/master/allocator/sorter/drf/sorter.cpp 
9e863dd0ca5e2f2f0d517cb833687b757bed2c52 
  src/master/allocator/sorter/sorter.hpp 
ba91a38e47065718af87c9b3b7c5b74d25a258df 

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


Testing
---

make check

Perf:

```
[ RUN  ] HierarchicalAllocator_BENCHMARK_Test.ResourceLabels
Using 2000 slaves and 200 frameworks
round 0 allocate took 2.458462secs to make 200 offers
round 1 allocate took 2.427941secs to make 200 offers
round 2 allocate took 2.460724secs to make 200 offers
round 3 allocate took 2.443408secs to make 200 offers
round 4 allocate took 2.464784secs to make 200 offers
round 5 allocate took 2.501429secs to make 200 offers
round 6 allocate took 2.468777secs to make 200 offers
round 7 allocate took 2.482268secs to make 200 offers
round 8 allocate took 2.479014secs to make 200 offers
round 9 allocate took 2.529951secs to make 200 offers
round 10 allocate took 2.460059secs to make 200 offers
```

Performance of `DeclineOffers` without labels is about ~2.1 seconds.


Thanks,

Neil Conway



Re: Review Request 44278: Added HTTP scheduler stream IDs.

2016-03-02 Thread Greg Mann

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

(Updated March 2, 2016, 7:14 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Added HTTP scheduler stream IDs.

In some failure scenarios involving highly-available HTTP schedulers with 
multiple instances, it's possible for a non-leading instance to successfully 
make HTTP calls to the master. This patch enables the master to use HTTP 
scheduler stream IDs to uniquely identify each HTTP subscription stream, 
preventing any non-leading scheduler instance from making calls to the master. 
The patch also adds stream ID support to the HTTP scheduler library.

Three new tests have been added in this patch: 
SchedulerHttpApiTest.TeardownWithoutStreamId, 
SchedulerHttpApiTest.TeardownWrongStreamId, 
SchedulerHttpApiTest.SubscribeWithStreamId


Diffs
-

  src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
  src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
  src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
  src/tests/scheduler_http_api_tests.cpp 
428e12646d80b45daec30cfe607b97f36170fdf5 

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


Testing (updated)
---

New tests were added, and `make check` was used to test on both OSX and CentOS 
7.1.

The new tests were run 1000 times to check for flakiness; no failures were 
observed.


Thanks,

Greg Mann



Review Request 44278: Added HTTP scheduler stream IDs.

2016-03-02 Thread Greg Mann

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Added HTTP scheduler stream IDs.

In some failure scenarios involving highly-available HTTP schedulers with 
multiple instances, it's possible for a non-leading instance to successfully 
make HTTP calls to the master. This patch enables the master to use HTTP 
scheduler stream IDs to uniquely identify each HTTP subscription stream, 
preventing any non-leading scheduler instance from making calls to the master. 
The patch also adds stream ID support to the HTTP scheduler library.

Three new tests have been added in this patch: 
SchedulerHttpApiTest.TeardownWithoutStreamId, 
SchedulerHttpApiTest.TeardownWrongStreamId, 
SchedulerHttpApiTest.SubscribeWithStreamId


Diffs
-

  src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
  src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
  src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
  src/tests/scheduler_http_api_tests.cpp 
428e12646d80b45daec30cfe607b97f36170fdf5 

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


Testing
---

New tests were added, and `make check` was used to test on OSX.

The new tests were run 1000 times to check for flakiness; no failures were 
observed.


Thanks,

Greg Mann



Re: Review Request 43884: Added allocator metrics for used quotas.

2016-03-02 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [44070, 44071, 44073, 44260, 43879, 43880, 43881, 44261, 
43882, 43883, 43884]

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 2, 2016, 3:43 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43884/
> ---
> 
> (Updated March 2, 2016, 3:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4723
> https://issues.apache.org/jira/browse/MESOS-4723
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 24fa50f62dec10ed43089297473cc386d6ba2f78 
>   src/master/allocator/mesos/metrics.hpp PRE-CREATION 
>   src/master/allocator/mesos/metrics.cpp PRE-CREATION 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43884/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Review Request 44280: Removed `FLAGS_v` assignment in test case.

2016-03-02 Thread Neil Conway

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

Per discussion with James Peach, this doesn't serve a
useful purpose.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 

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


Testing
---

make check


Thanks,

Neil Conway



Re: Review Request 44250: Added `Resources::createStrippedScalarQuantity()`.

2016-03-02 Thread Neil Conway

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

(Updated March 2, 2016, 6:27 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Tweak unit tests.


Repository: mesos


Description
---

This returns a new `Resources` object that omits dynamic reservation
and persistent volume information. This is intended primarily for
situations in which code wants to efficiently compute aggregate
statistics about many `Resource` values for which reservation and
persistent volume information is not relevant.


Diffs (updated)
-

  include/mesos/resources.hpp fe8a5745ea7d4943c47ac22c73db70488c6dfa9f 
  include/mesos/v1/resources.hpp c27927e4f0d7f45e69fe3312b2423afb64c5c51e 
  src/common/resources.cpp 4fa1e78606485d6657d3776e28b78a43cc6449d2 
  src/tests/resources_tests.cpp e7525a00957e903993f4dd4b73e05c86f84c5c29 
  src/v1/resources.cpp bca523159577994d5890f832e4f61101b5dbf3bc 

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


Testing
---

make check


Thanks,

Neil Conway



Review Request 44275: Fixed flakiness in tests using the scheduler library.

2016-03-02 Thread Anand Mazumdar

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change fixes the tests using the scheduler library by
explicitly invoking `stop()` from the testing interface. This
ensures that no further callbacks are delivered to the scheduler.

For one-off async callbacks that are already on the libprocess queue
we need to do a `Clock::settle` to ensure they are executed before
the mock object goes out of scope.


Diffs
-

  src/tests/mesos.hpp 0c55d5901753dc3f6e71486f727a2af7bd920429 

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


Testing
---

make check + Induced a sleep in the async callbacks. Previously, the test used 
to crash but with the fix it no longer crashes.


Thanks,

Anand Mazumdar



Review Request 44274: Added the ability to stop running the scheduler library process.

2016-03-02 Thread Anand Mazumdar

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change adds the ability to stop running the scheduler library
process so that no future callbacks are delivered to the scheduler.

This helps us during testing to ensure no further callbacks happen
to stack allocated mock objects.


Diffs
-

  include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
  src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 

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


Testing
---

make check. This is used later in the chain when we move the interface to use 
the `stop()` function.


Thanks,

Anand Mazumdar



Review Request 44273: Modified scheduler tests to use the callback interface.

2016-03-02 Thread Anand Mazumdar

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change modifies the existing scheduler tests to use the new callback
interface introduced in MESOS-3339.


Diffs
-

  src/tests/scheduler_tests.cpp 70c5b218aa231b21277580567d92f31c16a95efb 

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


Testing
---

make check (gtest_repeat=100)

I did not yet modify the maintenance tests due to a bug I found while
modifying the tests. Filed MESOS-4831.


Thanks,

Anand Mazumdar



Re: Review Request 43614: Refactor MesosTest and remove cleanup logic.

2016-03-02 Thread Joseph Wu


> On March 2, 2016, 7:28 a.m., Bernd Mathiske wrote:
> > src/tests/mesos.hpp, line 135
> > 
> >
> > So this injection is allowed to be a shared_ptr and the others are not? 
> > This makes me wonder if it would not be simpler if all injections were 
> > shared_ptr, even the ones that are sometimes Owned and sometimes not (i.e. 
> > declared with a parallel member of type *). Then we would only need one 
> > kind for all cases and a lot of code would look substantially simpler. The 
> > only Owned ones would be those that are always Owned.

I'll add a note above this one.  The reason this is a `shared_ptr` is because 
the `Master` expects a `shared_ptr` for this one.
Also see: https://reviews.apache.org/r/43613/#comment181352


- Joseph


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


On Feb. 16, 2016, 2:15 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43614/
> ---
> 
> (Updated Feb. 16, 2016, 2:15 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updates `StartMaster` and `StartSlave` test helpers to use the reworked 
> `cluster` helpers.  Removes all `MesosTest` cleanup logic and as well as the 
> helpers that accept a `MockExecutor` pointer.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43614/diff/
> 
> 
> Testing
> ---
> 
> Tests are run at the end of this review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 44260: Moved metrics of the hierarchical allocator to its own file.

2016-03-02 Thread Benjamin Bannier

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

(Updated March 2, 2016, 6:49 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/Makefile.am 5d8fe8bf33acb352589b2b3e0341987f9a41bc17 
  src/master/allocator/mesos/hierarchical.hpp 
3043888630b066505410d3b32c5b3f813cc458c1 
  src/master/allocator/mesos/hierarchical.cpp 
24fa50f62dec10ed43089297473cc386d6ba2f78 
  src/master/allocator/mesos/metrics.hpp PRE-CREATION 
  src/master/allocator/mesos/metrics.cpp PRE-CREATION 

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


Testing (updated)
---

`make distcheck` on OS X.


Thanks,

Benjamin Bannier



Re: Review Request 44070: Allowed disabling metrics endpoint rate limiting via the environment.

2016-03-02 Thread Benjamin Bannier

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

(Updated March 2, 2016, 6:48 p.m.)


Review request for mesos, Alexander Rukletsov and Ben Mahler.


Changes
---

Addressed offline comments from bmahler.


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


Repository: mesos


Description
---

Allowed disabling metrics endpoint rate limiting via the environment.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
09b716be56eac38f75d79d917799c001aa0b205c 
  3rdparty/libprocess/src/metrics/metrics.cpp 
a9840083722dd6b7b6aab692ed449407ab125ac7 

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


Testing
---

make check (OS X, not optimized)


Thanks,

Benjamin Bannier



Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

2016-03-02 Thread Mesos ReviewBot

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



Bad patch!

Reviews applied: [43615, 43630, 43629, 43614, 43613]

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

Error:
2016-03-02 17:13:08 URL:https://reviews.apache.org/r/43615/diff/raw/ 
[778972/778972] -> "43615.patch" [1]
error: patch failed: src/tests/fault_tolerance_tests.cpp:1904
error: src/tests/fault_tolerance_tests.cpp: patch does not apply
error: patch failed: src/tests/master_tests.cpp:3254
error: src/tests/master_tests.cpp: patch does not apply
error: patch failed: src/tests/slave_tests.cpp:2328
error: src/tests/slave_tests.cpp: patch does not apply

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

- Mesos ReviewBot


On Feb. 29, 2016, 9:55 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43615/
> ---
> 
> (Updated Feb. 29, 2016, 9:55 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Includes the following changes:
> 
> * Added the `` header where appropriate.
> * Added the namespace `using process::Owned;` where appropriate.
> * Generally replaced `Try` with `Owned`.  And 
> `Try` with `Owned`.
> * Added the (now required) `MasterDetector` argument to all slaves.  Before, 
> this was fetched from the first master in `Cluster`.
> * Removed `Shutdown();` from all tests.
> * Replaced `Stop(...)` with the appropriate master/slave destruction calls.
> * Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
> launchers, etc).
> * Replace `CHECK` in tests with `ASSERT`.
> 
> 
> Diffs
> -
> 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/command_executor_tests.cpp 
> 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
>   src/tests/container_logger_tests.cpp 
> 00f4129e46aa9268fbb66da25b34e61004fa87b2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6aecd912fc84b72d2b64f7a842891fddcbc469ac 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 6a60962b4593b3521c182c7320331743ccffd4ba 
>   src/tests/containerizer/isolator_tests.cpp 
> 7b257de2afbc66f63c47a80c1f828e3e95bd602d 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 79f134996c4b80bf49cbb8bee28eab5e6b4f5822 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 983a6be160aefe5a32acb6111bb3c85230ec 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 5b685bfd842d0d98e8ea5ec5ddea8d2cd893dd81 
>   src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
>   src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
>   src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
>   src/tests/executor_http_api_tests.cpp 
> 2fc0893f5f5e80a783296fb31b30abe86d92df1b 
>   src/tests/fault_tolerance_tests.cpp 
> 982468f851cd9d95eb6cde7c57f2d737d46a827c 
>   src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/hook_tests.cpp 88ad7806bf9f3194e434743c35199a896edea92c 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_maintenance_tests.cpp 
> 3faa8136cf57276295553910319480028f433e4c 
>   src/tests/master_quota_tests.cpp 8357ec911b2a158632a708ae3adff6eabc536697 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
>   src/tests/master_validation_tests.cpp 
> c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
>   src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 
>   src/tests/oversubscription_tests.cpp 
> e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
>   src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 81185a161498394020a27f1f5bf747bac5425f43 
>   src/tests/persistent_volume_tests.cpp 
> 

  1   2   >