Review Request 39338: Added code that appends the fetcher log to the agent log upon fetcher failure.

[Bernd Mathiske]

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

Review request for mesos, Benjamin Bannier, Ben Mahler, and Till Toenshoff.


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


Repository: mesos


Description
---

Added an onFailed() clause to the inspection of the fetcher subprocess run. 
This clause copies the fetcher log from /stderr and appends it to 
the agent log.

This is to facilitate debugging spurious fetch failures in production or CI.

Similar, but not the same: https://reviews.apache.org/r/37813/ (see MESOS-3743 
for an explanation).


Diffs
-

  src/slave/containerizer/fetcher.cpp 2b2298c329ed5fb5863cb0fed1491e478c3e5d5a 

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


Testing
---

Ran make check. As expected no change in behavior.
When I modified the fetcher to fail, 
I observed the expected extra output.


Thanks,

Bernd Mathiske

Re: Review Request 39320: Speeded up the test by reducing the allocation timeout.

[Benjamin Bannier]

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



src/tests/fault_tolerance_tests.cpp (line 721)


I don't think hardcoding specific timeouts here is optimal ... there's 
nothing here that prevents this from not failing on slow/high load machines. 
Better would be to e.g. `wait` for some observable side effect before 
continuing; at least add a `TODO` (or better ticket) for that.


- Benjamin Bannier


