Re: Review Request 41299: Allow DockerContainerizer log to console.

2015-12-12 Thread Timothy Chen

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

Ship it!


Ship It!


src/slave/containerizer/docker.cpp (line 835)


Default is None already.



src/slave/containerizer/docker.cpp (line 838)


Space in between here please.


- Timothy Chen


On Dec. 12, 2015, 9 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41299/
> ---
> 
> (Updated Dec. 12, 2015, 9 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: MESOS-4141
> https://issues.apache.org/jira/browse/MESOS-4141
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow DockerContainerizer log to console.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> dcdf98fea4ca6f96658886db5d09c99f3bff501d 
>   src/slave/containerizer/docker.hpp 35712f599395b5f5fbc311a37c6e29b33bac0c8e 
>   src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 3f199e622dd337cdbf32d4368f4ead424c39823c 
>   src/tests/health_check_tests.cpp 0fbccc373204d3b9431c614bdd6d046cc07e4566 
>   src/tests/hook_tests.cpp e034534295fe6ddd280e25f1d9ee5182cddae187 
>   src/tests/mesos.hpp 2429ac5cbcd9c1a3949c11de94b542108a3c13d8 
>   src/tests/mesos.cpp a501f04dfbf9c93c2196e83b512519e925a67c97 
> 
> Diff: https://reviews.apache.org/r/41299/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 41299: Allow DockerContainerizer log to console.

2015-12-12 Thread haosdent huang

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

(Updated Dec. 13, 2015, 5:35 a.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

Address comments


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


Repository: mesos


Description
---

Allow DockerContainerizer log to console.


Diffs (updated)
-

  src/slave/containerizer/containerizer.cpp 
dcdf98fea4ca6f96658886db5d09c99f3bff501d 
  src/slave/containerizer/docker.hpp 35712f599395b5f5fbc311a37c6e29b33bac0c8e 
  src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
  src/tests/containerizer/docker_containerizer_tests.cpp 
3f199e622dd337cdbf32d4368f4ead424c39823c 
  src/tests/health_check_tests.cpp 0fbccc373204d3b9431c614bdd6d046cc07e4566 
  src/tests/hook_tests.cpp e034534295fe6ddd280e25f1d9ee5182cddae187 
  src/tests/mesos.hpp 2429ac5cbcd9c1a3949c11de94b542108a3c13d8 
  src/tests/mesos.cpp a501f04dfbf9c93c2196e83b512519e925a67c97 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

2015-12-12 Thread Klaus Ma

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

(Updated Dec. 12, 2015, 6:12 p.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van Remoortere, 
and Joseph Wu.


Changes
---

Address comments


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


Repository: mesos


Description
---

In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps to 
update `RevocableInfo::type` for Oversubscription.


Diffs (updated)
-

  include/mesos/slave/resource_estimator.hpp 27612ca 
  src/slave/slave.cpp 9bd86e1 

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


Testing
---

make (make check is on-going)


Thanks,

Klaus Ma



Re: Review Request 41212: Adjust timeout value in HealthCheckTest.CheckCommandTimeout.

2015-12-12 Thread haosdent huang

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

(Updated Dec. 12, 2015, 8:11 a.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

Make MesosContainerizer log to stdout and stderr.


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


Repository: mesos


Description
---

Adjust timeout value in HealthCheckTest.CheckCommandTimeout.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 0fbccc373204d3b9431c614bdd6d046cc07e4566 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 38172: Stout: Simplified hashset implementation.

2015-12-12 Thread Michael Park

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



3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp (lines 46 - 49)


Why not just `std::unordered_set(set.begin(), 
set.end())`? i.e. I don't see why `set.size()` should be the `bucket_size`.



3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp (lines 69 - 71)


Same question as the `std::set` constructor. Why do we want `list.size()` 
to be the `bucket_size`?



3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp (lines 73 - 83)


Let's not add this constructor. I don't think it adds much value (if at 
all) over `std::initializer_list`.


- Michael Park


On Sept. 8, 2015, 9:52 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38172/
> ---
> 
> (Updated Sept. 8, 2015, 9:52 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3217
> https://issues.apache.org/jira/browse/MESOS-3217
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/hashset.hpp 
> 1839d28638cd82dae10ba9b0f99c1a97cf34f9c9 
> 
> Diff: https://reviews.apache.org/r/38172/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Review Request 41299: Allow DockerContainerizer log to console.

2015-12-12 Thread haosdent huang

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

Review request for mesos, Ben Mahler and Timothy Chen.


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


Repository: mesos


Description
---

Allow DockerContainerizer log to console.


Diffs
-

  src/slave/containerizer/containerizer.cpp 
dcdf98fea4ca6f96658886db5d09c99f3bff501d 
  src/slave/containerizer/docker.hpp 35712f599395b5f5fbc311a37c6e29b33bac0c8e 
  src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
  src/tests/containerizer/docker_containerizer_tests.cpp 
3f199e622dd337cdbf32d4368f4ead424c39823c 
  src/tests/health_check_tests.cpp 0fbccc373204d3b9431c614bdd6d046cc07e4566 
  src/tests/hook_tests.cpp e034534295fe6ddd280e25f1d9ee5182cddae187 
  src/tests/mesos.hpp 2429ac5cbcd9c1a3949c11de94b542108a3c13d8 
  src/tests/mesos.cpp a501f04dfbf9c93c2196e83b512519e925a67c97 

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


Testing
---


Thanks,

haosdent huang



Review Request 41308: MESOS-1718: Unit Test for moving getExecutorInfo from slave to master

2015-12-12 Thread Klaus Ma

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

Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
---

MESOS-1718: Unit Test for moving getExecutorInfo from slave to master


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 3f199e6 
  src/tests/master_tests.cpp 865fa4a 
  src/tests/master_validation_tests.cpp fbf8fad 
  src/tests/monitor_tests.cpp a848d14 
  src/tests/reservation_endpoints_tests.cpp d5d2aa7 
  src/tests/slave_recovery_tests.cpp c0e4ff7 
  src/tests/slave_tests.cpp 4975bea 

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


Testing
---


Thanks,

Klaus Ma



Re: Review Request 40966: Corrected termination of Docker containers.

2015-12-12 Thread haosdent huang


> On Dec. 12, 2015, 8:36 a.m., haosdent huang wrote:
> > Ship It!

Thank you very much. I verify this in Ubuntu 14.04 with your test command. 
Before apply your patch could reproduce problem, after apply this patch could 
pass.


- haosdent


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


On Dec. 11, 2015, 11:15 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40966/
> ---
> 
> (Updated Dec. 11, 2015, 11:15 a.m.)
> 
> 
> Review request for mesos, Greg Mann, haosdent huang, Jojy Varghese, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4025
> https://issues.apache.org/jira/browse/MESOS-4025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tests cases have to wait until a container has been terminated by the
> DockerContainerizer. Otherwise there could be artifacts (e.g. locked cgroups)
> that can affect later test cases (see MESOS-4025, where cgroups couldn't be
> removed).
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 
> 
> Diff: https://reviews.apache.org/r/40966/diff/
> 
> 
> Testing
> ---
> 
> make check
> sudo ./bin/mesos-tests.sh --gtest_repeat=50 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_*:SlaveRecoveryTest*GCExecutor"
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>



Re: Review Request 38171: Stout: Refactored set to use initializer list for variadic constructor.

2015-12-12 Thread Michael Park

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


I would actually prefer we get rid of this class entirely.

1. I believe the variadic constructor for initialization doesn't give us much 
value over `std::initializer_list`.
2. This class isn't used at all in the entire codebase except for once instance 
of it in `src/log/log.cpp` which can simply be removed with:

```
-Set((UPID) replica->pid(,
+{static_cast(replica->pid())})),
```

since the member being initialized by this expression is `std::set` anyway.
After that, we just need to modify `set_tests.cpp` such that we test the added 
operators such as `operator|`.

- Michael Park


On Sept. 8, 2015, 6:23 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38171/
> ---
> 
> (Updated Sept. 8, 2015, 6:23 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3217
> https://issues.apache.org/jira/browse/MESOS-3217
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/set.hpp 
> 85466dbb36b3ac545562eafe8041ad79993fdf9f 
> 
> Diff: https://reviews.apache.org/r/38171/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Re: Review Request 38173: Stout: Simplified hashmap implementation.

2015-12-12 Thread Michael Park

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



3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp (lines 48 - 51)


Similar questions as in https://reviews.apache.org/r/38172/ regarding the 
specification of `bucket_size`. Here and below.


- Michael Park


On Sept. 8, 2015, 9:52 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38173/
> ---
> 
> (Updated Sept. 8, 2015, 9:52 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3217
> https://issues.apache.org/jira/browse/MESOS-3217
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp 
> f09bea125035aa3621402b83608b233e42877559 
> 
> Diff: https://reviews.apache.org/r/38173/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>



Review Request 41302: MESOS-1718: add slave's configuration into SlaveInfo

2015-12-12 Thread Klaus Ma

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

Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
---

MESOS-1718: add slave's configuration into SlaveInfo


Diffs
-

  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
  include/mesos/v1/mesos.proto 8f357b0fb778098ec66ac85d174bdd7e387954b5 

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


Testing
---


Thanks,

Klaus Ma



Re: Review Request 40966: Corrected termination of Docker containers.

2015-12-12 Thread haosdent huang

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

Ship it!


Ship It!

- haosdent huang


On Dec. 11, 2015, 11:15 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40966/
> ---
> 
> (Updated Dec. 11, 2015, 11:15 a.m.)
> 
> 
> Review request for mesos, Greg Mann, haosdent huang, Jojy Varghese, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4025
> https://issues.apache.org/jira/browse/MESOS-4025
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Tests cases have to wait until a container has been terminated by the
> DockerContainerizer. Otherwise there could be artifacts (e.g. locked cgroups)
> that can affect later test cases (see MESOS-4025, where cgroups couldn't be
> removed).
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp b1454b085b36bb7c4d8ef012c764cd8466b4fb02 
> 
> Diff: https://reviews.apache.org/r/40966/diff/
> 
> 
> Testing
> ---
> 
> make check
> sudo ./bin/mesos-tests.sh --gtest_repeat=50 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_*:SlaveRecoveryTest*GCExecutor"
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>



Re: Review Request 35711: Disallow special characters in role name.

2015-12-12 Thread haosdent huang


> On Nov. 5, 2015, 11:59 p.m., Neil Conway wrote:
> > include/mesos/roles.hpp, line 51
> > 
> >
> > Should this be a member function? i.e., not static.

When I write this, I refer to the static validate method in Resources. Is it 
better to make it as a member function?


> On Nov. 5, 2015, 11:59 p.m., Neil Conway wrote:
> > include/mesos/roles.hpp, line 53
> > 
> >
> > Is this necessary?

Because we have `Resources() {}`, I still keep it here.


> On Nov. 5, 2015, 11:59 p.m., Neil Conway wrote:
> > include/mesos/roles.hpp, line 59
> > 
> >
> > Is this necessary?

ditto


> On Nov. 5, 2015, 11:59 p.m., Neil Conway wrote:
> > include/mesos/roles.hpp, line 61
> > 
> >
> > Is this necessary?

ditto


> On Nov. 5, 2015, 11:59 p.m., Neil Conway wrote:
> > include/mesos/roles.hpp, line 63
> > 
> >
> > Is this necessary?

ditto


> On Nov. 5, 2015, 11:59 p.m., Neil Conway wrote:
> > src/common/roles.cpp, line 67
> > 
> >
> > Error message is inaccurate.

`inaccurate` means we should change `invalid` to `illegal`, or what description 
would be better here?


- haosdent


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


On Dec. 12, 2015, 2:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35711/
> ---
> 
> (Updated Dec. 12, 2015, 2:29 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-2210
> https://issues.apache.org/jira/browse/MESOS-2210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Disallow special characters in role name.
> 
> 
> Diffs
> -
> 
>   include/mesos/roles.hpp PRE-CREATION 
>   src/CMakeLists.txt 0d46043dd06007f2cba56626c82e8edd251cb8fb 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
>   src/common/roles.cpp PRE-CREATION 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
>   src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
>   src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
>   src/tests/resources_tests.cpp ce47bac73937a6a608016f46121137d6116c1399 
>   src/tests/roles_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35711/diff/
> 
> 
> Testing
> ---
> 
> make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 35711: Disallow special characters in role name.

2015-12-12 Thread haosdent huang

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

(Updated Dec. 12, 2015, 2:29 p.m.)


Review request for mesos, Adam B, Jie Yu, and Michael Park.


Changes
---

Address comments


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


Repository: mesos


Description
---

Disallow special characters in role name.


Diffs (updated)
-

  include/mesos/roles.hpp PRE-CREATION 
  src/CMakeLists.txt 0d46043dd06007f2cba56626c82e8edd251cb8fb 
  src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
  src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
  src/common/roles.cpp PRE-CREATION 
  src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
  src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
  src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
  src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
  src/tests/resources_tests.cpp ce47bac73937a6a608016f46121137d6116c1399 
  src/tests/roles_tests.cpp PRE-CREATION 

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


Testing
---

make -j8 check


Thanks,

haosdent huang



Re: Review Request 41248: Enabled master get USAGE_SLACK metrics.

2015-12-12 Thread Guangya Liu

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

(Updated 十二月 12, 2015, 4:43 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Enabled master get USAGE_SLACK metrics.


Diffs (updated)
-

  src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
  src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
  src/master/metrics.hpp 551e4eb0f910aa4ce28a2a955019108fcd3d6371 
  src/master/metrics.cpp 5e4f4d352579dcd391a133600f9d4abc6b2402a7 

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


Testing
---

$ curl "http://192.168.0.107:5050/metrics/snapshot; 2>/dev/null| jq . | grep 
usage_slack
  "master/cpus_usage_slack_revocable_percent": 0,
  "master/cpus_usage_slack_revocable_total": 0,
  "master/cpus_usage_slack_revocable_used": 0,
  "master/disk_usage_slack_revocable_percent": 0,
  "master/disk_usage_slack_revocable_total": 0,
  "master/disk_usage_slack_revocable_used": 0,
  "master/mem_usage_slack_revocable_percent": 0,
  "master/mem_usage_slack_revocable_total": 0,
  "master/mem_usage_slack_revocable_used": 0,


Thanks,

Guangya Liu



Re: Review Request 41246: Enabled slave get USAGE_SLACK metrics.

2015-12-12 Thread Guangya Liu

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

(Updated 十二月 12, 2015, 4:43 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Enabled slave get USAGE_SLACK metrics.


Diffs (updated)
-

  src/slave/metrics.hpp 2c22de521309806d70f9bd124359756b5e4c75b4 
  src/slave/metrics.cpp 1b73121264de3ff6eabc8f620e5b4d5930ba5d43 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 9bd86e122c070cd072a54d4de8097f844bd95bb0 

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


Testing
---

$ curl "http://192.168.0.107:5051/metrics/snapshot; 2>/dev/null| jq . | grep 
usage_slack
  "slave/cpus_usage_slack_revocable_percent": 0,
  "slave/cpus_usage_slack_revocable_total": 0,
  "slave/cpus_usage_slack_revocable_used": 0,
  "slave/disk_usage_slack_revocable_percent": 0,
  "slave/disk_usage_slack_revocable_total": 0,
  "slave/disk_usage_slack_revocable_used": 0,
  "slave/mem_usage_slack_revocable_percent": 0,
  "slave/mem_usage_slack_revocable_total": 0,
  "slave/mem_usage_slack_revocable_used": 0,


Thanks,

Guangya Liu



Re: Review Request 41254: Enable master get ALLOCATION_SLACK metrics.

2015-12-12 Thread Guangya Liu

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

(Updated 十二月 12, 2015, 4:43 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Enable master get ALLOCATION_SLACK metrics.


Diffs (updated)
-

  src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
  src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
  src/master/metrics.hpp 551e4eb0f910aa4ce28a2a955019108fcd3d6371 
  src/master/metrics.cpp 5e4f4d352579dcd391a133600f9d4abc6b2402a7 

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


Testing
---

$ curl "http://192.168.0.107:5050/metrics/snapshot; 2>/dev/null| jq . | grep 
alloc_slack
  "master/cpus_alloc_slack_revocable_total": 3,
  "master/cpus_alloc_slack_revocable_used": 0,
  "master/disk_alloc_slack_revocable_total": 0,
  "master/disk_alloc_slack_revocable_used": 0,
  "master/mem_alloc_slack_revocable_total": 1000,
  "master/mem_alloc_slack_revocable_used": 0,


Thanks,

Guangya Liu



Review Request 41314: Fixed hang introduced in committed version of HTTP AuthenticatorRouter

2015-12-12 Thread Alexander Rojas

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

Review request for mesos, Ben Mahler, Michael Park, and Till Toenshoff.


Repository: mesos


Description
---

The committed version of the HTTP Authentication remove a cleanup function which
ended causing mesos to hang. This patch reintroduces the discarded cleanup
function and per receiven process message queuing.


Diffs
-

  3rdparty/libprocess/src/process.cpp 43c83e30d5c59bd9845f5126ed79985d7c70b0c2 

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


Testing
---

At least three times
`make -j8 check`

Additionaly
`./3rdparty/libprocess/libprocess-tests --gtest_filter="HTTPTest.*" 
--gtest_repeat=1 --verbose`
`./3rdparty/libprocess/libprocess-tests --gtest_filter="HTTPTest.Endpoints" 
--gtest_repeat=1 --verbose`


Thanks,

Alexander Rojas



Re: Review Request 35711: Disallow special characters in role name.

2015-12-12 Thread haosdent huang

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

(Updated Dec. 12, 2015, 2:33 p.m.)


Review request for mesos, Adam B, Jie Yu, and Michael Park.


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


Repository: mesos


Description
---

Disallow special characters in role name.


Diffs (updated)
-

  include/mesos/roles.hpp PRE-CREATION 
  src/CMakeLists.txt 0d46043dd06007f2cba56626c82e8edd251cb8fb 
  src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
  src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
  src/common/roles.cpp PRE-CREATION 
  src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
  src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
  src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
  src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
  src/tests/resources_tests.cpp ce47bac73937a6a608016f46121137d6116c1399 
  src/tests/roles_tests.cpp PRE-CREATION 

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


Testing
---

make -j8 check


Thanks,

haosdent huang



Re: Review Request 35711: Disallow special characters in role name.

2015-12-12 Thread haosdent huang


> On Nov. 9, 2015, 11:26 p.m., Greg Mann wrote:
> > src/master/master.cpp, line 568
> > 
> >
> > Should we add a comment here saying that the validation of roles occurs 
> > in flags.cpp?

I put off the check to `Master::initialize()` like we check `flags.resources` 
in `Slave::initialize()`


- haosdent


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


On Dec. 12, 2015, 2:33 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35711/
> ---
> 
> (Updated Dec. 12, 2015, 2:33 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-2210
> https://issues.apache.org/jira/browse/MESOS-2210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Disallow special characters in role name.
> 
> 
> Diffs
> -
> 
>   include/mesos/roles.hpp PRE-CREATION 
>   src/CMakeLists.txt 0d46043dd06007f2cba56626c82e8edd251cb8fb 
>   src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e 
>   src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211 
>   src/common/roles.cpp PRE-CREATION 
>   src/master/flags.cpp f9223edb33483cb5619e7dd75221f735306462c6 
>   src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
>   src/master/master.cpp 0d1482279c68f2a4a27dabaf28774769a5d515c4 
>   src/slave/flags.cpp c4343ab7ff7b7b4d2c335119d41319b0779d2806 
>   src/tests/resources_tests.cpp ce47bac73937a6a608016f46121137d6116c1399 
>   src/tests/roles_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35711/diff/
> 
> 
> Testing
> ---
> 
> make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 40375: Support distinguishing revocable resources in the Resource protobuf.

2015-12-12 Thread Klaus Ma


> On Dec. 12, 2015, 3:47 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 1380-1382
> > 
> >
> > I'm not sure if this is necessary.  
> > 
> > To be as backwards compatible as possible, the `USAGE_SLACK` type would 
> > output as `{REV}`.  But is there any parsing of stringified `Resources` 
> > between the master and agent?

The communitcation between master and agent should be fine, we passed all UT 
with the patch.
Just not sure whether the 3rd part application will pass endpoint's output. If 
we updated it, we should drop an email to dev@; any comments?


- Klaus


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


On Dec. 12, 2015, 11:19 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40375/
> ---
> 
> (Updated Dec. 12, 2015, 11:19 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3888
> https://issues.apache.org/jira/browse/MESOS-3888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3888: We need to distinguish revocable resource for usage slack and 
> allocation slack.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 8ca2130 
>   include/mesos/v1/mesos.proto 8f357b0 
>   src/common/resources.cpp 5a79817 
>   src/tests/resources_tests.cpp ce47bac 
>   src/v1/resources.cpp d300842 
> 
> Diff: https://reviews.apache.org/r/40375/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 41250: Enabled slave get ALLOCATION_SLACK metrics.

2015-12-12 Thread Guangya Liu

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

(Updated 十二月 12, 2015, 4:43 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, Joseph Wu, 
and Klaus Ma.


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


Repository: mesos


Description
---

Enabled slave get ALLOCATION_SLACK metrics.


Diffs (updated)
-

  src/slave/metrics.hpp 2c22de521309806d70f9bd124359756b5e4c75b4 
  src/slave/metrics.cpp 1b73121264de3ff6eabc8f620e5b4d5930ba5d43 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 9bd86e122c070cd072a54d4de8097f844bd95bb0 

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


Testing
---

$ curl "http://192.168.0.107:5051/metrics/snapshot; 2>/dev/null| jq . | grep 
alloc_slack
  "slave/cpus_alloc_slack_revocable_percent": 0,
  "slave/cpus_alloc_slack_revocable_total": 3,
  "slave/cpus_alloc_slack_revocable_used": 0,
  "slave/disk_alloc_slack_revocable_percent": 0,
  "slave/disk_alloc_slack_revocable_total": 0,
  "slave/disk_alloc_slack_revocable_used": 0,
  "slave/mem_alloc_slack_revocable_percent": 0,
  "slave/mem_alloc_slack_revocable_total": 1000,
  "slave/mem_alloc_slack_revocable_used": 0,


Thanks,

Guangya Liu



Re: Review Request 40375: Support distinguishing revocable resources in the Resource protobuf.

2015-12-12 Thread Klaus Ma

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

(Updated Dec. 12, 2015, 11:19 p.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, and Joris Van 
Remoortere.


Changes
---

Address comments


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


Repository: mesos


Description
---

MESOS-3888: We need to distinguish revocable resource for usage slack and 
allocation slack.


Diffs (updated)
-

  include/mesos/mesos.proto 8ca2130 
  include/mesos/v1/mesos.proto 8f357b0 
  src/common/resources.cpp 5a79817 
  src/tests/resources_tests.cpp ce47bac 
  src/v1/resources.cpp d300842 

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


Testing
---

make && make check


Thanks,

Klaus Ma



Re: Review Request 36816: Support HTTP checks in Mesos health check program

2015-12-12 Thread haosdent huang

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

(Updated Dec. 12, 2015, 4:08 p.m.)


Review request for mesos, Adam B, Michael Park, and Timothy Chen.


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


Repository: mesos


Description
---

Support HTTP checks in Mesos health check program


Diffs
-

  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
  src/docker/executor.cpp 4042cec0acbe03d937ea3c53ffde745cbba552d2 
  src/health-check/main.cpp 0beaed575ec865d81e6e3d83d8a0c894613acba4 
  src/tests/health_check_tests.cpp 0fbccc373204d3b9431c614bdd6d046cc07e4566 

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


Testing (updated)
---

* Add unit test cases: HealthCheckTest.HealthyTaskThroughHttp and 
HealthCheckTest.HealthyTaskNotMatchHttpStatuses
make check


Thanks,

haosdent huang



Re: Review Request 36816: Support HTTP checks in Mesos health check program

2015-12-12 Thread haosdent huang

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

(Updated Dec. 12, 2015, 4:08 p.m.)


Review request for mesos, Adam B, Michael Park, and Timothy Chen.


Changes
---

Address comments


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


Repository: mesos


Description
---

Support HTTP checks in Mesos health check program


Diffs (updated)
-

  include/mesos/mesos.proto 8ca213062c480f0266ffc51a621eb4a118140c77 
  src/docker/executor.cpp 4042cec0acbe03d937ea3c53ffde745cbba552d2 
  src/health-check/main.cpp 0beaed575ec865d81e6e3d83d8a0c894613acba4 
  src/tests/health_check_tests.cpp 0fbccc373204d3b9431c614bdd6d046cc07e4566 

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


Testing
---

* Add a new unit test: HealthCheckTest.HealthyTaskThroughHttp
make check


Thanks,

haosdent huang



Re: Review Request 39622: CMake: Pointed Stout test linker flags at correct gtest directory.

2015-12-12 Thread Joris Van Remoortere

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

Ship it!


Ship It!

- Joris Van Remoortere


On Dec. 11, 2015, 6:11 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39622/
> ---
> 
> (Updated Dec. 11, 2015, 6:11 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Pointed Stout test linker flags at correct gtest directory.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
> 9893d741cd7c611dc65eba76be03e06dac618132 
> 
> Diff: https://reviews.apache.org/r/39622/diff/
> 
> 
> Testing
> ---
> 
> `make check` from CMake on OS X 10.10.
> `make check` from Autotools on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> PLEASE NOTE: I am on a terrible network with proxy problems and I can't SSH 
> into my Ubuntu box to test this from Ubuntu
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 41314: Fixed hang introduced in committed version of HTTP AuthenticatorRouter

2015-12-12 Thread Till Toenshoff

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


Can you try to explain why we need a sequence per receiver instead of a single, 
global one?


3rdparty/libprocess/src/process.cpp (lines 1136 - 1138)


Have a hard time understanding the comment. What do you think about this 
variant?

```
  CHECK(authentications.find(receiver) != authentications.end());
  
  // Remove the receiver's authentication sequence once it has been
  // processed completely.
  if (authentications[receiver].last == future) {
authentications.erase(receiver);
  }
```

In the end, I am not even sure we need this comment - leaving it up to you.


- Till Toenshoff


On Dec. 12, 2015, 4:12 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41314/
> ---
> 
> (Updated Dec. 12, 2015, 4:12 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, Michael Park, and Till 
> Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The committed version of the HTTP Authentication remove a cleanup function 
> which
> ended causing mesos to hang. This patch reintroduces the discarded cleanup
> function and per receiven process message queuing.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 43c83e30d5c59bd9845f5126ed79985d7c70b0c2 
> 
> Diff: https://reviews.apache.org/r/41314/diff/
> 
> 
> Testing
> ---
> 
> At least three times
> `make -j8 check`
> 
> Additionaly
> `./3rdparty/libprocess/libprocess-tests --gtest_filter="HTTPTest.*" 
> --gtest_repeat=1 --verbose`
> `./3rdparty/libprocess/libprocess-tests --gtest_filter="HTTPTest.Endpoints" 
> --gtest_repeat=1 --verbose`
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 39019: Windows: Added dirent compat code for non-Unix systems.

2015-12-12 Thread Joris Van Remoortere


> On Oct. 6, 2015, 5:12 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp,
> >  line 87
> > 
> >
> > Seems like this will give a negative array access if you set `path = 
> > "";`.

I don't see where this issue was "fixed".


- Joris


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


On Nov. 17, 2015, 7:06 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39019/
> ---
> 
> (Updated Nov. 17, 2015, 7:06 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3441
> https://issues.apache.org/jira/browse/MESOS-3441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dirent compat code for non-Unix systems.
> 
> 
> Diffs
> -
> 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 1a7037d64afeedc340258c92067e95d1d3caa027 
> 
> Diff: https://reviews.apache.org/r/39019/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39288: Quota: Added authentication of quota requests.

2015-12-12 Thread Joris Van Remoortere
Actually we do:
https://github.com/apache/mesos/blob/master/src/master/quota_handler.cpp#L391

On Fri, Dec 11, 2015 at 12:23 AM, Alexander Rukletsov 
wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39288/
> src/master/quota_handler.cpp
>  
> (Diff
> revision 11)
>
> 19
>
> #include 
>
> Guys, we don't need vector here any more, let's remove it.
>
>
> - Alexander Rukletsov
>
> On December 4th, 2015, 1:51 p.m. UTC, Jan Schlicht wrote:
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Joris Van
> Remoortere, and Till Toenshoff.
> By Jan Schlicht.
>
> *Updated Dec. 4, 2015, 1:51 p.m.*
> *Bugs: * MESOS-3861 
> *Repository: * mesos
> Description
>
> Added authentication of quota requests.
>
> Testing
>
> make check
>
> Diffs
>
>- src/master/master.hpp (4683fa542a740f9a0b80fff7fbe0e63ec66266f2)
>- src/master/quota_handler.cpp
>(76de7c44c2cbd16c5f76b1dc0a94f567e037bffe)
>
> View Diff 
>


Re: Review Request 41223: Cleaned up creation of HTTP auth headers in tests.

2015-12-12 Thread Joris Van Remoortere

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

Ship it!



src/tests/mesos.hpp (lines 539 - 542)


The indentation here changed.



src/tests/mesos.hpp (line 1189)


I'm going to pull this out in to a separate typo fix for you.


- Joris Van Remoortere


On Dec. 10, 2015, 9:46 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41223/
> ---
> 
> (Updated Dec. 10, 2015, 9:46 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up creation of HTTP auth headers in tests.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 12820fcf7c0d2a791071464ad8ed738664ad85de 
>   src/tests/mesos.hpp 2429ac5cbcd9c1a3949c11de94b542108a3c13d8 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 0a03b5f1ac7dec14bd99c31768f86100f2b60616 
>   src/tests/reservation_endpoints_tests.cpp 
> d5d2aa7c203aa7357b564ff51cd3b38230195d04 
> 
> Diff: https://reviews.apache.org/r/41223/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 39019: Windows: Added dirent compat code for non-Unix systems.

2015-12-12 Thread Joris Van Remoortere

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


Let's get some Windows team reviews on this.
let's try and use snake_case in stout.


3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(lines 17 - 18)


alphabetize.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(line 26)


brace on new line.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(lines 53 - 55)


As Joseph mentioned, this `NOTE:` is not necessary.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(line 81)


`directory == NULL`



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(line 89)


`directory->d_name == NULL`



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(lines 198 - 200)


Where does the internal data for DIR get freed?


- Joris Van Remoortere


On Nov. 17, 2015, 7:06 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39019/
> ---
> 
> (Updated Nov. 17, 2015, 7:06 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3441
> https://issues.apache.org/jira/browse/MESOS-3441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dirent compat code for non-Unix systems.
> 
> 
> Diffs
> -
> 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 1a7037d64afeedc340258c92067e95d1d3caa027 
> 
> Diff: https://reviews.apache.org/r/39019/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 41318: Fixed a connection leak in ProcessTest.Http1.

2015-12-12 Thread Joris Van Remoortere

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

Ship it!


Thanks for fixing this Ben!


3rdparty/libprocess/src/tests/process_tests.cpp (line 1416)


We capture the response, but don't use it later.
Should we either:
(1) not capture it
(2) `ASSERT_AWAIT_READY(response)` before `AWAIT_READY(body)`
(3) `ASSERT_AWAIT_READY(connection.send(request))`
What are your thoughts?



3rdparty/libprocess/src/tests/process_tests.cpp (lines 1778 - 1794)


This test also suffers from the same problem.
Mind fixing this one as well?


- Joris Van Remoortere


On Dec. 12, 2015, 11:11 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41318/
> ---
> 
> (Updated Dec. 12, 2015, 11:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> ProcessTest.Http1 hangs after a number of iterations because it uses
> http::post to do libprocess message passing but it uses the
> "User-Agent" header which means libprocess does not reply with a 202.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3970dd83aa4ddd2cbe3664c157fc15943ab1182d 
> 
> Diff: https://reviews.apache.org/r/41318/diff/
> 
> 
> Testing
> ---
> 
> Ran with many iterations.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Re: Review Request 41320: Clarified Subprocess PIPE usage in Subprocess tests.

2015-12-12 Thread Joris Van Remoortere

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

Ship it!


Ship It!

- Joris Van Remoortere


On Dec. 12, 2015, 11:11 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41320/
> ---
> 
> (Updated Dec. 12, 2015, 11:11 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we use PIPE() in many places where it is not necessary. This
> switches these instances to use the default (re-inherit the parent FD).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 869b920b4f25a74a69813ac4777e843d38300cbd 
> 
> Diff: https://reviews.apache.org/r/41320/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Review Request 41318: Fixed a connection leak in ProcessTest.Http1.

2015-12-12 Thread Ben Mahler

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

Review request for mesos and Alexander Rojas.


Repository: mesos


Description
---

ProcessTest.Http1 hangs after a number of iterations because it uses
http::post to do libprocess message passing but it uses the
"User-Agent" header which means libprocess does not reply with a 202.


Diffs
-

  3rdparty/libprocess/src/tests/process_tests.cpp 
3970dd83aa4ddd2cbe3664c157fc15943ab1182d 

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


Testing
---

Ran with many iterations.


Thanks,

Ben Mahler



Review Request 41320: Clarified Subprocess PIPE usage in Subprocess tests.

2015-12-12 Thread Ben Mahler

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

Review request for mesos and Alexander Rojas.


Repository: mesos


Description
---

Currently we use PIPE() in many places where it is not necessary. This
switches these instances to use the default (re-inherit the parent FD).


Diffs
-

  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
869b920b4f25a74a69813ac4777e843d38300cbd 

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


Testing
---

make check


Thanks,

Ben Mahler



Re: Review Request 39583: Windows: Added `WindowsError` to parallel `ErrnoError`.

2015-12-12 Thread Alex Clemmer

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

(Updated Dec. 12, 2015, 11:08 p.m.)


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


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


Repository: mesos


Description
---

Windows: Added `WindowsError` to parallel `ErrnoError`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
a25e2c1e5584e744c666bbc654eafbfc5f7b10e6 
  3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp 
605c23bb9273f12dabbd2d6fa87ca0b06d9958ca 
  3rdparty/libprocess/3rdparty/stout/include/stout/errorbase.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
9e3b007d9f8db1e16c1f6928d85c230612741cc0 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp 
PRE-CREATION 

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


Testing
---

`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.


Thanks,

Alex Clemmer



Re: Review Request 39583: Windows: Added `WindowsError` to parallel `ErrnoError`.

2015-12-12 Thread Alex Clemmer


> On Dec. 10, 2015, 2:48 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp, line 21
> > 
> >
> > Maybe we can drop the `Try` specific part of this comment.

This comment is copied from the Unix version, and it does appear to be true. So 
I recommend we keep it. But, if we want to change it, we should actually change 
it in both places.


- Alex


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


On Dec. 10, 2015, 2:14 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39583/
> ---
> 
> (Updated Dec. 10, 2015, 2:14 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-4110
> https://issues.apache.org/jira/browse/MESOS-4110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `WindowsError` to parallel `ErrnoError`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> a8c35c086ecae21701f6a720f25231c1b0d4e329 
>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
> fdd33512c8d8752093f72f597a7d647eb5e3c285 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39583/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 41291: Modified the scheduler tests to use the new executor HTTP based library.

2015-12-12 Thread Anand Mazumdar

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

(Updated Dec. 12, 2015, 11:50 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Fixed a race condition that showed up on ASF CI(reviewbot) where the executor 
`connected` callback can be invoked before `Containerizer::_launch` is 
completed. Surprisingly, they pass on my box with 300+ iterations.


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


Repository: mesos


Description
---

This change modifies the existing scheduler tests to use the new executor HTTP 
library instead of the old driver based interface.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp 67432109c7df6be0aa76e94a03bd5b2e9c96d14e 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Re: Review Request 39583: Windows: Added `WindowsError` to parallel `ErrnoError`.

2015-12-12 Thread Alex Clemmer


> On Dec. 10, 2015, 2:48 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp, lines 27-29
> > 
> >
> > Let's move this into ``

Per discussion we actually move the error type to errorbase.hpp, and then have 
error.hpp #include both `errorbase.hpp` and `windows/error.hpp`.


- Alex


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


On Dec. 12, 2015, 11:08 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39583/
> ---
> 
> (Updated Dec. 12, 2015, 11:08 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-4110
> https://issues.apache.org/jira/browse/MESOS-4110
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `WindowsError` to parallel `ErrnoError`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> a25e2c1e5584e744c666bbc654eafbfc5f7b10e6 
>   3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp 
> 605c23bb9273f12dabbd2d6fa87ca0b06d9958ca 
>   3rdparty/libprocess/3rdparty/stout/include/stout/errorbase.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 
> 9e3b007d9f8db1e16c1f6928d85c230612741cc0 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39583/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 41291: Modified the scheduler tests to use the new executor HTTP based library.

2015-12-12 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [41275, 41277, 41280, 41281, 41282, 41283, 41285, 41288, 
41290, 41291]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 12, 2015, 11:50 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41291/
> ---
> 
> (Updated Dec. 12, 2015, 11:50 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3550
> https://issues.apache.org/jira/browse/MESOS-3550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change modifies the existing scheduler tests to use the new executor 
> HTTP library instead of the old driver based interface.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 67432109c7df6be0aa76e94a03bd5b2e9c96d14e 
> 
> Diff: https://reviews.apache.org/r/41291/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>