On Oct. 14, 2015, 4:29 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39320/
> ---
> 
> (Updated Oct. 14, 2015, 4:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3732
> https://issues.apache.org/jira/browse/MESOS-3732
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/fault_tolerance_tests.cpp 
> c97bc4691f9bac4a8677e6d2247be96ee9674b57 
> 
> Diff: https://reviews.apache.org/r/39320/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 39340: RegistryClient: Added streaming response read

[Jojy Varghese]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

RegistryClient: Added streaming response read


Diffs
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39338: Added code that appends the fetcher log to the agent log upon fetcher failure.

[Benjamin Bannier]

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



src/slave/containerizer/fetcher.cpp (line 703)


Could as well be `const`.



src/slave/containerizer/fetcher.cpp (line 799)


It would probably be better to stream the full message into the `LOG` 
object to get the full message into a single block in the block in face of 
concurrent `LOG` calls from other threads.


- Benjamin Bannier


On Oct. 15, 2015, 1:11 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39338/
> ---
> 
> (Updated Oct. 15, 2015, 1:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3743
> https://issues.apache.org/jira/browse/MESOS-3743
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an onFailed() clause to the inspection of the fetcher subprocess run. 
> This clause copies the fetcher log from /stderr and appends it 
> to the agent log.
> 
> This is to facilitate debugging spurious fetch failures in production or CI.
> 
> Similar, but not the same: https://reviews.apache.org/r/37813/ (see 
> MESOS-3743 for an explanation).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 2b2298c329ed5fb5863cb0fed1491e478c3e5d5a 
> 
> Diff: https://reviews.apache.org/r/39338/diff/
> 
> 
> Testing
> ---
> 
> Ran make check. As expected no change in behavior.
> When I modified the fetcher to fail, 
> I observed the expected extra output.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 39338: Added code that appends the fetcher log to the agent log upon fetcher failure.

[Till Toenshoff]

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

Ship it!



src/slave/containerizer/fetcher.cpp (line 703)


It could but we don't enforce that - actually some of us go as far as 
saying we shouldn't make this const unless we do that for all possibly const 
types (consistency > strictness).


- Till Toenshoff


On Oct. 15, 2015, 1:11 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39338/
> ---
> 
> (Updated Oct. 15, 2015, 1:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3743
> https://issues.apache.org/jira/browse/MESOS-3743
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an onFailed() clause to the inspection of the fetcher subprocess run. 
> This clause copies the fetcher log from /stderr and appends it 
> to the agent log.
> 
> This is to facilitate debugging spurious fetch failures in production or CI.
> 
> Similar, but not the same: https://reviews.apache.org/r/37813/ (see 
> MESOS-3743 for an explanation).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 2b2298c329ed5fb5863cb0fed1491e478c3e5d5a 
> 
> Diff: https://reviews.apache.org/r/39338/diff/
> 
> 
> Testing
> ---
> 
> Ran make check. As expected no change in behavior.
> When I modified the fetcher to fail, 
> I observed the expected extra output.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 39217: Windows: Added `stout/os/chsize.hpp`.

[James Peach]

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



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ftruncate.hpp (line 
30)


Should this be ```_chsize_s``` so you can use a large offset?


- James Peach


On Oct. 15, 2015, 8:36 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39217/
> ---
> 
> (Updated Oct. 15, 2015, 8:36 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `stout/os/chsize.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/ftruncate.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ftruncate.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39217/diff/
> 
> 
> Testing
> ---
> 
> Built CMake solution and ran `make check` (or equivalent) on Windows 10, 
> Ubuntu 15, OS X 10.10. Built autotools solution and ran `make check` on 
> Ubuntu 15.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 39363: Fixed a double deletion race in ProcessManager::wait.

[Ben Mahler]

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

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


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


Repository: mesos


Description
---

The current code non-atomically checks for the gate being empty in order to 
have last thread to arrive at the gate delete it. This is racy, and can lead to 
multiple threads trying to delete the gate! The information about how many 
waiters remain must come atomically from the call to Gate::arrive.


Diffs
-

  3rdparty/libprocess/src/gate.hpp 7d5df706c601c3a1e2055d7f9b78caea0f4182e8 
  3rdparty/libprocess/src/process.cpp 0454554e7b6a39f94cfea02f08ca51ef6b35859a 

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


Testing
---

Ran the tests in a loop.


Thanks,

Ben Mahler

Re: Review Request 39365: HTTP Scheduler should abort when MasterDetector create fails.

[Joseph Wu]

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

(Updated Oct. 15, 2015, 5:03 p.m.)


Review request for mesos, Anand Mazumdar and Artem Harutyunyan.


Changes
---

Change ABORT to EXIT.


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


Repository: mesos


Description
---

If not, the process will segfault during initialization.


Diffs (updated)
-

  src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 

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


Testing (updated)
---

`make check`

```
export DEFAULT_PRINCIPAL=root
build/src/event-call-framework --master="asdf://127.0.0.1:5050"
```
Check that it does not segfault.


Thanks,

Joseph Wu

Re: Review Request 39340: RegistryClient: Added streaming response read

[Jojy Varghese]

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

(Updated Oct. 16, 2015, 12:15 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Fixed io::write to use the correct write overload.


Repository: mesos


Description
---

RegistryClient: Added streaming response read


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39250: Puller refactor: moved untar to a common place

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39013, 38443, 39184, 39155, 39156, 38579, 39014, 39068, 
38941, 39015, 39016, 39017, 39053, 39112, 38580, 38747, 39250]

All tests passed.

- Mesos ReviewBot


On Oct. 16, 2015, 12:20 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39250/
> ---
> 
> (Updated Oct. 16, 2015, 12:20 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Puller refactor: moved untar to a common place
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 74d0e1ead7d630e65a7e75cb6123139b9197efef 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39250/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39250: Puller refactor: moved untar to a common place

[Jojy Varghese]

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

(Updated Oct. 15, 2015, 10:54 p.m.)


Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

Puller refactor: moved untar to a common place


Diffs
-

  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39353: Fixed and added tests for docker image name parsing.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39353]

All tests passed.

- Mesos ReviewBot


On Oct. 15, 2015, 7:17 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39353/
> ---
> 
> (Updated Oct. 15, 2015, 7:17 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing docker image parsing did not work in many cases: if a digest is 
> present, a registry port is included. And it could not resolve the ambiguity 
> that exists between the registry and the repository.
> 
> This follows docker's approach to parsing, which is a bit hacky due to the 
> way the registry and repository are disambiguated, see the comments.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f2ea4fc62507ce30b7a7981fd6a8c4749eb97190 
>   src/slave/containerizer/provisioner/docker/message.hpp 
> 6368bf4caec6f8c3ac97282f41c55381f920bce9 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> 637c97c0973bda492826803a962c99635647692d 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9c3c45a81be6398722a37911788e347a4e91cce8 
> 
> Diff: https://reviews.apache.org/r/39353/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 39250: Puller refactor: moved untar to a common place

[Jojy Varghese]

> On Oct. 15, 2015, 10:51 p.m., Timothy Chen wrote:
> > Ship It!
> 
> Timothy Chen wrote:
> This doesn't seem to depend on anything from 38747. I'll be merging this 
> without the previous one.
> 
> Timothy Chen wrote:
> Btw please rebase this patch.

The dependency is because RemotePuller has not yet landed.


- Jojy


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


On Oct. 15, 2015, 10:54 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39250/
> ---
> 
> (Updated Oct. 15, 2015, 10:54 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Puller refactor: moved untar to a common place
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 74d0e1ead7d630e65a7e75cb6123139b9197efef 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39250/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39353: Fixed and added tests for docker image name parsing.

[Gilbert Song]

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

Ship it!


Ship It!

- Gilbert Song


On Oct. 15, 2015, 12:17 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39353/
> ---
> 
> (Updated Oct. 15, 2015, 12:17 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing docker image parsing did not work in many cases: if a digest is 
> present, a registry port is included. And it could not resolve the ambiguity 
> that exists between the registry and the repository.
> 
> This follows docker's approach to parsing, which is a bit hacky due to the 
> way the registry and repository are disambiguated, see the comments.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f2ea4fc62507ce30b7a7981fd6a8c4749eb97190 
>   src/slave/containerizer/provisioner/docker/message.hpp 
> 6368bf4caec6f8c3ac97282f41c55381f920bce9 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> 637c97c0973bda492826803a962c99635647692d 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9c3c45a81be6398722a37911788e347a4e91cce8 
> 
> Diff: https://reviews.apache.org/r/39353/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 39340: RegistryClient: Added streaming response read

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39013, 38443, 39184, 39155, 39156, 38579, 39014, 39068, 
38941, 39015, 39016, 39017, 39053, 39112, 39340]

All tests passed.

- Mesos ReviewBot


On Oct. 16, 2015, 12:15 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39340/
> ---
> 
> (Updated Oct. 16, 2015, 12:15 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RegistryClient: Added streaming response read
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 471783d88b73b62afacac3d7952ebb5d5f442097 
> 
> Diff: https://reviews.apache.org/r/39340/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

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

(Updated 十月 16, 2015, 2:36 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Only update the task status when its old status is not terminal.


Diffs (updated)
-

  src/master/master.cpp ba12a83 
  src/tests/status_update_manager_tests.cpp 9970d71 

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


Testing
---

UT:
1. Write a test for this change.
2. make successfully!
3. make check successfully!
4. Run test framework successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 39365: HTTP Scheduler should abort when MasterDetector create fails.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39365]

All tests passed.

- Mesos ReviewBot


On Oct. 16, 2015, 12:30 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39365/
> ---
> 
> (Updated Oct. 16, 2015, 12:30 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Vinod Kone.
> 
> 
> Bugs: MESOS-3748
> https://issues.apache.org/jira/browse/MESOS-3748
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master detector is created during construction of the `MesosProcess`.  If 
> the flags cannot be loaded or if the `MasterDetector::create` fails, the 
> master detector will be null and will result in a segfault if it is used 
> during initialization.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 
> 
> Diff: https://reviews.apache.org/r/39365/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> ```
> export DEFAULT_PRINCIPAL=root
> build/src/event-call-framework --master="asdf://127.0.0.1:5050"
> ```
> Check that it does not segfault.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39368: Updated /state.json to show revocable resources.

[Jie Yu]

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

Ship it!



src/common/http.cpp (line 68)


static?



src/common/http.cpp (line 101)


Not yours, but any reason need this temp variable? Can we just do:

```
foreachpair (.., .., nonRevocable.types())
```

Also, I prefer Value::Type than Value_Type



src/common/http.cpp (line 103)


s/Value_Type/Value::Type/



src/common/http.cpp (line 110)


Ditto here.



src/common/http.cpp (line 112)


s/Value_Type/Value::Type/


- Jie Yu


On Oct. 16, 2015, 12:48 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39368/
> ---
> 
> (Updated Oct. 16, 2015, 12:48 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-3563
> https://issues.apache.org/jira/browse/MESOS-3563
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revocable resources now show up in the state.json endpoint with "_revocable" 
> suffix.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 99b843afac8bb07ef233c70117f52e64747f9502 
>   src/tests/common/http_tests.cpp 8a01ffc655f9c19f776da2f99283a9ffdd7020aa 
> 
> Diff: https://reviews.apache.org/r/39368/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 39250: Puller refactor: moved untar to a common place

[Jojy Varghese]

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

(Updated Oct. 16, 2015, 12:17 a.m.)


Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

Puller refactor: moved untar to a common place


Diffs
-

  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

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

(Updated 十月 16, 2015, 2:03 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Only update the task status when its old status is not terminal.


Diffs (updated)
-

  src/master/master.cpp 6bee4f3 
  src/tests/status_update_manager_tests.cpp 9970d71 

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


Testing
---

UT:
1. Write a test for this change.
2. make successfully!
3. make check successfully!
4. Run test framework successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 39365: HTTP Scheduler should abort when MasterDetector create fails.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十月 16, 2015, 12:30 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39365/
> ---
> 
> (Updated 十月 16, 2015, 12:30 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Vinod Kone.
> 
> 
> Bugs: MESOS-3748
> https://issues.apache.org/jira/browse/MESOS-3748
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The master detector is created during construction of the `MesosProcess`.  If 
> the flags cannot be loaded or if the `MasterDetector::create` fails, the 
> master detector will be null and will result in a segfault if it is used 
> during initialization.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 
> 
> Diff: https://reviews.apache.org/r/39365/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> ```
> export DEFAULT_PRINCIPAL=root
> build/src/event-call-framework --master="asdf://127.0.0.1:5050"
> ```
> Check that it does not segfault.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39363: Fixed a double deletion race in ProcessManager::wait.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On Oct. 15, 2015, 11:32 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39363/
> ---
> 
> (Updated Oct. 15, 2015, 11:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3728
> https://issues.apache.org/jira/browse/MESOS-3728
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current code non-atomically checks for the gate being empty in order to 
> have last thread to arrive at the gate delete it. This is racy, and can lead 
> to multiple threads trying to delete the gate! The information about how many 
> waiters remain must come atomically from the call to Gate::arrive.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/gate.hpp 7d5df706c601c3a1e2055d7f9b78caea0f4182e8 
>   3rdparty/libprocess/src/process.cpp 
> 0454554e7b6a39f94cfea02f08ca51ef6b35859a 
> 
> Diff: https://reviews.apache.org/r/39363/diff/
> 
> 
> Testing
> ---
> 
> Ran the tests in a loop.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 39250: Puller refactor: moved untar to a common place

[Timothy Chen]

> On Oct. 15, 2015, 10:51 p.m., Timothy Chen wrote:
> > Ship It!
> 
> Timothy Chen wrote:
> This doesn't seem to depend on anything from 38747. I'll be merging this 
> without the previous one.

Btw please rebase this patch.


- Timothy


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


On Oct. 15, 2015, 10:54 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39250/
> ---
> 
> (Updated Oct. 15, 2015, 10:54 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Puller refactor: moved untar to a common place
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 74d0e1ead7d630e65a7e75cb6123139b9197efef 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39250/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39250: Puller refactor: moved untar to a common place

[Timothy Chen]

> On Oct. 15, 2015, 10:51 p.m., Timothy Chen wrote:
> > Ship It!

This doesn't seem to depend on anything from 38747. I'll be merging this 
without the previous one.


- Timothy


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


On Oct. 15, 2015, 10:54 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39250/
> ---
> 
> (Updated Oct. 15, 2015, 10:54 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Puller refactor: moved untar to a common place
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 74d0e1ead7d630e65a7e75cb6123139b9197efef 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39250/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39365: HTTP Scheduler should abort when MasterDetector create fails.

[Anand Mazumdar]

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

Ship it!


LGTM

- Anand Mazumdar


On Oct. 16, 2015, 12:03 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39365/
> ---
> 
> (Updated Oct. 16, 2015, 12:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-3748
> https://issues.apache.org/jira/browse/MESOS-3748
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If not, the process will segfault during initialization.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 
> 
> Diff: https://reviews.apache.org/r/39365/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> ```
> export DEFAULT_PRINCIPAL=root
> build/src/event-call-framework --master="asdf://127.0.0.1:5050"
> ```
> Check that it does not segfault.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39250: Puller refactor: moved untar to a common place

[Jojy Varghese]

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

(Updated Oct. 16, 2015, 12:20 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

review comments.


Repository: mesos


Description
---

Puller refactor: moved untar to a common place


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39365: HTTP Scheduler should abort when MasterDetector create fails.

[Joseph Wu]

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

(Updated Oct. 15, 2015, 5:30 p.m.)


Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Vinod Kone.


Changes
---

Change the other `return` to an `exit` in the constructor.


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


Repository: mesos


Description (updated)
---

The master detector is created during construction of the `MesosProcess`.  If 
the flags cannot be loaded or if the `MasterDetector::create` fails, the master 
detector will be null and will result in a segfault if it is used during 
initialization.


Diffs (updated)
-

  src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 

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


Testing
---

`make check`

```
export DEFAULT_PRINCIPAL=root
build/src/event-call-framework --master="asdf://127.0.0.1:5050"
```
Check that it does not segfault.


Thanks,

Joseph Wu

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

> On 十月 15, 2015, 6:38 p.m., Vinod Kone wrote:
> > src/tests/status_update_manager_tests.cpp, line 844
> > 
> >
> > new line.

Do you mean to add an another new line? I found two new lines are added before 
all functions in this test file, so I add an another new line before this 
function.


- Yong Qiao


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


On 十月 16, 2015, 2:36 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated 十月 16, 2015, 2:36 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ba12a83 
>   src/tests/status_update_manager_tests.cpp 9970d71 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 1. Write a test for this change.
> 2. make successfully!
> 3. make check successfully!
> 4. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 39368: Updated /state.json to show revocable resources.

[Jian Qiu]

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

Ship it!


Ship It!

- Jian Qiu


On Oct. 16, 2015, 12:48 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39368/
> ---
> 
> (Updated Oct. 16, 2015, 12:48 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-3563
> https://issues.apache.org/jira/browse/MESOS-3563
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revocable resources now show up in the state.json endpoint with "_revocable" 
> suffix.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 99b843afac8bb07ef233c70117f52e64747f9502 
>   src/tests/common/http_tests.cpp 8a01ffc655f9c19f776da2f99283a9ffdd7020aa 
> 
> Diff: https://reviews.apache.org/r/39368/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 39358: Network monitoring metrics table has common style

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39358]

All tests passed.

- Mesos ReviewBot


On Oct. 15, 2015, 8:18 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39358/
> ---
> 
> (Updated Oct. 15, 2015, 8:18 p.m.)
> 
> 
> Review request for mesos, Dave Lester and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Network monitoring metrics table has common style
> 
> 
> Diffs
> -
> 
>   docs/network-monitoring.md b266ae59cd698ed11dc49ee1987efb868e2ef746 
> 
> Diff: https://reviews.apache.org/r/39358/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 39360: Relocate MesosContainerizer specific files to the correct location

[Gilbert Song]

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

(Updated Oct. 15, 2015, 4:23 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, Jojy Varghese, and Timothy Chen.


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


Repository: mesos


Description
---

Relocate MesosContainerizer specific files to the correct location


Diffs (updated)
-

  src/Makefile.am 96ce73b301c55d23bf4a5292e3d028148426a878 
  src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
  src/examples/test_isolator_module.cpp 
577dfcac260b4f5df7ab4e9599e4caac46ccd1e1 
  src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
  src/slave/containerizer/isolators/cgroups/constants.hpp  
  src/slave/containerizer/isolators/cgroups/cpushare.hpp 
54b83a7d67f9cacbca4f9dd9b9b72a3dbc2e5263 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp 
ba748c6caec7253b42167e8a4f9b4535da858259 
  src/slave/containerizer/isolators/cgroups/mem.hpp  
  src/slave/containerizer/isolators/cgroups/mem.cpp 
6f49e5ac77ab03248127a607664c8f895be72877 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp  
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
03035dfbfb84470ba39ed9ecfd1eb73890e3f784 
  src/slave/containerizer/isolators/filesystem/linux.hpp 
93e85f2aa7bfceb7e55ff33bdc2e0e0a5cb8f880 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.hpp  
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.hpp  
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/isolators/namespaces/pid.hpp  
  src/slave/containerizer/isolators/namespaces/pid.cpp 
a9823e08b195b8df82de2a7b410a4e6ef99f8853 
  src/slave/containerizer/isolators/network/helper.cpp 
e5fb99e87ac16150b85b1c6f6965681f7fe77ce0 
  src/slave/containerizer/isolators/network/port_mapping.hpp  
  src/slave/containerizer/isolators/network/port_mapping.cpp 
e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
  src/slave/containerizer/isolators/posix.hpp  
  src/slave/containerizer/isolators/posix/disk.hpp  
  src/slave/containerizer/isolators/posix/disk.cpp 
73e62a225da062733557287afa2273d8183d76fd 
  src/slave/containerizer/linux_launcher.cpp 
c03b89eb0678825b03a052874d6262f377a39e13 
  src/slave/containerizer/mesos/containerizer.cpp 
d1fc5a460e7313828014eea999cf4e63dde01921 
  src/slave/containerizer/provisioner/appc/paths.hpp  
  src/slave/containerizer/provisioner/appc/paths.cpp 
8817c0ff4b6806f08afd322e250a9a53b7b0a5d6 
  src/slave/containerizer/provisioner/appc/spec.hpp  
  src/slave/containerizer/provisioner/appc/spec.cpp 
bbe523d2ee1dd558cc5007e578cbf23abac8e1de 
  src/slave/containerizer/provisioner/appc/store.hpp 
e8455197dcc3f4c9856db20605f6862b8755a946 
  src/slave/containerizer/provisioner/appc/store.cpp 
a5ef4ea7cd08423360120430833c5881053637f5 
  src/slave/containerizer/provisioner/backend.hpp  
  src/slave/containerizer/provisioner/backend.cpp 
b5d96701ae6bd49365b169f4e5150b8c4dae1870 
  src/slave/containerizer/provisioner/backends/bind.hpp 
1685938fb4349e790b9595cc4c67584c7f31a392 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/containerizer/provisioner/backends/copy.hpp 
7a5aaa41d8f6842ef437ed7a34235d8baac4bfff 
  src/slave/containerizer/provisioner/backends/copy.cpp 
92fb0988da0bdd5a2b5a5f53ab61b7bb19c61cda 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/message.hpp 
6368bf4caec6f8c3ac97282f41c55381f920bce9 
  src/slave/containerizer/provisioner/docker/message.proto  
  src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
885080dbd3603f8c71ac867b88edcfd22276567f 
  src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
2b2de5245bccbd01a856b214ac6525278d794537 
  src/slave/containerizer/provisioner/docker/paths.hpp  
  src/slave/containerizer/provisioner/docker/paths.cpp 
5733fb7137b1ecb8a904cc5354425c60c9e065f5 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp  
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 
  src/slave/containerizer/provisioner/docker/store.hpp 
b5e6c87d38bea531c1b504915a84625fc9777e0d 
  

Re: Review Request 39365: HTTP Scheduler should abort when MasterDetector create fails.

[Vinod Kone]

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



src/scheduler/scheduler.cpp (line 170)


For posterity, can you add a blurb in the review on what the exact issue is?

Also, are the other error() calls in this method safe to execute?


- Vinod Kone


On Oct. 16, 2015, 12:03 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39365/
> ---
> 
> (Updated Oct. 16, 2015, 12:03 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-3748
> https://issues.apache.org/jira/browse/MESOS-3748
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If not, the process will segfault during initialization.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 
> 
> Diff: https://reviews.apache.org/r/39365/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> ```
> export DEFAULT_PRINCIPAL=root
> build/src/event-call-framework --master="asdf://127.0.0.1:5050"
> ```
> Check that it does not segfault.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39365: HTTP Scheduler should abort when MasterDetector create fails.

[Joseph Wu]

> On Oct. 15, 2015, 5:13 p.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp, line 170
> > 
> >
> > For posterity, can you add a blurb in the review on what the exact 
> > issue is?
> > 
> > Also, are the other error() calls in this method safe to execute?

Can do.

There are 2 other `error()` calls:
1) In the initialization, in case the flags fail to parse.  This one will lead 
to a segfault.
2) In the master detection loop (after initialization).  This one is ok, since 
we handle it by retrying in a loop.


- Joseph


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


On Oct. 15, 2015, 5:03 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39365/
> ---
> 
> (Updated Oct. 15, 2015, 5:03 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-3748
> https://issues.apache.org/jira/browse/MESOS-3748
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If not, the process will segfault during initialization.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 
> 
> Diff: https://reviews.apache.org/r/39365/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> ```
> export DEFAULT_PRINCIPAL=root
> build/src/event-call-framework --master="asdf://127.0.0.1:5050"
> ```
> Check that it does not segfault.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39360: Relocate MesosContainerizer specific files to the correct location

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39360]

All tests passed.

- Mesos ReviewBot


On Oct. 15, 2015, 11:23 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39360/
> ---
> 
> (Updated Oct. 15, 2015, 11:23 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3129
> https://issues.apache.org/jira/browse/MESOS-3129
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Relocate MesosContainerizer specific files to the correct location
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 96ce73b301c55d23bf4a5292e3d028148426a878 
>   src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
>   src/examples/test_isolator_module.cpp 
> 577dfcac260b4f5df7ab4e9599e4caac46ccd1e1 
>   src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
>   src/slave/containerizer/isolators/cgroups/constants.hpp  
>   src/slave/containerizer/isolators/cgroups/cpushare.hpp 
> 54b83a7d67f9cacbca4f9dd9b9b72a3dbc2e5263 
>   src/slave/containerizer/isolators/cgroups/cpushare.cpp 
> ba748c6caec7253b42167e8a4f9b4535da858259 
>   src/slave/containerizer/isolators/cgroups/mem.hpp  
>   src/slave/containerizer/isolators/cgroups/mem.cpp 
> 6f49e5ac77ab03248127a607664c8f895be72877 
>   src/slave/containerizer/isolators/cgroups/perf_event.hpp  
>   src/slave/containerizer/isolators/cgroups/perf_event.cpp 
> 03035dfbfb84470ba39ed9ecfd1eb73890e3f784 
>   src/slave/containerizer/isolators/filesystem/linux.hpp 
> 93e85f2aa7bfceb7e55ff33bdc2e0e0a5cb8f880 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> 8823b7850a1ac17fc4f4868aadf1b04428d2381b 
>   src/slave/containerizer/isolators/filesystem/posix.hpp  
>   src/slave/containerizer/isolators/filesystem/posix.cpp 
> eec510c4f7655d67b33ad90210eeb57fcc910684 
>   src/slave/containerizer/isolators/filesystem/shared.hpp  
>   src/slave/containerizer/isolators/filesystem/shared.cpp 
> 73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
>   src/slave/containerizer/isolators/namespaces/pid.hpp  
>   src/slave/containerizer/isolators/namespaces/pid.cpp 
> a9823e08b195b8df82de2a7b410a4e6ef99f8853 
>   src/slave/containerizer/isolators/network/helper.cpp 
> e5fb99e87ac16150b85b1c6f6965681f7fe77ce0 
>   src/slave/containerizer/isolators/network/port_mapping.hpp  
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
>   src/slave/containerizer/isolators/posix.hpp  
>   src/slave/containerizer/isolators/posix/disk.hpp  
>   src/slave/containerizer/isolators/posix/disk.cpp 
> 73e62a225da062733557287afa2273d8183d76fd 
>   src/slave/containerizer/linux_launcher.cpp 
> c03b89eb0678825b03a052874d6262f377a39e13 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d1fc5a460e7313828014eea999cf4e63dde01921 
>   src/slave/containerizer/provisioner/appc/paths.hpp  
>   src/slave/containerizer/provisioner/appc/paths.cpp 
> 8817c0ff4b6806f08afd322e250a9a53b7b0a5d6 
>   src/slave/containerizer/provisioner/appc/spec.hpp  
>   src/slave/containerizer/provisioner/appc/spec.cpp 
> bbe523d2ee1dd558cc5007e578cbf23abac8e1de 
>   src/slave/containerizer/provisioner/appc/store.hpp 
> e8455197dcc3f4c9856db20605f6862b8755a946 
>   src/slave/containerizer/provisioner/appc/store.cpp 
> a5ef4ea7cd08423360120430833c5881053637f5 
>   src/slave/containerizer/provisioner/backend.hpp  
>   src/slave/containerizer/provisioner/backend.cpp 
> b5d96701ae6bd49365b169f4e5150b8c4dae1870 
>   src/slave/containerizer/provisioner/backends/bind.hpp 
> 1685938fb4349e790b9595cc4c67584c7f31a392 
>   src/slave/containerizer/provisioner/backends/bind.cpp 
> 1fe1746c0bc1c9c12e1378e6438122a91b58316b 
>   src/slave/containerizer/provisioner/backends/copy.hpp 
> 7a5aaa41d8f6842ef437ed7a34235d8baac4bfff 
>   src/slave/containerizer/provisioner/backends/copy.cpp 
> 92fb0988da0bdd5a2b5a5f53ab61b7bb19c61cda 
>   src/slave/containerizer/provisioner/docker/local_puller.hpp 
> 4574e8a04663482625d7b54f765741f221ec13e0 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 74d0e1ead7d630e65a7e75cb6123139b9197efef 
>   src/slave/containerizer/provisioner/docker/message.hpp 
> 6368bf4caec6f8c3ac97282f41c55381f920bce9 
>   src/slave/containerizer/provisioner/docker/message.proto  
>   src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
> 885080dbd3603f8c71ac867b88edcfd22276567f 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
> 2b2de5245bccbd01a856b214ac6525278d794537 
>   src/slave/containerizer/provisioner/docker/paths.hpp  
>   

Re: Review Request 39338: Added code that appends the fetcher log to the agent log upon fetcher failure.

[Marco Massenzio]

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

Ship it!


Ship It!

- Marco Massenzio


On Oct. 15, 2015, 1:11 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39338/
> ---
> 
> (Updated Oct. 15, 2015, 1:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3743
> https://issues.apache.org/jira/browse/MESOS-3743
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an onFailed() clause to the inspection of the fetcher subprocess run. 
> This clause copies the fetcher log from /stderr and appends it 
> to the agent log.
> 
> This is to facilitate debugging spurious fetch failures in production or CI.
> 
> Similar, but not the same: https://reviews.apache.org/r/37813/ (see 
> MESOS-3743 for an explanation).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 2b2298c329ed5fb5863cb0fed1491e478c3e5d5a 
> 
> Diff: https://reviews.apache.org/r/39338/diff/
> 
> 
> Testing
> ---
> 
> Ran make check. As expected no change in behavior.
> When I modified the fetcher to fail, 
> I observed the expected extra output.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 38747: Adding digest utilities

[Jojy Varghese]

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

(Updated Oct. 16, 2015, 1:45 a.m.)


Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.


Changes
---

Added std::async for async dispatch


Repository: mesos


Description
---

Added SHA256, SHA512 implementation.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am eb7393904988afc0bc5e9f7dadd545e0d6c04e5f 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
99c9754b6a5c7ceb12808a782da9cb9fb3fc731e 
  3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 39372: Introduced a callback interface for testing the Scheduler Library

[Anand Mazumdar]

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

Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


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


Repository: mesos


Description
---

Instead of introducing a `DROP_HTTP_EVENT` filtering abstraction, made the 
existing interface callback based that allows to use the already existing 
testing semantics. Also, the new interface allows us to use a generic interface 
across scheduler library versions.

This gets rid of the following hack:
```
// Enqueues all received events into a libprocess queue.
ACTION_P(Enqueue, queue)
{
  std::queue events = arg0;
  while (!events.empty()) {
// Note that we currently drop HEARTBEATs because most of these tests
// are not designed to deal with heartbeats.
// TODO(vinod): Implement DROP_HTTP_CALLS that can filter heartbeats.
if (events.front().type() == Event::HEARTBEAT) {
  VLOG(1) << "Ignoring HEARTBEAT event";
} else {
  queue->put(events.front());
}
events.pop();
  }
}
```

New way ( similar to what we do for the driver implementation )
```
EXPECT_CALL(callbacks, heartbeat())
.WillRepeatedly(Return()); // Ignore heartbeats.
```


Diffs
-

  src/tests/mesos.hpp 3e58b454c75a2ab9f8b4a29785fa823afefd0c8a 
  src/tests/scheduler_tests.cpp 7946cb48d62f4ed6d0fdbc771746518e31921f97 

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


Testing
---

make check ( I only modified a couple of existing tests to show the new 
interface usage. If the interface looks good, would go ahead and modify all the 
tests )


Thanks,

Anand Mazumdar

Re: Review Request 39217: Windows: Added `stout/os/chsize.hpp`.

[Alex Clemmer]

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

(Updated Oct. 15, 2015, 8:36 p.m.)


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


Repository: mesos


Description
---

Windows: Added `stout/os/chsize.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/ftruncate.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ftruncate.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
PRE-CREATION 

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


Testing
---

Built CMake solution and ran `make check` (or equivalent) on Windows 10, Ubuntu 
15, OS X 10.10. Built autotools solution and ran `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Re: Review Request 39210: Windows: Moved `realpath` to its own file, `stout/os/realpath.hpp`.

[Alex Clemmer]

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

(Updated Oct. 15, 2015, 8:31 p.m.)


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


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


Repository: mesos


Description
---

Windows: Moved `realpath` to its own file, `stout/os/realpath.hpp`.

(NOTE: This does not resolve MESOS-3632, it's only a partial solution.)


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
5141c1369af60afd6cd5c70c6295d575ea960a83 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/realpath.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp 
ddcda7bdb294272a7a64a7a46e0576e8c4430eec 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39180: Windows: Added support for `stout/os/open.hpp`.

[Alex Clemmer]

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

(Updated Oct. 15, 2015, 8:31 p.m.)


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


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


Repository: mesos


Description
---

Windows: Added support for `stout/os/open.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp 
43f261fd7a60b534f642f826ebf6ab18d72180c2 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/open.hpp 
023993d859e3a101ca387c1a514cd75de0d2beb1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/open.hpp 
14fa11765c222cb4e80f5e45360d0f05facb578e 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 
  src/Makefile.am 96ce73b301c55d23bf4a5292e3d028148426a878 
  src/sched/sched.cpp 9c5e3b8e42605f4647d897ba02ea3a17e494f355 
  src/tests/scheduler_http_api_tests.cpp 
ae1f6ffda966a2eca1eda578261a0a0c4432e6b3 

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


Testing
---

Ran both stout_tests from VS.


Thanks,

Alex Clemmer

Re: Review Request 39250: Puller refactor: moved untar to a common place

[Timothy Chen]

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

Ship it!


Ship It!


src/slave/containerizer/provisioner/docker/puller.hpp (line 70)


target directory to be more clear


- Timothy Chen


On Oct. 14, 2015, 12:45 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39250/
> ---
> 
> (Updated Oct. 14, 2015, 12:45 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Puller refactor: moved untar to a common place
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/local_puller.cpp 
> 74d0e1ead7d630e65a7e75cb6123139b9197efef 
>   src/slave/containerizer/provisioner/docker/puller.hpp 
> 105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
>   src/slave/containerizer/provisioner/docker/puller.cpp 
> cb05324689ffa26ce830b513e2d71b55517da3cb 
>   src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39250/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39353: Fixed and added tests for docker image name parsing.

[Ben Mahler]

> On Oct. 15, 2015, 8:28 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioner/docker/message.hpp, line 51
> > 
> >
> > Would this also allow @@ or @@@ ? Wondering if we can use a regular 
> > expression parser for parsing ?
> 
> Ben Mahler wrote:
> Yes, this won't reject invalid input (given this doesn't return a Try).
> 
> The first step (this change) is to fix our code to accept valid input 
> (which we weren't doing for all cases!). I've left the TODO for validation 
> and rejecting bad input.
> 
> Jojy Varghese wrote:
> Agreed. What do you think about using a RE parser?

Hm.. worth considering for sure, the ambiguity around whether the first 
component is a registry or a repository component might make it tricky..


- Ben


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


On Oct. 15, 2015, 7:17 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39353/
> ---
> 
> (Updated Oct. 15, 2015, 7:17 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing docker image parsing did not work in many cases: if a digest is 
> present, a registry port is included. And it could not resolve the ambiguity 
> that exists between the registry and the repository.
> 
> This follows docker's approach to parsing, which is a bit hacky due to the 
> way the registry and repository are disambiguated, see the comments.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f2ea4fc62507ce30b7a7981fd6a8c4749eb97190 
>   src/slave/containerizer/provisioner/docker/message.hpp 
> 6368bf4caec6f8c3ac97282f41c55381f920bce9 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> 637c97c0973bda492826803a962c99635647692d 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9c3c45a81be6398722a37911788e347a4e91cce8 
> 
> Diff: https://reviews.apache.org/r/39353/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Review Request 39358: Network monitoring metrics table has common style

[Tomasz Janiszewski]

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

Review request for mesos, Dave Lester and Jie Yu.


Repository: mesos


Description
---

Network monitoring metrics table has common style


Diffs
-

  docs/network-monitoring.md b266ae59cd698ed11dc49ee1987efb868e2ef746 

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


Testing
---


Thanks,

Tomasz Janiszewski

Re: Review Request 39262: Windows:[1/3] Moved `os::environ` -> `os::raw::environment`.

[Alex Clemmer]

> On Oct. 13, 2015, 8:32 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/raw/environment.hpp, 
> > lines 34-37
> > 
> >
> > Which header defines the `environ` macro on Windows?  And why isn't it 
> > included in this file?  (Or does it need to be included at all?)

It's one of the headers that's included by default, sort of how libc gets 
included on Unix. You can tell GCC to not include libc with special flags, but 
I actually don't know if you can do this with MSVC.


- Alex


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


On Oct. 13, 2015, 6:36 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39262/
> ---
> 
> (Updated Oct. 13, 2015, 6:36 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows standard headers define a macro, `environ`, that holds the
> environment data for the executing process. Unfortunately, the existance
> of this macro makes it impossible to use a function called
> `os::environ`.
> 
> Our solution to this problem in this commit is to move the function
> family at `os::environ`* to `os::raw::environment`. This family exists
> in the `os::raw` because they are the "unstructured" variants of
> functions with the same names that exist in the `os` namespace. For
> example, `os::raw::environment` returns a `char**` which is the
> "unstructured" equivalent of `os::environment`, which returns a
> `map`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5141c1369af60afd6cd5c70c6295d575ea960a83 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/raw/environment.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> a042d061159c83e1652e7288b90809981d2818c9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 2b0966889af73238a08e29f1136d0ce286a0ffda 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> e6d36ec1bf414b52d0899f0edf83e0ad8910dd0e 
> 
> Diff: https://reviews.apache.org/r/39262/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Ubuntu 15, OS X 10.10, and running the `check` project on 
> Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39353: Fixed and added tests for docker image name parsing.

[Jojy Varghese]

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



src/slave/containerizer/provisioner/docker/message.hpp (line 46)


How about handling parse errors ? Maybe change this to a Try?



src/slave/containerizer/provisioner/docker/message.hpp (line 51)


Would this also allow @@ or @@@ ? Wondering if we can use a regular 
expression parser for parsing ?


- Jojy Varghese


On Oct. 15, 2015, 7:17 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39353/
> ---
> 
> (Updated Oct. 15, 2015, 7:17 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing docker image parsing did not work in many cases: if a digest is 
> present, a registry port is included. And it could not resolve the ambiguity 
> that exists between the registry and the repository.
> 
> This follows docker's approach to parsing, which is a bit hacky due to the 
> way the registry and repository are disambiguated, see the comments.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f2ea4fc62507ce30b7a7981fd6a8c4749eb97190 
>   src/slave/containerizer/provisioner/docker/message.hpp 
> 6368bf4caec6f8c3ac97282f41c55381f920bce9 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> 637c97c0973bda492826803a962c99635647692d 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9c3c45a81be6398722a37911788e347a4e91cce8 
> 
> Diff: https://reviews.apache.org/r/39353/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 39353: Fixed and added tests for docker image name parsing.

[Ben Mahler]

> On Oct. 15, 2015, 8:28 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioner/docker/message.hpp, line 46
> > 
> >
> > How about handling parse errors ? Maybe change this to a Try?

Agreed, did you see the TODO..?

```
// TODO(bmahler): Validate based on docker's validation logic
// and return a Try here.
```


> On Oct. 15, 2015, 8:28 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioner/docker/message.hpp, line 51
> > 
> >
> > Would this also allow @@ or @@@ ? Wondering if we can use a regular 
> > expression parser for parsing ?

Yes, this won't reject invalid input (given this doesn't return a Try).

The first step (this change) is to fix our code to accept valid input (which we 
weren't doing for all cases!). I've left the TODO for validation and rejecting 
bad input.


- Ben


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


On Oct. 15, 2015, 7:17 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39353/
> ---
> 
> (Updated Oct. 15, 2015, 7:17 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing docker image parsing did not work in many cases: if a digest is 
> present, a registry port is included. And it could not resolve the ambiguity 
> that exists between the registry and the repository.
> 
> This follows docker's approach to parsing, which is a bit hacky due to the 
> way the registry and repository are disambiguated, see the comments.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f2ea4fc62507ce30b7a7981fd6a8c4749eb97190 
>   src/slave/containerizer/provisioner/docker/message.hpp 
> 6368bf4caec6f8c3ac97282f41c55381f920bce9 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> 637c97c0973bda492826803a962c99635647692d 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9c3c45a81be6398722a37911788e347a4e91cce8 
> 
> Diff: https://reviews.apache.org/r/39353/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 39217: Windows: Added `stout/os/chsize.hpp`.

[Alex Clemmer]

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

(Updated Oct. 15, 2015, 8:33 p.m.)


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


Repository: mesos


Description
---

Windows: Added `stout/os/chsize.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/ftruncate.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ftruncate.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
PRE-CREATION 

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


Testing
---

Built CMake solution and ran `make check` (or equivalent) on Windows 10, Ubuntu 
15, OS X 10.10. Built autotools solution and ran `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

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

[Jan Schlicht]

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

(Updated Oct. 15, 2015, 11:15 p.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

Fix issues.


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


Repository: mesos


Description
---

[WIP] Added authentication of quota requests.


Diffs (updated)
-

  src/master/master.hpp e7b16fdd21a8caa77a39956a8520cf1381186598 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---


Thanks,

Jan Schlicht

Re: Review Request 39340: RegistryClient: Added streaming response read

[Jojy Varghese]

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

(Updated Oct. 15, 2015, 9:23 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Fixed total read count


Repository: mesos


Description
---

RegistryClient: Added streaming response read


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38747: Adding digest utilities

[Jojy Varghese]

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

(Updated Oct. 15, 2015, 9:24 p.m.)


Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.


Changes
---

reverted back to async loop; fixed read size.


Repository: mesos


Description
---

Added SHA256, SHA512 implementation.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am eb7393904988afc0bc5e9f7dadd545e0d6c04e5f 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
99c9754b6a5c7ceb12808a782da9cb9fb3fc731e 
  3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39219: Windows: Added support for `slave/state.cpp`.

[Alex Clemmer]

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

(Updated Oct. 15, 2015, 8:31 p.m.)


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


Bugs: MESOS-3615, MESOS-3627, MESOS-3628, MESOS-3629, MESOS-3630, MESOS-3631, 
MESOS-3633, MESOS-3658, MESOS-3659, MESOS-3660, and MESOS-3693
https://issues.apache.org/jira/browse/MESOS-3615
https://issues.apache.org/jira/browse/MESOS-3627
https://issues.apache.org/jira/browse/MESOS-3628
https://issues.apache.org/jira/browse/MESOS-3629
https://issues.apache.org/jira/browse/MESOS-3630
https://issues.apache.org/jira/browse/MESOS-3631
https://issues.apache.org/jira/browse/MESOS-3633
https://issues.apache.org/jira/browse/MESOS-3658
https://issues.apache.org/jira/browse/MESOS-3659
https://issues.apache.org/jira/browse/MESOS-3660
https://issues.apache.org/jira/browse/MESOS-3693


Repository: mesos


Description
---

Windows: Added support for `slave/state.cpp`.


Diffs (updated)
-

  src/slave/state.hpp 5a1a9bb2c86639612a8f065b7a66c8179696297a 
  src/slave/state.cpp 81c4b96d879f974f0dfba3fb977184122eab 

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


Testing
---

Built CMake solution and ran `make check` (or equivalent) on Windows 10, Ubuntu 
15, OS X 10.10. Built autotools solution and ran `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Re: Review Request 39202: CMake: Moved libevent, gmock, http-parser to CMake on Windows.

[Alex Clemmer]

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

(Updated Oct. 15, 2015, 8:31 p.m.)


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


Repository: mesos


Description
---

CMake: Moved libevent, gmock, http-parser to CMake on Windows.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
f0de224b0d7924344fb1945b387b728a7241df87 
  3rdparty/libprocess/3rdparty/http-parser/CMakeLists.txt.template PRE-CREATION 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
577aac82f527f3cce9f5b0cd24cc51091c311789 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
60108ff598fb075584196aa3d8e8e66e726c9f2a 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
d60bce237ec73f462fd373590a3589d554e76fc3 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39353: Fixed and added tests for docker image name parsing.

[Jojy Varghese]

> On Oct. 15, 2015, 8:28 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioner/docker/message.hpp, line 46
> > 
> >
> > How about handling parse errors ? Maybe change this to a Try?
> 
> Ben Mahler wrote:
> Agreed, did you see the TODO..?
> 
> ```
> // TODO(bmahler): Validate based on docker's validation logic
> // and return a Try here.
> ```

Ah my bad.


> On Oct. 15, 2015, 8:28 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/provisioner/docker/message.hpp, line 51
> > 
> >
> > Would this also allow @@ or @@@ ? Wondering if we can use a regular 
> > expression parser for parsing ?
> 
> Ben Mahler wrote:
> Yes, this won't reject invalid input (given this doesn't return a Try).
> 
> The first step (this change) is to fix our code to accept valid input 
> (which we weren't doing for all cases!). I've left the TODO for validation 
> and rejecting bad input.

Agreed. What do you think about using a RE parser?


- Jojy


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


On Oct. 15, 2015, 7:17 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39353/
> ---
> 
> (Updated Oct. 15, 2015, 7:17 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing docker image parsing did not work in many cases: if a digest is 
> present, a registry port is included. And it could not resolve the ambiguity 
> that exists between the registry and the repository.
> 
> This follows docker's approach to parsing, which is a bit hacky due to the 
> way the registry and repository are disambiguated, see the comments.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f2ea4fc62507ce30b7a7981fd6a8c4749eb97190 
>   src/slave/containerizer/provisioner/docker/message.hpp 
> 6368bf4caec6f8c3ac97282f41c55381f920bce9 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> 637c97c0973bda492826803a962c99635647692d 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9c3c45a81be6398722a37911788e347a4e91cce8 
> 
> Diff: https://reviews.apache.org/r/39353/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Review Request 39360: Relocate MesosContainerizer specific files to the correct location

[Gilbert Song]

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

Review request for mesos, Ben Mahler, Jie Yu, Jojy Varghese, and Timothy Chen.


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


Repository: mesos


Description
---

Relocate MesosContainerizer specific files to the correct location


Diffs
-

  src/Makefile.am 96ce73b301c55d23bf4a5292e3d028148426a878 
  src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
  src/examples/test_isolator_module.cpp 
577dfcac260b4f5df7ab4e9599e4caac46ccd1e1 
  src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
  src/slave/containerizer/isolators/cgroups/constants.hpp  
  src/slave/containerizer/isolators/cgroups/cpushare.hpp 
54b83a7d67f9cacbca4f9dd9b9b72a3dbc2e5263 
  src/slave/containerizer/isolators/cgroups/cpushare.cpp 
ba748c6caec7253b42167e8a4f9b4535da858259 
  src/slave/containerizer/isolators/cgroups/mem.hpp  
  src/slave/containerizer/isolators/cgroups/mem.cpp 
6f49e5ac77ab03248127a607664c8f895be72877 
  src/slave/containerizer/isolators/cgroups/perf_event.hpp  
  src/slave/containerizer/isolators/cgroups/perf_event.cpp 
03035dfbfb84470ba39ed9ecfd1eb73890e3f784 
  src/slave/containerizer/isolators/filesystem/linux.hpp 
93e85f2aa7bfceb7e55ff33bdc2e0e0a5cb8f880 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.hpp  
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.hpp  
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/isolators/namespaces/pid.hpp  
  src/slave/containerizer/isolators/namespaces/pid.cpp 
a9823e08b195b8df82de2a7b410a4e6ef99f8853 
  src/slave/containerizer/isolators/network/helper.cpp 
e5fb99e87ac16150b85b1c6f6965681f7fe77ce0 
  src/slave/containerizer/isolators/network/port_mapping.hpp  
  src/slave/containerizer/isolators/network/port_mapping.cpp 
e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
  src/slave/containerizer/isolators/posix.hpp  
  src/slave/containerizer/isolators/posix/disk.hpp  
  src/slave/containerizer/isolators/posix/disk.cpp 
73e62a225da062733557287afa2273d8183d76fd 
  src/slave/containerizer/linux_launcher.cpp 
c03b89eb0678825b03a052874d6262f377a39e13 
  src/slave/containerizer/mesos/containerizer.cpp 
d1fc5a460e7313828014eea999cf4e63dde01921 
  src/slave/containerizer/provisioner/appc/paths.hpp  
  src/slave/containerizer/provisioner/appc/paths.cpp 
8817c0ff4b6806f08afd322e250a9a53b7b0a5d6 
  src/slave/containerizer/provisioner/appc/spec.hpp  
  src/slave/containerizer/provisioner/appc/spec.cpp 
bbe523d2ee1dd558cc5007e578cbf23abac8e1de 
  src/slave/containerizer/provisioner/appc/store.hpp 
e8455197dcc3f4c9856db20605f6862b8755a946 
  src/slave/containerizer/provisioner/appc/store.cpp 
a5ef4ea7cd08423360120430833c5881053637f5 
  src/slave/containerizer/provisioner/backend.hpp  
  src/slave/containerizer/provisioner/backend.cpp 
b5d96701ae6bd49365b169f4e5150b8c4dae1870 
  src/slave/containerizer/provisioner/backends/bind.hpp 
1685938fb4349e790b9595cc4c67584c7f31a392 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/containerizer/provisioner/backends/copy.hpp 
7a5aaa41d8f6842ef437ed7a34235d8baac4bfff 
  src/slave/containerizer/provisioner/backends/copy.cpp 
92fb0988da0bdd5a2b5a5f53ab61b7bb19c61cda 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/message.hpp 
6368bf4caec6f8c3ac97282f41c55381f920bce9 
  src/slave/containerizer/provisioner/docker/message.proto  
  src/slave/containerizer/provisioner/docker/metadata_manager.hpp 
885080dbd3603f8c71ac867b88edcfd22276567f 
  src/slave/containerizer/provisioner/docker/metadata_manager.cpp 
2b2de5245bccbd01a856b214ac6525278d794537 
  src/slave/containerizer/provisioner/docker/paths.hpp  
  src/slave/containerizer/provisioner/docker/paths.cpp 
5733fb7137b1ecb8a904cc5354425c60c9e065f5 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp  
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 
  src/slave/containerizer/provisioner/docker/store.hpp 
b5e6c87d38bea531c1b504915a84625fc9777e0d 
  src/slave/containerizer/provisioner/docker/store.cpp 

Re: Review Request 39217: Windows: Added `stout/os/chsize.hpp`.

[Alex Clemmer]

> On Oct. 15, 2015, 8:58 p.m., James Peach wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ftruncate.hpp, 
> > line 30
> > 
> >
> > Should this be ```_chsize_s``` so you can use a large offset?

Ah. I didn't know about that function. Yes, that looks like an improvement! 
Thanks!


- Alex


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


On Oct. 15, 2015, 8:36 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39217/
> ---
> 
> (Updated Oct. 15, 2015, 8:36 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `stout/os/chsize.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/ftruncate.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ftruncate.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39217/diff/
> 
> 
> Testing
> ---
> 
> Built CMake solution and ran `make check` (or equivalent) on Windows 10, 
> Ubuntu 15, OS X 10.10. Built autotools solution and ran `make check` on 
> Ubuntu 15.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

[Timothy Chen]

> On Oct. 9, 2015, 8:12 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioner/docker/spec.hpp, line 22
> > 
> >
> > Do we need this ? If not, remove this include.
> 
> Gilbert Song wrote:
> Ditto. It is used in the next patch.

You should only include it in the next patch then. I'll be merging both patches 
together this time but please only include what you need per patch.


- Timothy


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


On Oct. 9, 2015, 9:39 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Oct. 9, 2015, 9:39 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 39382: Windows: Moved `os::rm` to its own file, `stout/os/rm.hpp`.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

Windows: Moved `os::rm` to its own file, `stout/os/rm.hpp`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
5141c1369af60afd6cd5c70c6295d575ea960a83 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/rm.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
a042d061159c83e1652e7288b90809981d2818c9 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
2b0966889af73238a08e29f1136d0ce286a0ffda 

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


Testing
---

CMake `make check` on Ubuntu 15, OS X 10.10, and ran `check` target in VS2015 
on Windows 10.

Autotools `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Review Request 39381: CMake: Added protobuf and `slave/flags.cpp` to Windows builds.

[Alex Clemmer]

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

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


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


Repository: mesos


Description
---

CMake: Added protobuf and `slave/flags.cpp` to Windows builds.


Diffs
-

  src/CMakeLists.txt 98e76cee81ab206f3ffe7989711abc38f49c4352 

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


Testing
---

CMake `make check` on Ubuntu 15, OS X 10.10, and ran `check` target in VS2015 
on Windows 10.

Autotools `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Review Request 39385: Fixed link conversion regexp in website.

[Neil Conway]

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

Review request for mesos and Dave Lester.


Repository: mesos


Description
---

The previous regexp was:

/\((.*)(\.md)\)/

Given text like:

Text (comment): [link](foo.md)

This will greedily match ".*", so \1 is bound to "comment): [link](foo", which
seems unlikely to be the desired result.


Diffs
-

  site/Rakefile 9fe4730579ee0e802880e6e3547b4e550be41c8c 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 38919: Validation of Docker Image Manifests

[Timothy Chen]

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


Mesos style checker didn't pass for this patch. I'll fix this for you for now, 
please run the style check before pushing,

- Timothy Chen


On Oct. 9, 2015, 11:39 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38919/
> ---
> 
> (Updated Oct. 9, 2015, 11:39 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-3099
> https://issues.apache.org/jira/browse/MESOS-3099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Validation of Docker Image Manifests
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38919/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 39217: Windows: Added `stout/os/chsize.hpp`.

[Alex Clemmer]

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

(Updated Oct. 16, 2015, 4:45 a.m.)


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


Repository: mesos


Description
---

Windows: Added `stout/os/chsize.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/ftruncate.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ftruncate.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
PRE-CREATION 

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


Testing
---

Built CMake solution and ran `make check` (or equivalent) on Windows 10, Ubuntu 
15, OS X 10.10. Built autotools solution and ran `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Review Request 39376: Windows: Prepared agent for Windows support of `process/socket.hpp`.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

In particular, we will remove the inclusion of `stout/os.hpp` from
`process/socket.hpp`, which will cause  a couple of files to break. Our
solution to this problem is to include `os.hpp` directly in those files.


Diffs
-

  src/linux/perf.cpp f7035ddb2507a7646d88dd517d048018f695448a 
  src/slave/containerizer/composing.cpp 
8c3a2353999523ef055332320443931b61843d21 
  src/slave/containerizer/isolators/cgroups/mem.cpp 
6f49e5ac77ab03248127a607664c8f895be72877 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
471783d88b73b62afacac3d7952ebb5d5f442097 
  src/zookeeper/zookeeper.cpp e44403ee91904415471382dfa4e0a6e0adfdb74f 

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


Testing
---

CMake `make check` on Ubuntu 15, OS X 10.10, and ran `check` target in VS2015 
on Windows 10.

Autotools `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Review Request 39375: Windows: Introduced socket flag interop.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

POSIX has a bunch of flags like `SHUT_SD` which let you do things like
specify (e.g.) how a socket shuts down. Windows has equivalents for many
of these flags as well, but they have different names.

So that we can avoid changing any of the socket code, we alias the flags
we know we need with their POSIX names.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 

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


Testing
---

CMake `make check` on Ubuntu 15, OS X 10.10, and ran `check` target in VS2015 
on Windows 10.

Autotools `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Review Request 39379: Windows: Prepared agent for Windows changes to `stout/flags/flags.hpp`.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

When we remove the line that includes `stout/os.hpp` from
`stout/flags/flags.hpp`, it will break `slave/launcher.cpp`. Our
solution is to directly include the parts of `os.hpp` that are relevant
to the launcher.


Diffs
-

  src/slave/containerizer/isolators/posix.hpp 
ee9d275e7fe5fc22c1bab86dd0a558cc8ab9044e 
  src/slave/containerizer/launcher.cpp 6649d35ec593d264e8508a8195f2470f05888c0a 

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


Testing
---

CMake `make check` on Ubuntu 15, OS X 10.10, and ran `check` target in VS2015 
on Windows 10.

Autotools `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Review Request 39377: Windows: Add Windows support to `process/socket.hpp`.

[Alex Clemmer]

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

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


Bugs: MESOS-3628, MESOS-3649 and MESOS-3656
https://issues.apache.org/jira/browse/MESOS-3628
https://issues.apache.org/jira/browse/MESOS-3649
https://issues.apache.org/jira/browse/MESOS-3656


Repository: mesos


Description
---

Windows: Add Windows support to `process/socket.hpp`.


Diffs
-

  3rdparty/libprocess/include/process/socket.hpp 
4b2597f617fc4072144c9666a378d4ffad53a592 
  3rdparty/libprocess/src/poll_socket.cpp 
28ed102972a9d8f88048aea4046ed837b6a25b35 

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


Testing
---

CMake `make check` on Ubuntu 15, OS X 10.10, and ran `check` target in VS2015 
on Windows 10.

Autotools `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Re: Review Request 39385: Fixed link conversion regexp in website.

[Neil Conway]

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

(Updated Oct. 16, 2015, 4:42 a.m.)


Review request for mesos and Dave Lester.


Repository: mesos


Description
---

The previous regexp was:

/\((.*)(\.md)\)/

Given text like:

Text (comment): [link](foo.md)

This will greedily match ".*", so \1 is bound to "comment): [link](foo", which
seems unlikely to be the desired result.


Diffs
-

  site/Rakefile 9fe4730579ee0e802880e6e3547b4e550be41c8c 

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


Testing (updated)
---

Tested using "http://rubular.com/; -- didn't actually test on 
mesos-website-container, since it doesn't use the site code from Git just yet.


Thanks,

Neil Conway

Review Request 39386: Fix uncorrect launcher dir in docker executor.

[haosdent huang]

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

Review request for mesos and Timothy Chen.


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


Repository: mesos


Description
---

Fix uncorrect launcher dir in docker executor.


Diffs
-

  src/docker/executor.hpp 8ab4e98c41017e8420b98c63a59d8e4bc1a99172 
  src/docker/executor.cpp 1e4901335854c49e46cd7b132e79ccb11cd72ade 
  src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 

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


Testing
---

* make check
* make install and then test with marathon to check if launcher_dir passes 
correctly.


Thanks,

haosdent huang

Re: Review Request 39262: Windows:[1/3] Moved `os::environ` -> `os::raw::environment`.

[Alex Clemmer]

> On Oct. 13, 2015, 8:32 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/raw/environment.hpp, 
> > lines 34-37
> > 
> >
> > Which header defines the `environ` macro on Windows?  And why isn't it 
> > included in this file?  (Or does it need to be included at all?)
> 
> Alex Clemmer wrote:
> It's one of the headers that's included by default, sort of how libc gets 
> included on Unix. You can tell GCC to not include libc with special flags, 
> but I actually don't know if you can do this with MSVC.

I'm marking this as dropped, but I don't know if that's the right thing to do! 
Let me know if this doesn't address the issue.


- Alex


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


On Oct. 15, 2015, 8:33 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39262/
> ---
> 
> (Updated Oct. 15, 2015, 8:33 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows standard headers define a macro, `environ`, that holds the
> environment data for the executing process. Unfortunately, the existance
> of this macro makes it impossible to use a function called
> `os::environ`.
> 
> Our solution to this problem in this commit is to move the function
> family at `os::environ`* to `os::raw::environment`. This family exists
> in the `os::raw` because they are the "unstructured" variants of
> functions with the same names that exist in the `os` namespace. For
> example, `os::raw::environment` returns a `char**` which is the
> "unstructured" equivalent of `os::environment`, which returns a
> `map`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5141c1369af60afd6cd5c70c6295d575ea960a83 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/raw/environment.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> a042d061159c83e1652e7288b90809981d2818c9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 2b0966889af73238a08e29f1136d0ce286a0ffda 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> ee2a7a79617612ed448e650cc77608c4962fe5a5 
> 
> Diff: https://reviews.apache.org/r/39262/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Ubuntu 15, OS X 10.10, and running the `check` project on 
> Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38051]

All tests passed.

- Mesos ReviewBot


On Oct. 16, 2015, 2:36 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Oct. 16, 2015, 2:36 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ba12a83 
>   src/tests/status_update_manager_tests.cpp 9970d71 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 1. Write a test for this change.
> 2. make successfully!
> 3. make check successfully!
> 4. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 39340: RegistryClient: Added streaming response read

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39013, 38443, 39184, 39155, 39156, 38579, 39014, 39068, 
38941, 39015, 39016, 39017, 39053, 39112, 39340]

All tests passed.

- Mesos ReviewBot


On Oct. 15, 2015, 4:03 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39340/
> ---
> 
> (Updated Oct. 15, 2015, 4:03 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RegistryClient: Added streaming response read
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> 471783d88b73b62afacac3d7952ebb5d5f442097 
> 
> Diff: https://reviews.apache.org/r/39340/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 39345: Enable build on FreeBSD, start porting components.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [39345]

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

Error:
 2015-10-15 17:21:02 URL:https://reviews.apache.org/r/39345/diff/raw/ 
[22252/22252] -> "39345.patch" [1]
Successfully applied: Enable build on FreeBSD, start porting components.

Enable build on FreeBSD, start porting components.

My build steps are:

- Install dependencies from http://mesos.apache.org/gettingstarted/
- Install libexecinfo
- Install clang36 (system clang is 3.4)
- boostrap
- ../configure --with-curl=/usr/local --with-apr=/usr/local 
--with-svn=/usr/local CC=clang36 CXX=clang++36
- gmake CC=clang36 CXX=clang++36
- gmake CC=clang36 CXX=clang++36 check

I disabled one test because I haven't had a chance to debug it and I wanted to 
get a bit further in the test suite. A check run is attached.


Review: https://reviews.apache.org/r/39345
Checking 14 files using filter 
--filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
Total errors found: 0
ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
mesos:
  configure.ac
  src/Makefile.am
stout:
  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/mac.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/signals.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/os/sysctl.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp
  3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
libprocess:
  3rdparty/libprocess/3rdparty/Makefile.am
  3rdparty/libprocess/configure.ac
  3rdparty/libprocess/src/poll_socket.cpp
  3rdparty/libprocess/src/tests/http_tests.cpp
Failed to commit patch

- Mesos ReviewBot


On Oct. 15, 2015, 5:16 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39345/
> ---
> 
> (Updated Oct. 15, 2015, 5:16 p.m.)
> 
> 
> Review request for mesos and Ian Downes.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-1563
> 
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1563
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable build on FreeBSD, start porting components.
> 
> My build steps are:
> 
> - Install dependencies from http://mesos.apache.org/gettingstarted/
> - Install libexecinfo
> - Install clang36 (system clang is 3.4)
> - boostrap
> - ../configure --with-curl=/usr/local --with-apr=/usr/local 
> --with-svn=/usr/local CC=clang36 CXX=clang++36
> - gmake CC=clang36 CXX=clang++36
> - gmake CC=clang36 CXX=clang++36 check
> 
> I disabled one test because I haven't had a chance to debug it and I wanted 
> to get a bit further in the test suite. A check run is attached.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> e64e3d9e958fab3c7c25fad17dcc2b279d255500 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> d1e2df6151149e03ffb4a76e2c24ff78b891e016 
>   3rdparty/libprocess/3rdparty/stout/include/stout/mac.hpp 
> 9428717fac4655898d7768957f02937592e1a398 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> 3f829bafe96526bc2263c9f228f85de38c435f60 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5141c1369af60afd6cd5c70c6295d575ea960a83 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
> 7eb51e8771e95f57548fc35753e75c6d56cd97cd 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
> 828c9c777b1b0e067c2551b79b9747a3cf4fb0aa 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/signals.hpp 
> e9b05ef3b59fd068137cb12e36591de2d4a801a1 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/sysctl.hpp 
> 8a8ede325cfe8f024e1be4db24b0c8118d18f359 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> 

Re: Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

[Alexander Rojas]

> On Oct. 14, 2015, 2:17 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2354
> > 
> >
> > Let's clean this up a little bit. IIUC this is the only place in the 
> > code where we dispatch to HttpProxy::handle, and it's not clear that we 
> > need a pointer to the future (it's a shared_ptr under the covers anyway). 
> > So how about we do something like:
> > 
> > HttpProxy::handle(Future&& future, Request&& request)
> > {
> >   items.push(new Item(move(future), move(request)));
> >   ...;
> > }
> > 
> > Then at the call site:
> > 
> > // Dispatch to the proxy needs to be done at this point ...
> > // ...
> > // Also note that we need to pass a copy of  'request' because 
> > HttpProxy::handle takes ownership.
> > dispatch(proxy, ::handle, promise->future(), 
> > Request(*request));
> > 
> > Sound good? Or am I missing something?

So the deal here with moving is with the `dispatch` function which is created 
via some boost macros to simulate variadic templates. So if we made something 
like the requested code `dispatch` gets resolved to:

```cpp
template 
void dispatch(const Process* process,
  void (T::*method)(P0, P1),
  A0 a0,
  A1 a1)
{
  dispatch(process->self(), method, a0, a1);
}
```

Which in turn calls another template function

```cpp
template 
void dispatch(const Process& process,
  void (T::*method)(P0, P1),
  A0 a0,
  A1 a1)
{
  dispatch(process.self(), method, a0, a1);
}
```

we can make the first `dispatch` to receive an rvalue reference, but since it 
doesn't forward its parameters with an `std::move` to the second, they get 
copied, but for some reason as constants values. When the third dispatch gets 
called:

```cpp
template 
void dispatch(const PID& pid, void (T::*method)(P0, P1), A0 a0, A1 a1)
{
  std::shared_ptr> f(
new std::function([=](ProcessBase* process) {
  assert(process != NULL);
  T* t = dynamic_cast(process);
  assert(t != NULL);
  (t->*method)(a0, a1);
}));
  internal::dispatch(pid, f, (method));
}
```

Creates an error where we are trying to move a `const reference`. So even if it 
worked, we wouldn't be saving copies since `dispatch` will be doing them 
anyway. However the pointer type is really unnecessary given that copies of 
`Future` are not expensive, so instead of a pointer, we can have a copy and 
the code will look like the one you proposed (removing that `new 
Future`) adn the copy of request is already there.

If there is nothing against this idea, I will be proceeding with it.


- Alexander


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


On Oct. 14, 2015, 5:35 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Oct. 14, 2015, 5:35 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3705
> https://issues.apache.org/jira/browse/MESOS-3705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When using the same socket to send multiple HTTP requests to different 
> actors. If the actor responsible for handling the first request is stuck 
> handling another event while a subsequent request can reply immediatly, the 
> order of the responses is altered violating HTTP Pipelining.
> 
> This patch fixes that problem enforcing that every response is sent in the 
> order the corresponding request arrived. It also adds a test to reproduce the 
> issue and verify the fix works.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/src/process.cpp 
> 0454554e7b6a39f94cfea02f08ca51ef6b35859a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d13d3888abbf3db552df4a9f83e54667e598ded9 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Review Request 39345: Enable build on FreeBSD, start porting components.

[David Forsythe]

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

Review request for mesos and Ian Downes.


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

https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1563


Repository: mesos


Description
---

Enable build on FreeBSD, start porting components.

My build steps are:

- Install dependencies from http://mesos.apache.org/gettingstarted/
- Install libexecinfo
- Install clang36 (system clang is 3.4)
- boostrap
- ../configure --with-curl=/usr/local --with-apr=/usr/local 
--with-svn=/usr/local CC=clang36 CXX=clang++36
- gmake CC=clang36 CXX=clang++36
- gmake CC=clang36 CXX=clang++36 check

I disabled one test because I didn't haven't had a chance to debug it and I 
wanted to get a bit further in the test suite. A check run is attached.


Diffs
-

  3rdparty/libprocess/3rdparty/Makefile.am 
e64e3d9e958fab3c7c25fad17dcc2b279d255500 
  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
d1e2df6151149e03ffb4a76e2c24ff78b891e016 
  3rdparty/libprocess/3rdparty/stout/include/stout/mac.hpp 
9428717fac4655898d7768957f02937592e1a398 
  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
3f829bafe96526bc2263c9f228f85de38c435f60 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
5141c1369af60afd6cd5c70c6295d575ea960a83 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
7eb51e8771e95f57548fc35753e75c6d56cd97cd 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
828c9c777b1b0e067c2551b79b9747a3cf4fb0aa 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/signals.hpp 
e9b05ef3b59fd068137cb12e36591de2d4a801a1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/sysctl.hpp 
8a8ede325cfe8f024e1be4db24b0c8118d18f359 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
a042d061159c83e1652e7288b90809981d2818c9 
  3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 
4cc781bddbf7ee10cc0671f62d710fb4fa80e293 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
ee2a7a79617612ed448e650cc77608c4962fe5a5 
  3rdparty/libprocess/configure.ac 7c2bcffe5c7be1f7d90e6df470d20a00245bfbff 
  3rdparty/libprocess/src/poll_socket.cpp 
28ed102972a9d8f88048aea4046ed837b6a25b35 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d13d3888abbf3db552df4a9f83e54667e598ded9 
  configure.ac 66f9b32773861d2921d99189e0fbcaea48c456a9 
  src/Makefile.am ba9feb10057a863a41a85ab3612f3a841de01df4 

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


Testing
---

make check on coreos (I'll get a ubuntu instance for future testing)

gmake check on freebsd 10.2 (failing) (log attached)


File Attachments


gmake check
  
https://reviews.apache.org/media/uploaded/files/2015/10/15/6661fae4-32bf-409a-b975-05514aae3174__freebsd_check.log


Thanks,

David Forsythe

Re: Review Request 39345: Enable build on FreeBSD, start porting components.

[David Forsythe]

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

(Updated Oct. 15, 2015, 5:16 p.m.)


Review request for mesos and Ian Downes.


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

https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1563


Repository: mesos


Description (updated)
---

Enable build on FreeBSD, start porting components.

My build steps are:

- Install dependencies from http://mesos.apache.org/gettingstarted/
- Install libexecinfo
- Install clang36 (system clang is 3.4)
- boostrap
- ../configure --with-curl=/usr/local --with-apr=/usr/local 
--with-svn=/usr/local CC=clang36 CXX=clang++36
- gmake CC=clang36 CXX=clang++36
- gmake CC=clang36 CXX=clang++36 check

I disabled one test because I haven't had a chance to debug it and I wanted to 
get a bit further in the test suite. A check run is attached.


Diffs
-

  3rdparty/libprocess/3rdparty/Makefile.am 
e64e3d9e958fab3c7c25fad17dcc2b279d255500 
  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
d1e2df6151149e03ffb4a76e2c24ff78b891e016 
  3rdparty/libprocess/3rdparty/stout/include/stout/mac.hpp 
9428717fac4655898d7768957f02937592e1a398 
  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
3f829bafe96526bc2263c9f228f85de38c435f60 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
5141c1369af60afd6cd5c70c6295d575ea960a83 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
7eb51e8771e95f57548fc35753e75c6d56cd97cd 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
828c9c777b1b0e067c2551b79b9747a3cf4fb0aa 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/signals.hpp 
e9b05ef3b59fd068137cb12e36591de2d4a801a1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/sysctl.hpp 
8a8ede325cfe8f024e1be4db24b0c8118d18f359 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
a042d061159c83e1652e7288b90809981d2818c9 
  3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 
4cc781bddbf7ee10cc0671f62d710fb4fa80e293 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
ee2a7a79617612ed448e650cc77608c4962fe5a5 
  3rdparty/libprocess/configure.ac 7c2bcffe5c7be1f7d90e6df470d20a00245bfbff 
  3rdparty/libprocess/src/poll_socket.cpp 
28ed102972a9d8f88048aea4046ed837b6a25b35 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d13d3888abbf3db552df4a9f83e54667e598ded9 
  configure.ac 66f9b32773861d2921d99189e0fbcaea48c456a9 
  src/Makefile.am ba9feb10057a863a41a85ab3612f3a841de01df4 

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


Testing
---

make check on coreos (I'll get a ubuntu instance for future testing)

gmake check on freebsd 10.2 (failing) (log attached)


File Attachments


gmake check
  
https://reviews.apache.org/media/uploaded/files/2015/10/15/6661fae4-32bf-409a-b975-05514aae3174__freebsd_check.log


Thanks,

David Forsythe

Review Request 39350: Adopt os::pipe in libprocess.

[James Peach]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Adopt os::pipe in libprocess.


Diffs
-

  3rdparty/libprocess/src/subprocess.cpp 
a457cbe35ad33531c49f74550cd570cf28758f5d 
  3rdparty/libprocess/src/tests/io_tests.cpp 
56eb21bf92e2b50a00221777e4143c2446922c98 

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


Testing
---

OS X and CentOS 6
make && make check


Thanks,

James Peach

Re: Review Request 39259: Enable scheduler driver can use Call::REQUEST to request resource

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Oct. 13, 2015, 1:29 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39259/
> ---
> 
> (Updated Oct. 13, 2015, 1:29 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3715
> https://issues.apache.org/jira/browse/MESOS-3715
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable scheduler driver can use Call::REQUEST to request resource
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 724d7c0cef073342436de7cf5fe834fffbf0 
> 
> Diff: https://reviews.apache.org/r/39259/diff/
> 
> 
> Testing
> ---
> 
> Ubuntu 14.04 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Vinod Kone]

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

Ship it!



src/master/master.cpp (line 6022)


s/transited/transitioned/ ?



src/tests/status_update_manager_tests.cpp (line 844)


new line.



src/tests/status_update_manager_tests.cpp (lines 845 - 849)


This comment is a bit confusing. The fix in this review is for master and 
not slave/status update manager. Why is this comment talking about semantics of 
slave/status update manager?

How about?

// This test verifies that if master receives a status update for an 
already terminated
// task it forwards it without changing the state of the task.



src/tests/status_update_manager_tests.cpp (line 943)


No need for settle() here.


- Vinod Kone


On Sept. 24, 2015, 9:45 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 24, 2015, 9:45 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 6bee4f3 
>   src/tests/status_update_manager_tests.cpp 9970d71 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 1. Write a test for this change.
> 2. make successfully!
> 3. make check successfully!
> 4. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Review Request 39348: Fix signal blocking race condition on OS X.

[James Peach]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

OS X will raise SIGPIPE against the process rathr than the triggerring
thread, causing the SUPPRESS macro to sometimes fail to squash the
signal. The right way to avoid this on OS X to block SIGPIPE in the
process (which we are loth to do in libprocess), or to request that
SIGPIPE not be raised on a particular file descriptor.

  - Add os::nonSigpipe() helper to check whether SIPIPE is suppressed
on a file descriptor.
  - Add os::nosigpipe() to enable or disable SIGPIPE suppression.
  - Use os::nosigpipe() to suppress SIGPIPE when we create sockets.
  - Check and restore the SIGPIPE status in process::io::internal::write(),
which makes repeated iterations of the IOTest.Write test reliable.


Diffs
-

  3rdparty/libprocess/include/process/network.hpp 
61bfa8243728dc19ab0e6fb43d33ca83fb3709c3 
  3rdparty/libprocess/src/io.cpp 26686e1a96484e3f09d41a7292f38b7579ce9c48 

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


Testing
---

OS X and CentOS 6
make && make check


Thanks,

James Peach

Review Request 39349: Add a Stout wrapper for pipe(2).

[James Peach]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Add a Stout wrapper for pipe(2).


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
5141c1369af60afd6cd5c70c6295d575ea960a83 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/pipe.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/pipe.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pipe.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 
4bb79fdc161483cfc2b7a7a15e5c3ce62ceb493d 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
ee2a7a79617612ed448e650cc77608c4962fe5a5 

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


Testing
---

OS X and CentOS 6
make && make check


Thanks,

James Peach

Review Request 39353: Fixed and added tests for docker image name parsing.

[Ben Mahler]

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

Review request for mesos, Jojy Varghese and Timothy Chen.


Repository: mesos


Description
---

The existing docker image parsing did not work in many cases: if a digest is 
present, a registry port is included. And it could not resolve the ambiguity 
that exists between the registry and the repository.

This follows docker's approach to parsing, which is a bit hacky due to the way 
the registry and repository are disambiguated, see the comments.


Diffs
-

  include/mesos/mesos.proto f2ea4fc62507ce30b7a7981fd6a8c4749eb97190 
  src/slave/containerizer/provisioner/docker/message.hpp 
6368bf4caec6f8c3ac97282f41c55381f920bce9 
  src/slave/containerizer/provisioner/docker/message.proto 
bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
  src/slave/containerizer/provisioner/docker/store.cpp 
637c97c0973bda492826803a962c99635647692d 
  src/tests/containerizer/provisioner_docker_tests.cpp 
9c3c45a81be6398722a37911788e347a4e91cce8 

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


Testing
---

Added a test.


Thanks,

Ben Mahler

Re: Review Request 39285: Added Quota Request Validation.

[Joerg Schad]

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

(Updated Oct. 15, 2015, 6:50 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van 
Remoortere.


Changes
---

Rebased


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


Repository: mesos


Description
---

Added Quota Request Validation.


Diffs (updated)
-

  src/master/master.hpp e7b16fdd21a8caa77a39956a8520cf1381186598 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---


Thanks,

Joerg Schad

Review Request 39347: Add stout wrappers for toggling NOSIGPIPE on file descriptors.

[James Peach]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Add stout wrappers for toggling NOSIGPIPE on file descriptors.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/README.md 
d7596e58e67699c1bc9c28da841b89ccd26f3a34 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fcntl.hpp 
a9cb07ecd16f363cf2f77bc867277c737adfb68b 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/fcntl.hpp 
0bad61561ae25adb7ff6d731452577133688f8d4 

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


Testing
---

OS X and CentOS 6
make && make check


Thanks,

James Peach

Review Request 39351: Adopt os::pipe in Mesos.

[James Peach]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Adopt os::pipe in Mesos.


Diffs
-

  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/slave/containerizer/linux_launcher.cpp 
c03b89eb0678825b03a052874d6262f377a39e13 
  src/slave/containerizer/mesos/containerizer.cpp 
d1fc5a460e7313828014eea999cf4e63dde01921 
  src/tests/containerizer/cgroups_tests.cpp 
75a3bc0009c037dc18ce319db2eb44630f083e8c 
  src/tests/containerizer/isolator_tests.cpp 
237f3f27722b01ff92d0dcbaba7910613542a1a7 
  src/tests/containerizer/port_mapping_tests.cpp 
feca2043503436ac9abac6017ae9059b3fcbed21 

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


Testing
---

OS X and CentOS 6
make && make check


Thanks,

James Peach