Re: Review Request 46168: Add subdirectory support to URI.filename field.

[Jiang Yan Xu]

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


Ship it!




Looks great! I'll commit it with the following minor cleanups.


src/launcher/fetcher.cpp (line 316)


4 space indentation.



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


s/filename/path/.



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


No trailing period in error messsages.



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


No trailing period in error messsages.



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


s/filenameValidation/outputFileValiation/



src/tests/fetcher_cache_tests.cpp (line 691)


s/filename/output/



src/tests/fetcher_cache_tests.cpp (line 707)


s/Filename/OutputFile



src/tests/fetcher_cache_tests.cpp (line 736)


4 spaces



src/tests/fetcher_tests.cpp (line 135)


s/filename/path/



src/slave/containerizer/fetcher.hpp (line 70)


s/filename/output_file/



src/slave/containerizer/fetcher.hpp (line 71)


s/filename/path.


- Jiang Yan Xu


On April 21, 2016, 10:44 a.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46168/
> ---
> 
> (Updated April 21, 2016, 10:44 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jiang Yan Xu, and Zhitao Li.
> 
> 
> Bugs: MESOS-5119
> https://issues.apache.org/jira/browse/MESOS-5119
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add subdirectory support to URI.filename field.
> 
> URI.filename allows the user to specify the name of the file that will
> be saved in the sandbox when the URI is fetched, but previously it would
> fail at fetch time if "filename" had a directory component. This change
> allows users to specify a relative path for custom filenames within the
> sandbox.
> 
> 
> Diffs
> -
> 
>   CHANGELOG d2e902f8295644c527964123e409be460c2a5789 
>   docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 
>   include/mesos/mesos.proto 87af4a06db8cc3889fe4d3b314206103f5ce5f2d 
>   include/mesos/v1/mesos.proto 34da0a1484dc2f71262d8b97484b8edaae35bb6c 
>   src/launcher/fetcher.cpp 47583eeaed53b3e7ed4db26fee7cdd2fae5e0c9d 
>   src/slave/containerizer/fetcher.hpp 
> eeb663eac4c86e079228ac806018050d5d039e07 
>   src/slave/containerizer/fetcher.cpp 
> d5910ad570371ba54580be5bb94344a1de38d1f9 
>   src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e 
>   src/tests/fetcher_tests.cpp 23a8dc5f4402c5613744753284aabbe3d09bf797 
> 
> Diff: https://reviews.apache.org/r/46168/diff/
> 
> 
> Testing
> ---
> 
> Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory 
> tests that the fetcher creates a file in a specified subdirectory in the 
> sandbox, and AbsoluteCustomSubdirectoryFails tests that a custom filename 
> with an absolute path is rejected. In fetcher_cache_tests.cpp, 
> CachedCustomFilenameWithSubdirectory tests that the same behavior holds when 
> the URI is fetched from the cache.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 42806: Added the fetcher plugin module interface.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42806]

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

- Mesos ReviewBot


On April 23, 2016, 2:33 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42806/
> ---
> 
> (Updated April 23, 2016, 2:33 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3926
> https://issues.apache.org/jira/browse/MESOS-3926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the fetcher plugin module interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/module/fetcher_plugin.hpp PRE-CREATION 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/examples/test_fetcher_plugin_module.cpp PRE-CREATION 
>   src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
>   src/tests/module.hpp e661d95fa44fc1aedfe83c564c826d5b7d32c85b 
>   src/tests/module.cpp 5b83fd6358ddea4c9d849b8992e1a6040ef74505 
>   src/uri/fetcher.hpp 8af2c9122e0b15fd54f7d3a84779540e7186f566 
>   src/uri/fetcher.cpp aa9df5d0256a26b8684934c2bd37b82a069088f7 
> 
> Diff: https://reviews.apache.org/r/42806/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 46255: Added a realm parameter to 'process::initialize' (Mesos).

[Greg Mann]

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

(Updated April 23, 2016, 4:41 a.m.)


Review request for mesos, Adam B and Alexander Rojas.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

In order to enable authentication on libprocess-level
HTTP endpoints, this patch adds code to the master and
agent's main.cpp file which makes use of the new
`authenticationRealm` argument to `process::initialize`
which allows the authentication realm of such endpoints
to be set when libprocess is initialized. The argument is
added to libprocess initialization in the tests as well.


Diffs (updated)
-

  src/master/main.cpp 7bbc982192b96eed32674070e16575902af80c6d 
  src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 
  src/tests/main.cpp 142585096493a334ac9ac0df511ae0fc10798040 
  src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 

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


Testing
---

`sudo make check` on OSX.


Thanks,

Greg Mann

Re: Review Request 46259: Added authentication to `/metrics/snapshot` endpoint.

[Greg Mann]

> On April 22, 2016, 10:26 p.m., Kapil Arya wrote:
> > 3rdparty/libprocess/include/process/metrics/metrics.hpp, lines 55-58
> > 
> >
> > trailing underscore?

See my comment on https://reviews.apache.org/r/46258/


> On April 22, 2016, 10:26 p.m., Kapil Arya wrote:
> > 3rdparty/libprocess/include/process/metrics/metrics.hpp, line 67
> > 
> >
> > Any reason for not making it 'None()' by default to simplify the call 
> > site.

See my comment on https://reviews.apache.org/r/46258/: I don't think we can 
simplify the call site too much since different overloads of `route` are 
called; the authentication realm parameter only occurs in the signature of the 
authenticated overload, for example. What do you think?


- Greg


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


On April 15, 2016, 6:57 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46259/
> ---
> 
> (Updated April 15, 2016, 6:57 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds authentication to the `/metrics/snapshot`
> endpoint on the master and agent.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 7abfadf6d030c52b7e03e773b67c74db6ad5b5df 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 7677dffed98153bc6276aad492a6815e67740ea3 
>   3rdparty/libprocess/src/process.cpp 
> 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a 
> 
> Diff: https://reviews.apache.org/r/46259/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46258: Added authentication to `/logging/toggle` endpoint.

[Greg Mann]

> On April 22, 2016, 10:24 p.m., Kapil Arya wrote:
> > 3rdparty/libprocess/include/process/logging.hpp, lines 28-31
> > 
> >
> > Don't we prefere trailing underscores?

For function parameters we use leading underscores; see the section on 
"Variable Names" in the style guide.


> On April 22, 2016, 10:24 p.m., Kapil Arya wrote:
> > 3rdparty/libprocess/include/process/logging.hpp, line 59
> > 
> >
> > Just wondering if it makes more sense to have a default 'None()' here 
> > and simplify the `initialize` function?

In `initialize`, we need to call different `route` function overloads in each 
of the two `if ... else` branches, so we can't simplify that code too much by 
adding a default `None()` value here.


- Greg


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


On April 15, 2016, 6:56 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46258/
> ---
> 
> (Updated April 15, 2016, 6:56 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds authentication to the `/logging/toggle`
> HTTP endpoint on the master and agent.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/logging.hpp 
> 0c0774647d8048a9d8d395141a5864eadc64f8ef 
>   3rdparty/libprocess/src/logging.cpp 
> bdbf716ee46a459f1004f0f4b72c23aae135f347 
>   3rdparty/libprocess/src/process.cpp 
> 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a 
> 
> Diff: https://reviews.apache.org/r/46258/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 45377: Ignored docker volume when updating container volume path.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45377]

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

- Mesos ReviewBot


On April 23, 2016, 2:03 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45377/
> ---
> 
> (Updated April 23, 2016, 2:03 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored docker volume when updating container volume path.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp ebf26065326be67db435f1b097964c8ff2e1dbe0 
> 
> Diff: https://reviews.apache.org/r/45377/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46594: Added test for isolator cleanup before prepare.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [46594, 46593, 46577, 46576]

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

Error:
2016-04-23 02:47:04 URL:https://reviews.apache.org/r/46576/diff/raw/ 
[1376/1376] -> "46576.patch" [1]
error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:892
error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply

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

- Mesos ReviewBot


On April 22, 2016, 11:49 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46594/
> ---
> 
> (Updated April 22, 2016, 11:49 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Neil Conway, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for isolator cleanup before prepare.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 09742ff21513dc2570684d384b257868dd57a9ce 
> 
> Diff: https://reviews.apache.org/r/46594/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Verified that the test fails if we reverted the fix.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 42806: Added the fetcher plugin module interface.

[Shuai Lin]

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

(Updated April 23, 2016, 2:33 a.m.)


Review request for mesos and Jie Yu.


Changes
---

rebased


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


Repository: mesos


Description
---

Added the fetcher plugin module interface.


Diffs (updated)
-

  include/mesos/module/fetcher_plugin.hpp PRE-CREATION 
  src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
  src/examples/test_fetcher_plugin_module.cpp PRE-CREATION 
  src/module/manager.cpp 9f88ec3addab59e4a40b0b40612518178d535aa5 
  src/tests/module.hpp e661d95fa44fc1aedfe83c564c826d5b7d32c85b 
  src/tests/module.cpp 5b83fd6358ddea4c9d849b8992e1a6040ef74505 
  src/uri/fetcher.hpp 8af2c9122e0b15fd54f7d3a84779540e7186f566 
  src/uri/fetcher.cpp aa9df5d0256a26b8684934c2bd37b82a069088f7 

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


Testing
---

make check


Thanks,

Shuai Lin

Re: Review Request 45671: Added version checking for dvdcli.

[Guangya Liu]

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

(Updated 四月 23, 2016, 2:16 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added version checking for dvdcli.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
c63fa02b8f13014bbcac6984a49eaa919d26b489 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45377: Ignored docker volume when updating container volume path.

[Guangya Liu]

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

(Updated 四月 23, 2016, 2:03 a.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


Summary (updated)
-

Ignored docker volume when updating container volume path.


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


Repository: mesos


Description (updated)
---

Ignored docker volume when updating container volume path.


Diffs (updated)
-

  src/slave/slave.cpp ebf26065326be67db435f1b097964c8ff2e1dbe0 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46588: Added URI struct to stout.

[Joseph Wu]

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

(Updated April 22, 2016, 5:48 p.m.)


Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
Varghese.


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


Repository: mesos


Description
---

This will replace the `mesos::URI` protobuf currently used by the 
`uri::Fetcher`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
10be36652034ea5860f122ffb45ee50cc72ffdb3 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
33ddb06e25920096f2d16d4f372129ee2f6a8721 
  3rdparty/libprocess/3rdparty/stout/include/stout/uri.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/uri_tests.cpp PRE-CREATION 

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


Testing
---

make check (OSX) GTEST_FILTER="URITest*"


Ran a clean build (make check) on:

* Ubuntu 12, 14, 15
* CentOS 6, 7
* Debian 8


Thanks,

Joseph Wu

Re: Review Request 46588: Added URI struct to stout.

[Joseph Wu]

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

(Updated April 22, 2016, 5:48 p.m.)


Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
Varghese.


Changes
---

Updated previous commit, then ran this on a bunch of clean environments.


Repository: mesos


Description (updated)
---

This will replace the `mesos::URI` protobuf currently used by the 
`uri::Fetcher`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
10be36652034ea5860f122ffb45ee50cc72ffdb3 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
33ddb06e25920096f2d16d4f372129ee2f6a8721 
  3rdparty/libprocess/3rdparty/stout/include/stout/uri.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/uri_tests.cpp PRE-CREATION 

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


Testing (updated)
---

make check (OSX) GTEST_FILTER="URITest*"


Ran a clean build (make check) on:

* Ubuntu 12, 14, 15
* CentOS 6, 7
* Debian 8


Thanks,

Joseph Wu

Re: Review Request 46580: Added uriparser as a bundled dependency.

[Joseph Wu]

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

(Updated April 22, 2016, 5:47 p.m.)


Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
Varghese.


Changes
---

Ran build on some clean environments, fixed the resulting build errors.


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


Repository: mesos


Description (updated)
---

The source for `uriparser` can be found here:
http://uriparser.sourceforge.net/


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am f7aa579 
  3rdparty/libprocess/3rdparty/uriparser-0.8.4.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 5506eb3 
  3rdparty/libprocess/Makefile.am c51c31e 
  3rdparty/libprocess/configure.ac d27e46e 

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


Testing (updated)
---

On OSX:

../configure
make
make check


Thanks,

Joseph Wu

Re: Review Request 46220: Added documentation for Nvidia GPU support.

[Qian Zhang]

> On April 22, 2016, 5:55 p.m., Qian Zhang wrote:
> > docs/gpu-support.md, line 440
> > 
> >
> > Does this limitation mean that currently we do not support container 
> > with an image (e.g., Docker image, Appc image)? Instead we only support 
> > image-less container since such kind of containers will be able to directly 
> > access agent host filesystem so that they can access the needed GPU library 
> > to perform GPU-aware jobs.
> 
> Kevin Klues wrote:
> Yes. This is what it means currently.  However, I am planning to remove 
> this restriction before the 0.29 release.

Can you please let me know a bit more about why removing this restriction 
before 0.29 release? Do we already have a solution to make the container with 
an image can access the GPU library?


- Qian


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


On April 15, 2016, 4:33 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46220/
> ---
> 
> (Updated April 15, 2016, 4:33 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-5221
> https://issues.apache.org/jira/browse/MESOS-5221
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for Nvidia GPU support.
> 
> 
> Diffs
> -
> 
>   docs/gpu-support.md PRE-CREATION 
>   docs/home.md dc41fc4479e6c23650cd8ac78dcc4b9161d00721 
> 
> Diff: https://reviews.apache.org/r/46220/diff/
> 
> 
> Testing
> ---
> 
> Ran a local copy of the documentation site to make sure everything looks OK.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 46580: Added uriparser as a bundled dependency.

[Joseph Wu]

> On April 22, 2016, 2:14 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/configure.ac, line 892
> > 
> >
> > Is this some specicial autoconf thing? I don't see it being used...

Oops, that's a leftover from a more rigorous check I was considering.  (i.e. 
checking the version of `uriparser`)  Forgot to remove it when I just opted to 
check the header.


> On April 22, 2016, 2:14 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/Makefile.am, lines 189-191
> > 
> >
> > Not yours, but I don't know why those are needed.

Hmm... yeah, these shouldn't be necessary with these lines (143-147) in the 
same file:
```
libprocess_la_LIBADD =  \
  $(LIBGLOG)\
  $(HTTP_PARSER_LIB)\
  $(EVENT_LIB)  \
  $(LIBURIPARSER)
```


> On April 22, 2016, 2:14 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/Makefile.am, line 74
> > 
> >
> > Can you fix the tailing `\` alignment?

Added a tab (to that blob), so that the backslashes line up with the other ones 
in the file.


- Joseph


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


On April 22, 2016, 12:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46580/
> ---
> 
> (Updated April 22, 2016, 12:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
> Varghese.
> 
> 
> Bugs: MESOS-5254
> https://issues.apache.org/jira/browse/MESOS-5254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am f7aa579 
>   3rdparty/libprocess/3rdparty/uriparser-0.8.4.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 5506eb3 
>   3rdparty/libprocess/Makefile.am c51c31e 
>   3rdparty/libprocess/configure.ac d27e46e 
> 
> Diff: https://reviews.apache.org/r/46580/diff/
> 
> 
> Testing
> ---
> 
> ../configure
> make
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 46588: Added URI struct to stout.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [46588, 46580]

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

Error:
2016-04-23 00:39:55 URL:https://reviews.apache.org/r/46588/diff/raw/ 
[9453/9453] -> "46588.patch" [1]
Total errors found: 0
Checking 2 files
Error: No line in the commit message summary may exceed 72 characters.

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

- Mesos ReviewBot


On April 22, 2016, 11:39 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46588/
> ---
> 
> (Updated April 22, 2016, 11:39 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
> Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will replace the `mesos::URI` protobuf currently used by the 
> `uri::Fetcher`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 10be36652034ea5860f122ffb45ee50cc72ffdb3 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 33ddb06e25920096f2d16d4f372129ee2f6a8721 
>   3rdparty/libprocess/3rdparty/stout/include/stout/uri.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/uri_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46588/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX) GTEST_FILTER="URITest*"
> 
> TODO: Run build on a clean environment.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45370: Implemented prepare() for volume isolator.

[Guangya Liu]

> On 四月 22, 2016, 6:54 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp, line 242
> > 
> >
> > YOu can iterate through `volumeInfos` here.
> 
> Guangya Liu wrote:
> You mean `foreach` is not good for this?

Got it, I already created the `volumeInfos` above, so can use it directly.


- Guangya


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


On 四月 22, 2016, 4:08 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45370/
> ---
> 
> (Updated 四月 22, 2016, 4:08 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() for volume isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
>   src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526 
> 
> Diff: https://reviews.apache.org/r/45370/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> Will update `Sequence` later.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44454: Enabled Sequence mount for prepare().

[Jie Yu]

> On April 22, 2016, 9:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp, line 109
> > 
> >
> > Please use a Sequence for each DockerVolume. umount A and mount B can 
> > happen in parallel as the ordering does not matter.
> 
> Guangya Liu wrote:
> One question is if define `sequence` for each volume, then can we 
> guarantee that two different volumes can always keep the order?
> 
> e.g.
> Volume 1) driver: convoy, name: dvd1
> Volume 2) driver: flocker, name: dvd2
> 
> One container using the above two different volumes, can we guarantee 
> volume 1 mount point return first than volume 2 if letting differene sequence 
> manage those two volumes?
> 
> Jie Yu wrote:
> why does that matter?
> 
> Guangya Liu wrote:
> For such case:
> 
> Volume 1) driver: convoy, name: dvd1
> Volume 2) driver: flocker, name: dvd2
> 
> Volume 1 map to /tmp/abc1
> Volume 2 map to /tmp/abc2
> 
> If the return value of volume 1 and 2 mount point is not right, then 
> volume 1 mount point will mount to /tmp/abc1 and volume 2 will mount to 
> /tmp/abc2
> 
> I'm just not sure if using different volume sequence can guarantee the 
> order?
> 
> Guangya Liu wrote:
> Correct a typo: If the return value of volume 1 and 2 mount point is not 
> right, then volume 1 mount point will mount to `/tmp/abc2` and volume 2 will 
> mount to `/tmp/abc1`

I don't think that's an issue. I don't think a driver will return an incorrect 
mount point for a volume (otherwise, it's a bug).


- Jie


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


On April 22, 2016, 4:11 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44454/
> ---
> 
> (Updated April 22, 2016, 4:11 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled Sequence mount for prepare().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
> 
> Diff: https://reviews.apache.org/r/44454/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44454: Enabled Sequence mount for prepare().

[Guangya Liu]

> On 四月 22, 2016, 9:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp, line 109
> > 
> >
> > Please use a Sequence for each DockerVolume. umount A and mount B can 
> > happen in parallel as the ordering does not matter.
> 
> Guangya Liu wrote:
> One question is if define `sequence` for each volume, then can we 
> guarantee that two different volumes can always keep the order?
> 
> e.g.
> Volume 1) driver: convoy, name: dvd1
> Volume 2) driver: flocker, name: dvd2
> 
> One container using the above two different volumes, can we guarantee 
> volume 1 mount point return first than volume 2 if letting differene sequence 
> manage those two volumes?
> 
> Jie Yu wrote:
> why does that matter?
> 
> Guangya Liu wrote:
> For such case:
> 
> Volume 1) driver: convoy, name: dvd1
> Volume 2) driver: flocker, name: dvd2
> 
> Volume 1 map to /tmp/abc1
> Volume 2 map to /tmp/abc2
> 
> If the return value of volume 1 and 2 mount point is not right, then 
> volume 1 mount point will mount to /tmp/abc1 and volume 2 will mount to 
> /tmp/abc2
> 
> I'm just not sure if using different volume sequence can guarantee the 
> order?

Correct a typo: If the return value of volume 1 and 2 mount point is not right, 
then volume 1 mount point will mount to `/tmp/abc2` and volume 2 will mount to 
`/tmp/abc1`


- Guangya


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


On 四月 22, 2016, 4:11 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44454/
> ---
> 
> (Updated 四月 22, 2016, 4:11 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled Sequence mount for prepare().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
> 
> Diff: https://reviews.apache.org/r/44454/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44454: Enabled Sequence mount for prepare().

[Guangya Liu]

> On 四月 22, 2016, 9:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp, line 109
> > 
> >
> > Please use a Sequence for each DockerVolume. umount A and mount B can 
> > happen in parallel as the ordering does not matter.
> 
> Guangya Liu wrote:
> One question is if define `sequence` for each volume, then can we 
> guarantee that two different volumes can always keep the order?
> 
> e.g.
> Volume 1) driver: convoy, name: dvd1
> Volume 2) driver: flocker, name: dvd2
> 
> One container using the above two different volumes, can we guarantee 
> volume 1 mount point return first than volume 2 if letting differene sequence 
> manage those two volumes?
> 
> Jie Yu wrote:
> why does that matter?

For such case:

Volume 1) driver: convoy, name: dvd1
Volume 2) driver: flocker, name: dvd2

Volume 1 map to /tmp/abc1
Volume 2 map to /tmp/abc2

If the return value of volume 1 and 2 mount point is not right, then volume 1 
mount point will mount to /tmp/abc1 and volume 2 will mount to /tmp/abc2

I'm just not sure if using different volume sequence can guarantee the order?


- Guangya


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


On 四月 22, 2016, 4:11 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44454/
> ---
> 
> (Updated 四月 22, 2016, 4:11 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled Sequence mount for prepare().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
> 
> Diff: https://reviews.apache.org/r/44454/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45370: Implemented prepare() for volume isolator.

[Jie Yu]

> On April 22, 2016, 6:54 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp, line 77
> > 
> >
> > Should that be a hashset given that we don't allow duplicate?
> 
> Guangya Liu wrote:
> The problem is that `hashset` do not support using `DockerVolumeInfo` as 
> its value, so I was using a vector instead, any comments?

yeah, vector sounds fine.


- Jie


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


On April 22, 2016, 4:08 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45370/
> ---
> 
> (Updated April 22, 2016, 4:08 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() for volume isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
>   src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526 
> 
> Diff: https://reviews.apache.org/r/45370/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> Will update `Sequence` later.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44454: Enabled Sequence mount for prepare().

[Jie Yu]

> On April 22, 2016, 9:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp, line 109
> > 
> >
> > Please use a Sequence for each DockerVolume. umount A and mount B can 
> > happen in parallel as the ordering does not matter.
> 
> Guangya Liu wrote:
> One question is if define `sequence` for each volume, then can we 
> guarantee that two different volumes can always keep the order?
> 
> e.g.
> Volume 1) driver: convoy, name: dvd1
> Volume 2) driver: flocker, name: dvd2
> 
> One container using the above two different volumes, can we guarantee 
> volume 1 mount point return first than volume 2 if letting differene sequence 
> manage those two volumes?

why does that matter?


- Jie


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


On April 22, 2016, 4:11 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44454/
> ---
> 
> (Updated April 22, 2016, 4:11 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled Sequence mount for prepare().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
> 
> Diff: https://reviews.apache.org/r/44454/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 46594: Added test for isolator cleanup before prepare.

[Gilbert Song]

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

Review request for mesos, Artem Harutyunyan, Jie Yu, Neil Conway, and Timothy 
Chen.


Repository: mesos


Description
---

Added test for isolator cleanup before prepare.


Diffs
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
09742ff21513dc2570684d384b257868dd57a9ce 

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


Testing
---

make check

Verified that the test fails if we reverted the fix.


Thanks,

Gilbert Song

Review Request 46593: Added test for containerizer destroy while provisioning race.

[Gilbert Song]

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

Review request for mesos, Artem Harutyunyan, Jie Yu, Neil Conway, and Timothy 
Chen.


Repository: mesos


Description
---

Added test for containerizer destroy while provisioning race.


Diffs
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
09742ff21513dc2570684d384b257868dd57a9ce 

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


Testing
---

make check

Verified that the test is 90% chance to be segfault if we reverted the fix.


Thanks,

Gilbert Song

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Guangya Liu]

> On 四月 22, 2016, 10:20 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp, lines 
> > 181-185
> > 
> >
> > Please do not copy the comments blindly. The comments here does not 
> > apply to this isolator. Please remove it.

Seems this comment also fit to the `_recover` here well, as the `_recover` is 
also adding `Info` to `infos` and will not add it if the checkpoint directory 
was deleted.


- Guangya


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


On 四月 22, 2016, 7:15 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated 四月 22, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f991743f1a467f24a786e925983390a411792327 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
> bc0c3299f9f3e351aaa20652b784bccbfd40c1a5 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 46588: Added URI struct to stout.

[Joseph Wu]

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

Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
Varghese.


Repository: mesos


Description
---

This will replace the `mesos::URI` protobuf currently used by the 
`uri::Fetcher`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
10be36652034ea5860f122ffb45ee50cc72ffdb3 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
33ddb06e25920096f2d16d4f372129ee2f6a8721 
  3rdparty/libprocess/3rdparty/stout/include/stout/uri.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/uri_tests.cpp PRE-CREATION 

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


Testing
---

make check (OSX) GTEST_FILTER="URITest*"

TODO: Run build on a clean environment.


Thanks,

Joseph Wu

Re: Review Request 44454: Enabled Sequence mount for prepare().

[Guangya Liu]

> On 四月 22, 2016, 9:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp, line 109
> > 
> >
> > Please use a Sequence for each DockerVolume. umount A and mount B can 
> > happen in parallel as the ordering does not matter.

One question is if define `sequence` for each volume, then can we guarantee 
that two different volumes can always keep the order?

e.g.
Volume 1) driver: convoy, name: dvd1
Volume 2) driver: flocker, name: dvd2

One container using the above two different volumes, can we guarantee volume 1 
mount point return first than volume 2 if letting differene sequence manage 
those two volumes?


- Guangya


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


On 四月 22, 2016, 4:11 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44454/
> ---
> 
> (Updated 四月 22, 2016, 4:11 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled Sequence mount for prepare().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
> 
> Diff: https://reviews.apache.org/r/44454/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45370: Implemented prepare() for volume isolator.

[Guangya Liu]

> On 四月 22, 2016, 6:54 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp, line 77
> > 
> >
> > Should that be a hashset given that we don't allow duplicate?

The problem is that `hashset` do not support using `DockerVolumeInfo` as its 
value, so I was using a vector instead, any comments?


> On 四月 22, 2016, 6:54 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp, line 242
> > 
> >
> > YOu can iterate through `volumeInfos` here.

You mean `foreach` is not good for this?


- Guangya


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


On 四月 22, 2016, 4:08 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45370/
> ---
> 
> (Updated 四月 22, 2016, 4:08 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented prepare() for volume isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
>   src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526 
> 
> Diff: https://reviews.apache.org/r/45370/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> Will update `Sequence` later.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45275: Enabled "--explicitcreate" when call "dvdcli mount".

[Guangya Liu]

> On 四月 22, 2016, 5:12 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp, lines 
> > 56-65
> > 
> >
> > I think what we should do here is:
> > 
> > 1) If 'options' is None, we add `--explicitcreate=true`
> > 
> > 2) Otherwise, we don't
> > 
> > 3) Added a TODO to check dvdcli version

One question for 1) and 2), why only add `--explicitcreate=true` when `options` 
is None? Why not always enable this parameter? 

For 3), my thinking is that we only support version 0.2.0 which support the 
parameter of `--explicitcreate=true`, so the version checking logic will be if 
the version is not 0.2.0, return error.


- Guangya


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


On 四月 22, 2016, 2:29 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45275/
> ---
> 
> (Updated 四月 22, 2016, 2:29 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5251
> https://issues.apache.org/jira/browse/MESOS-5251
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled "--explicitcreate" when call "dvdcli mount".
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
> c63fa02b8f13014bbcac6984a49eaa919d26b489 
> 
> Diff: https://reviews.apache.org/r/45275/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46259: Added authentication to `/metrics/snapshot` endpoint.

[Kapil Arya]

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




3rdparty/libprocess/include/process/metrics/metrics.hpp (lines 55 - 58)


trailing underscore?



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


Any reason for not making it 'None()' by default to simplify the call site.


- Kapil Arya


On April 15, 2016, 2:57 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46259/
> ---
> 
> (Updated April 15, 2016, 2:57 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds authentication to the `/metrics/snapshot`
> endpoint on the master and agent.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 7abfadf6d030c52b7e03e773b67c74db6ad5b5df 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 7677dffed98153bc6276aad492a6815e67740ea3 
>   3rdparty/libprocess/src/process.cpp 
> 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a 
> 
> Diff: https://reviews.apache.org/r/46259/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46577: Fixed isolator cleaup issue when destroying a provisioning container.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 22, 2016, 6:21 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46577/
> ---
> 
> (Updated April 22, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Neil Conway, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-5253
> https://issues.apache.org/jira/browse/MESOS-5253
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed isolator cleaup issue when destroying a provisioning container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1e1a36903f4377497bb72b69e4ead63675d453c0 
> 
> Diff: https://reviews.apache.org/r/46577/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 46258: Added authentication to `/logging/toggle` endpoint.

[Kapil Arya]

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


Fix it, then Ship it!




Ship It!


3rdparty/libprocess/include/process/logging.hpp (lines 28 - 31)


Don't we prefere trailing underscores?



3rdparty/libprocess/include/process/logging.hpp (line 59)


Just wondering if it makes more sense to have a default 'None()' here and 
simplify the `initialize` function?


- Kapil Arya


On April 15, 2016, 2:56 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46258/
> ---
> 
> (Updated April 15, 2016, 2:56 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds authentication to the `/logging/toggle`
> HTTP endpoint on the master and agent.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/logging.hpp 
> 0c0774647d8048a9d8d395141a5864eadc64f8ef 
>   3rdparty/libprocess/src/logging.cpp 
> bdbf716ee46a459f1004f0f4b72c23aae135f347 
>   3rdparty/libprocess/src/process.cpp 
> 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a 
> 
> Diff: https://reviews.apache.org/r/46258/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46576: Fixed a mesos containerizer race destroy while preparing.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 22, 2016, 6:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46576/
> ---
> 
> (Updated April 22, 2016, 6:20 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Neil Conway.
> 
> 
> Bugs: MESOS-5238
> https://issues.apache.org/jira/browse/MESOS-5238
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a mesos containerizer race destroy while preparing.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1e1a36903f4377497bb72b69e4ead63675d453c0 
> 
> Diff: https://reviews.apache.org/r/46576/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45671: Added version checking for dvdcli.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 77 - 
103)


Let's put the check into DriverClient::create


- Jie Yu


On April 22, 2016, 9:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45671/
> ---
> 
> (Updated April 22, 2016, 9:09 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added version checking for dvdcli.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
> 
> Diff: https://reviews.apache.org/r/45671/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45674: Implemented recover() for dvd isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 120 - 
121)


Copy paste error? Please remove the comments here.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 181 - 
185)


Please do not copy the comments blindly. The comments here does not apply 
to this isolator. Please remove it.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 218)


no need to introduce a separate file and parse funcition. Simply put the 
parsing logic here.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 225)


s/dockerVolumes/volumes/



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 227)


This is no longer needed per my comments in ealier patches.


- Jie Yu


On April 22, 2016, 7:15 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45674/
> ---
> 
> (Updated April 22, 2016, 7:15 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5104
> https://issues.apache.org/jira/browse/MESOS-5104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented recover() for dvd isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt f991743f1a467f24a786e925983390a411792327 
>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
> bc0c3299f9f3e351aaa20652b784bccbfd40c1a5 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45674/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46255: Added a realm parameter to 'process::initialize' (Mesos).

[Kapil Arya]

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




src/slave/main.cpp (line 232)


slave->agent?



src/tests/main.cpp (lines 34 - 35)


Alphabetize?



src/tests/main.cpp (line 79)


Why not check the return value here too?


- Kapil Arya


On April 21, 2016, 4:39 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46255/
> ---
> 
> (Updated April 21, 2016, 4:39 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4951
> https://issues.apache.org/jira/browse/MESOS-4951
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to enable authentication on libprocess-level
> HTTP endpoints, this patch adds code to the master and
> agent's main.cpp file which makes use of the new
> `authenticationRealm` argument to `process::initialize`
> which allows the authentication realm of such endpoints
> to be set when libprocess is initialized. The argument is
> added to libprocess initialization in the tests as well.
> 
> 
> Diffs
> -
> 
>   src/master/main.cpp 7bbc982192b96eed32674070e16575902af80c6d 
>   src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 
>   src/tests/main.cpp 142585096493a334ac9ac0df511ae0fc10798040 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
> 
> Diff: https://reviews.apache.org/r/46255/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46254: Added a realm parameter to `process::initialize` (libprocess).

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On April 21, 2016, 4:21 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46254/
> ---
> 
> (Updated April 21, 2016, 4:21 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-4951
> https://issues.apache.org/jira/browse/MESOS-4951
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to enable authentication on libprocess-level
> HTTP endpoints, this patch adds a parameter to
> process::initialize which allows the authentication
> realm of such endpoints to be set when libprocess is
> initialized.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gtest.hpp 
> 3e0887257e1484813b3547170a5a1b9bb89de0d2 
>   3rdparty/libprocess/include/process/process.hpp 
> 77e96957ca556cf9a7e2bf427d6762572fe48c51 
>   3rdparty/libprocess/src/process.cpp 
> 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a 
>   3rdparty/libprocess/src/tests/main.cpp 
> 78858a2b84a439d8f8a60ec8bcb6ac3a308087a6 
> 
> Diff: https://reviews.apache.org/r/46254/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46577: Fixed isolator cleaup issue when destroying a provisioning container.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46576, 46577]

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

- Mesos ReviewBot


On April 22, 2016, 6:21 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46577/
> ---
> 
> (Updated April 22, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Neil Conway, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-5253
> https://issues.apache.org/jira/browse/MESOS-5253
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed isolator cleaup issue when destroying a provisioning container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1e1a36903f4377497bb72b69e4ead63675d453c0 
> 
> Diff: https://reviews.apache.org/r/46577/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45375: Implemented cleanup() for docker volume isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 370)


s/volumeReference/references/



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 372)


const Owned&

Please make sure to use const ref if possible in all other places.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 373 - 
374)


`foreach (const DockerVolume& volume, info->volumes)`



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 375 - 
378)


This is not necessary if you define a hash function as I mentioned in the 
earlier patch.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 392)


s/dockerVolumeInfo/volume/



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 394 - 
395)


No needed.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 402)


You can avoid this loop. Just combine with the loop above.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 408)


"Unmounting ..."



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 412 - 
414)


This is not needed any more.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 456)


for container 


- Jie Yu


On April 22, 2016, 4:13 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45375/
> ---
> 
> (Updated April 22, 2016, 4:13 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented cleanup() for docker volume isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
> 
> Diff: https://reviews.apache.org/r/45375/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46395: Windows: Removed `std::bind` from `process.cpp` to build on Windows.

[Joris Van Remoortere]

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


Fix it, then Ship it!





3rdparty/libprocess/src/process.cpp (lines 13 - 40)


Please leave these in the original order and use the smaller `#ifndef 
__WINDOWS__` blocks where needed. I think the 3? places necessary are still 
clean enough to keep the original ordering.



3rdparty/libprocess/src/process.cpp (line 888)


space between `(char*)` and ``


- Joris Van Remoortere


On April 19, 2016, 4:35 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46395/
> ---
> 
> (Updated April 19, 2016, 4:35 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3637
> https://issues.apache.org/jira/browse/MESOS-3637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Removed `std::bind` from `process.cpp` to build on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> d2c458ed93307f75358bb642aaf2ed8e17b2fe97 
> 
> Diff: https://reviews.apache.org/r/46395/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44454: Enabled Sequence mount for prepare().

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp (line 109)


Please use a Sequence for each DockerVolume. umount A and mount B can 
happen in parallel as the ordering does not matter.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 129 - 
132)


I would reformat:
```
return sequences[...].add(
defer(PID<...>(this), [=]() {
  return _mount(driver, name, options);
}));
```


- Jie Yu


On April 22, 2016, 4:11 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44454/
> ---
> 
> (Updated April 22, 2016, 4:11 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled Sequence mount for prepare().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> bedc687cc280d0b721fb84801039fd3614364cca 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 915e5ae755a55a02b7dfcda88165f27346cad955 
> 
> Diff: https://reviews.apache.org/r/44454/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46344: Libprocess: Created Winsock class to handle WSAStartup/WSACleanup.

[Joris Van Remoortere]

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




3rdparty/libprocess/include/process/windows/winsock.hpp (line 28)


brace on new line.



3rdparty/libprocess/include/process/windows/winsock.hpp (line 31)


no need for space between `{ }`



3rdparty/libprocess/include/process/windows/winsock.hpp (line 38)


can you put the initialization on a new line:
```
Class(
arg1,
arg2)
  : arg1(arg1)
{
}
```
brace on new line.



3rdparty/libprocess/include/process/windows/winsock.hpp (line 44)


brace on new line.



3rdparty/libprocess/include/process/windows/winsock.hpp (lines 49 - 51)


members below functions.



3rdparty/libprocess/include/process/windows/winsock.hpp (line 51)


holding a reference to a lambda like this that is not executed immediately 
is extremely dangerous.
Please take this by value.



3rdparty/libprocess/include/process/windows/winsock.hpp (line 56)


brace on new line.



3rdparty/libprocess/include/process/windows/winsock.hpp (lines 56 - 68)


Why not use the `Once` pattern used for initialization in the rest of the 
code base.
Take a look at `process.cpp`



3rdparty/libprocess/include/process/windows/winsock.hpp (line 74)


brace on new line.


- Joris Van Remoortere


On April 18, 2016, 7:21 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46344/
> ---
> 
> (Updated April 18, 2016, 7:21 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3656
> https://issues.apache.org/jira/browse/MESOS-3656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Libprocess: Created Winsock class to handle WSAStartup/WSACleanup.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> 47f5347988a61140c87bcd329e25d5a4d52e17a0 
>   3rdparty/libprocess/include/process/windows/winsock.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46344/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 46550: Fixed slave to initialize libprocess before modules.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 22, 2016, 1:41 a.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46550/
> ---
> 
> (Updated April 22, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Adam B, Kapil Arya, Joseph Wu, Niklas Nielsen, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is important because otherwise a module might call
> libprocess `initialize()` before slave calls `initialize(id)`.
> If this happens the  HTTP request process "delegate" will be wrong,
> 
> 
> Diffs
> -
> 
>   src/slave/main.cpp 38bd005 
> 
> Diff: https://reviews.apache.org/r/46550/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andy Pang
> 
>

Re: Review Request 46580: Added uriparser as a bundled dependency.

[Jie Yu]

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


Fix it, then Ship it!





3rdparty/libprocess/3rdparty/Makefile.am (line 74)


Can you fix the tailing `\` alignment?



3rdparty/libprocess/Makefile.am (lines 189 - 191)


Not yours, but I don't know why those are needed.



3rdparty/libprocess/Makefile.am (lines 215 - 217)


Ditto.



3rdparty/libprocess/configure.ac (line 892)


Is this some specicial autoconf thing? I don't see it being used...


- Jie Yu


On April 22, 2016, 7:24 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46580/
> ---
> 
> (Updated April 22, 2016, 7:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
> Varghese.
> 
> 
> Bugs: MESOS-5254
> https://issues.apache.org/jira/browse/MESOS-5254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am f7aa579 
>   3rdparty/libprocess/3rdparty/uriparser-0.8.4.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 5506eb3 
>   3rdparty/libprocess/Makefile.am c51c31e 
>   3rdparty/libprocess/configure.ac d27e46e 
> 
> Diff: https://reviews.apache.org/r/46580/diff/
> 
> 
> Testing
> ---
> 
> ../configure
> make
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 46341: Stout:[2/2] Transitioned reap.cpp to `os::waitpid`.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On April 20, 2016, 4:37 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46341/
> ---
> 
> (Updated April 20, 2016, 4:37 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4466
> https://issues.apache.org/jira/browse/MESOS-4466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout:[2/2] Transitioned reap.cpp to `os::waitpid`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/reap.cpp 110386842d5eeccfbc4e48bfca88d79a9d087fb0 
> 
> Diff: https://reviews.apache.org/r/46341/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 46472: Reran `generate-endpoint-help.py` script for endpoints redirection.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 22, 2016, 5:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46472/
> ---
> 
> (Updated April 22, 2016, 5:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reran `generate-endpoint-help.py` script for endpoints redirection.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/index.md d57239edb1d2708452ce2f687634227d50bbafc4 
>   docs/endpoints/master/api/v1/scheduler.md 
> f647dc0c0d9023ec9b21371db4fb8c28c64b0ef7 
>   docs/endpoints/master/create-volumes.md 
> 5c86480b488e2722a12ab09b0a28399402410e2b 
>   docs/endpoints/master/destroy-volumes.md 
> f75dd52bd6b75d35aa62dca54148a41cfec36ded 
>   docs/endpoints/master/frameworks.md 
> 15ecabfdd87faee6a1f24895088a3c08edf62b96 
>   docs/endpoints/master/machine/down.md 
> f7e81417aa663e287a61124b351f2461b1c106e6 
>   docs/endpoints/master/machine/up.md 
> d96be726f5cf524937e3ebc5b046bf5f47886a1c 
>   docs/endpoints/master/maintenance/schedule.md 
> 3d69900ffb376d64b463e7c01f349a4eb5d2f924 
>   docs/endpoints/master/maintenance/status.md 
> 249dae2706ccf30d79e0cd2f19821acd6ada74b7 
>   docs/endpoints/master/quota.md b1e3462bd35c5acfd65323db5d0660277b110b11 
>   docs/endpoints/master/redirect.md 833d9935b8fb038977fe08d826d73f03f2059bab 
>   docs/endpoints/master/reserve.md 8f38cc0f9c16c368ee3698ec0a6e7fd484110ebc 
>   docs/endpoints/master/roles.json.md 
> 01e265330c4c30de11301a2c4abbddfee10ac35d 
>   docs/endpoints/master/roles.md 9c01d370a86d77eb95fcc157de70304e73f5f1e9 
>   docs/endpoints/master/slaves.md 9e9fff7329d6bf05ae78997b1292657ff4dd10c3 
>   docs/endpoints/master/state-summary.md 
> a0da400b34c202602cb661a19c6199291da4ff4b 
>   docs/endpoints/master/state.json.md 
> 7998b80f29042f4b67565b03e2464b6e3a009335 
>   docs/endpoints/master/state.md 59518d624a0c5e010f856c900a9d6512a35b21af 
>   docs/endpoints/master/tasks.json.md 
> 29bb9738b1519174eaeac4f4db423000fe48d0d3 
>   docs/endpoints/master/tasks.md ab9bb09631400668adddeee363dd612c71e98e16 
>   docs/endpoints/master/teardown.md 9b62b2656162b4160a9845152aecf7b75231fbae 
>   docs/endpoints/master/unreserve.md 8e9a696a50f1821cfb11afae68cc4fb62b6f2a2c 
>   docs/endpoints/master/weights.md 7c4517343aa61bf6156fa54547690930051616a3 
>   docs/endpoints/slave/health.md 301a851035459c63a844c9a7ffd1f65ef39508af 
>   docs/endpoints/slave/monitor/statistics.json.md 
> f409a21315c99d0ee16ad73b83b9ddc4dcca4b8d 
>   docs/endpoints/slave/monitor/statistics.md 
> ec9f095456ac86ec31c65c7ca80b21553ec38edb 
>   docs/endpoints/slave/state.json.md 92cd4e19ede2d52a6f0e25c7c08cc67632f30b60 
>   docs/endpoints/slave/state.md 977746657b70aed531e2ef803c767d7e50561119 
> 
> Diff: https://reviews.apache.org/r/46472/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46391: Clarified several agent log messages.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46391]

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

- Mesos ReviewBot


On April 22, 2016, 3:28 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46391/
> ---
> 
> (Updated April 22, 2016, 3:28 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When diagnosing problems, it can be useful to distinguish
> between executors that are "terminating" vs. "terminated".
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526 
> 
> Diff: https://reviews.apache.org/r/46391/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 46364: Change to explicit case statements (`UNKNOWN`) instead of default.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 22, 2016, 5:28 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46364/
> ---
> 
> (Updated April 22, 2016, 5:28 a.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-5014 and MESOS-5015
> https://issues.apache.org/jira/browse/MESOS-5014
> https://issues.apache.org/jira/browse/MESOS-5015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes from `default` statement to explicit case
> statement (`UNKNOWN`) so that complier could help catching all
> switches when new enums are introduced in the future.
> 
> This patch is related to:
> https://reviews.apache.org/r/45317/ (MESOS-5014)
> https://reviews.apache.org/r/45304/ (MESOS-5015)
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
>   src/examples/long_lived_executor.cpp 
> ebb9cbf578d79fd87ac51ea85575950b7c0a1504 
>   src/examples/long_lived_framework.cpp 
> c696ccb6b2ace6e047f6509b291dd14be240cf70 
>   src/examples/test_http_executor.cpp 
> ceb489d43f35d24c8a7f5fbb0148529745ee357a 
>   src/examples/test_http_framework.cpp 
> 8cc3107034d46cb6a2966835f509508223c6e674 
>   src/launcher/http_command_executor.cpp 
> 9dfe48108cababeb2d4a6af74434880d79245c21 
>   src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 
>   src/master/master.cpp 6dacf5fbd73771e5c31ffb0e8723cd2905dcefb3 
>   src/master/validation.cpp 0cd118ee4f89f749a063f6ba7f419d5a220dc1d4 
>   src/sched/sched.cpp 5393f0de655d03ac1a31b3144d4d764d4aeb56fb 
>   src/slave/http.cpp f887a71684304a82ff0d81dbd240e29aa7e2ac67 
>   src/slave/validation.cpp ec1a4b6d9c55ab0c9894d5a49e290e15dee32c22 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
> 
> Diff: https://reviews.apache.org/r/46364/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 22, 2016, 5:58 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46314/
> ---
> 
> (Updated April 22, 2016, 5:58 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen and Vinod Kone.
> 
> 
> Bugs: MESOS-4678
> https://issues.apache.org/jira/browse/MESOS-4678
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds the upstream patch `717f807` which is contained in
> `protobuf-3.0.0-alpha-1`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 7b3621d24a05a3b3b860e3b8c13a1531208e5e26 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 895663b0095cfb1e682da90606c34a96ad036834 
>   3rdparty/libprocess/3rdparty/protobuf-2.6.1.patch PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46314/diff/
> 
> 
> Testing
> ---
> 
> Building with GCC6 w/o this patch leads to a hard failure to a comparison 
> between a signed and unsigned types; with this patch the build succeeds.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46391: Clarified several agent log messages.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 22, 2016, 3:28 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46391/
> ---
> 
> (Updated April 22, 2016, 3:28 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When diagnosing problems, it can be useful to distinguish
> between executors that are "terminating" vs. "terminated".
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526 
> 
> Diff: https://reviews.apache.org/r/46391/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 46469: Added support for credentials to mesos-execute.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 22, 2016, 3:15 p.m., Tim Anderegg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46469/
> ---
> 
> (Updated April 22, 2016, 3:15 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3402
> https://issues.apache.org/jira/browse/MESOS-3402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for credentials to mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
> 
> Diff: https://reviews.apache.org/r/46469/diff/
> 
> 
> Testing
> ---
> 
> $ ./mesos-execute --master=127.0.0.1:5050 --name=auth_test --command="while 
> true; do echo 'Hello'; sleep 2; done"
> I0420 21:55:11.347501  2751 scheduler.cpp:177] Version: 0.29.0
> Received an ERROR event: Received unexpected '401 Unauthorized' () for 
> SUBSCRIBE
> 
> $ ./mesos-execute --master=127.0.0.1:5050 --name=auth_test --command="while 
> true; do echo 'Hello'; sleep 2; done" --principal=principal --secret=secret
> I0420 21:54:02.924027  2587 scheduler.cpp:177] Version: 0.29.0
> Subscribed with ID '81fe5703-ccae-4c6e-8e39-a246109d812e-0001'
> Submitted task 'auth_test' to agent '9edb64fe-5b97-43d3-81fe-81f0cdcea0b4-S0'
> Received status update TASK_RUNNING for task 'auth_test'
>   source: SOURCE_EXECUTOR
> 
> 
> Thanks,
> 
> Tim Anderegg
> 
>

Re: Review Request 46469: Added support for credentials to mesos-execute.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46469]

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

- Mesos ReviewBot


On April 22, 2016, 3:15 p.m., Tim Anderegg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46469/
> ---
> 
> (Updated April 22, 2016, 3:15 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3402
> https://issues.apache.org/jira/browse/MESOS-3402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for credentials to mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
> 
> Diff: https://reviews.apache.org/r/46469/diff/
> 
> 
> Testing
> ---
> 
> $ ./mesos-execute --master=127.0.0.1:5050 --name=auth_test --command="while 
> true; do echo 'Hello'; sleep 2; done"
> I0420 21:55:11.347501  2751 scheduler.cpp:177] Version: 0.29.0
> Received an ERROR event: Received unexpected '401 Unauthorized' () for 
> SUBSCRIBE
> 
> $ ./mesos-execute --master=127.0.0.1:5050 --name=auth_test --command="while 
> true; do echo 'Hello'; sleep 2; done" --principal=principal --secret=secret
> I0420 21:54:02.924027  2587 scheduler.cpp:177] Version: 0.29.0
> Subscribed with ID '81fe5703-ccae-4c6e-8e39-a246109d812e-0001'
> Submitted task 'auth_test' to agent '9edb64fe-5b97-43d3-81fe-81f0cdcea0b4-S0'
> Received status update TASK_RUNNING for task 'auth_test'
>   source: SOURCE_EXECUTOR
> 
> 
> Thanks,
> 
> Tim Anderegg
> 
>

Re: Review Request 46504: Constructed error string in MethodNotAllowed.

[Alexander Rukletsov]

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



For posterity, could you please explain a bit in the description what you 
intention here is and why you take this implemntation approach?


3rdparty/libprocess/include/process/http.hpp (lines 569 - 571)


We usually insert two blank lines here.



3rdparty/libprocess/include/process/http.hpp (line 571)


Does it make sense to declare this struct inside `MethodNotAllowed`?



3rdparty/libprocess/include/process/http.hpp (lines 573 - 574)


It looks like you don't need to keep this two valus as fields, right?



3rdparty/libprocess/include/process/http.hpp (line 579)


Why do you take `Request` instead of a method?



3rdparty/libprocess/include/process/http.hpp (lines 603 - 605)


We usually insert two blank lines here.



3rdparty/libprocess/include/process/http.hpp (lines 623 - 624)


Blank line, please!



3rdparty/libprocess/include/process/http.hpp 


Why have you removed this line?


- Alexander Rukletsov


On April 22, 2016, 3:11 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46504/
> ---
> 
> (Updated April 22, 2016, 3:11 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4126
> https://issues.apache.org/jira/browse/MESOS-4126
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 8f4eabcbb71ead1f5c28e1d8a2dd40db7af1f297 
> 
> Diff: https://reviews.apache.org/r/46504/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 46550: Fixed slave to initialize libprocess before modules.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 21, 2016, 6:41 p.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46550/
> ---
> 
> (Updated April 21, 2016, 6:41 p.m.)
> 
> 
> Review request for mesos, Adam B, Kapil Arya, Joseph Wu, Niklas Nielsen, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is important because otherwise a module might call
> libprocess `initialize()` before slave calls `initialize(id)`.
> If this happens the  HTTP request process "delegate" will be wrong,
> 
> 
> Diffs
> -
> 
>   src/slave/main.cpp 38bd005 
> 
> Diff: https://reviews.apache.org/r/46550/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andy Pang
> 
>

Re: Review Request 46551: Experimental: Use liburiparser to parse URIs.

[Joseph Wu]

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

(Updated April 22, 2016, 12:24 p.m.)


Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
Varghese.


Changes
---

Changed uriparser to a bundled dependency (see dependent review).


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


Repository: mesos


Description (updated)
---

This demonstrates what it might look like to use `uriparser` to replace 
the implementation of `http::URL::parse`.

WIP: We want to use `uriparser` to parse a URI (not a URL).


Diffs (updated)
-

  3rdparty/libprocess/src/http.cpp 48f91d01555e760b1c4fd2cde684168d65f75e57 
  3rdparty/libprocess/src/tests/http_tests.cpp 
4337e6028a3a6e5279793c7c6f73bb9a4f60cb0a 

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


Testing
---

make check GTEST_FILTER="*ParseUrls"


Thanks,

Joseph Wu

Review Request 46580: Added uriparser as a bundled dependency.

[Joseph Wu]

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

Review request for mesos, Gilbert Song, Artem Harutyunyan, Jie Yu, and Jojy 
Varghese.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/Makefile.am f7aa579 
  3rdparty/libprocess/3rdparty/uriparser-0.8.4.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/versions.am 5506eb3 
  3rdparty/libprocess/Makefile.am c51c31e 
  3rdparty/libprocess/configure.ac d27e46e 

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


Testing
---

../configure
make
make check


Thanks,

Joseph Wu

Re: Review Request 46260: Added a MetricsTest with authentication to libprocess.

[Greg Mann]

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

(Updated April 22, 2016, 7:14 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


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


Repository: mesos


Description
---

The test `MetricsTest.SnapshotAuthenticationEnabled`
is added to the libprocess tests in this patch.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/metrics_tests.cpp 
b84dc8d858f58bc9f52b218b7153510417cf34c2 

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


Testing
---

`sudo make check` on OSX.


Thanks,

Greg Mann

Re: Review Request 45370: Implemented prepare() for volume isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp (line 65)


s/DockerVolumeInfo/VolumeInfo/



src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp (lines 65 - 
70)


We typically just put stuff that's needed for cleanup in 'Info'. Also, 
remember that any fields in 'Info' should be able to be recovered after agent 
restarts.

So I would suggest that we don't put that in Info:
```
struct VolumeInfo
{
  DockerVolume volume;
  string containerPath;
  Option> options;
};

struct Info
{
  vector volumes;
};
```



src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp (line 67)


s/dockerVolume/volume/



src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp (line 77)


Should that be a hashset given that we don't allow duplicate?



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 139)


s/dockerVolumeInfos/volumeInfos/



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 141 - 
143)


This sentense is broken. Please fix the gramma.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 144)


Instead of that, let's introduce a hash function for DockerVolume. You can 
put the hash function in state.hpp.

```
template <>
struct hash
{
  ...
};
```



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 144 - 
145)


s/dockerVolumeKey/volumes/



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 151 - 
152)


Please check the source.type as well. We might want to add more types later.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 154)


Once you define the hash for DockerVolume, you don't have to do that.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 156 - 
159)


I would return a Failure here.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 168)


I don't think this check is necessary.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 169)


Let's kill this temp variable as well. We try to avoid temp variables if 
possible.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 170 - 
171)


I would remove this logging.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 200 - 
202)


I would add 'volume' to 'info.volumes' after we successfully create the 
containerDir  and successfully checkpoint the state file.

This can save us from some unnecessary cleanup if any of the above failed.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 215 - 
217)


I would move this log below after the checkpointing  is done.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 229)


s/checkpointed/checkpoint/



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 235)


s/external/docker/



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 241)


Please use const ref here.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 242)


YOu can iterate through `volumeInfos` here.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 258)


you can bind `volumeInfos` here.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 266)


s/volumes/futures/



src/slave/slave.cpp (lines 3813 - 3818)

Re: Review Request 45922: Added agent authorization flags.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45922]

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

- Mesos ReviewBot


On April 22, 2016, 10:11 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45922/
> ---
> 
> (Updated April 22, 2016, 10:11 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rojas.
> 
> 
> Bugs: MESOS-5142
> https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956 
>   src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea 
>   src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526 
>   src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4 
>   src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
>   src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb 
> 
> Diff: https://reviews.apache.org/r/45922/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 46577: Fixed isolator cleaup issue when destroying a provisioning container.

[Gilbert Song]

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

Review request for mesos, Artem Harutyunyan, Jie Yu, Neil Conway, and Timothy 
Chen.


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


Repository: mesos


Description
---

Fixed isolator cleaup issue when destroying a provisioning container.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
1e1a36903f4377497bb72b69e4ead63675d453c0 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 46576: Fixed a mesos containerizer race destroy while preparing.

[Gilbert Song]

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

Review request for mesos, Artem Harutyunyan, Jie Yu, and Neil Conway.


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


Repository: mesos


Description
---

Fixed a mesos containerizer race destroy while preparing.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
1e1a36903f4377497bb72b69e4ead63675d453c0 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Jan Schlicht]

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

(Updated April 22, 2016, 7:46 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.


Changes
---

To remove dependency circle, "depends on" was changed back to 45922. But the 
actual dependency chain is now: 45922, 46318, 46203, 46319. Need to sync that 
with bbannier.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
  include/mesos/authorizer/acls.proto c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
  include/mesos/authorizer/authorizer.proto 
40d93ea257d1df8d22eee8a21667db90d579a8fe 
  src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
  src/authorizer/local/authorizer.cpp c744a16041c2466d3839a37fbee2bf86887bf4e1 
  src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
  src/slave/http.cpp f887a71684304a82ff0d81dbd240e29aa7e2ac67 
  src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
  src/tests/slave_authorization_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 46203: Added authorization of the '/flags' endpoint.

[Jan Schlicht]

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

(Updated April 22, 2016, 7:35 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.


Changes
---

Addresses issues (renamed ACL action, changed tests to TYPED_TEST_CASE as in 
`authorization_tests.cpp`)


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


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
  include/mesos/authorizer/acls.proto c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
  include/mesos/authorizer/authorizer.proto 
40d93ea257d1df8d22eee8a21667db90d579a8fe 
  src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
  src/authorizer/local/authorizer.cpp c744a16041c2466d3839a37fbee2bf86887bf4e1 
  src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
  src/slave/http.cpp f887a71684304a82ff0d81dbd240e29aa7e2ac67 
  src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
  src/tests/slave_authorization_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 45275: Enabled "--explicitcreate" when call "dvdcli mount".

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp (lines 56 - 65)


I think what we should do here is:

1) If 'options' is None, we add `--explicitcreate=true`

2) Otherwise, we don't

3) Added a TODO to check dvdcli version


- Jie Yu


On April 22, 2016, 2:29 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45275/
> ---
> 
> (Updated April 22, 2016, 2:29 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5251
> https://issues.apache.org/jira/browse/MESOS-5251
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled "--explicitcreate" when call "dvdcli mount".
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
> c63fa02b8f13014bbcac6984a49eaa919d26b489 
> 
> Diff: https://reviews.apache.org/r/45275/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46472: Reran `generate-endpoint-help.py` script for endpoints redirection.

[haosdent huang]

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

(Updated April 22, 2016, 5:10 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
Neil Conway, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Reran `generate-endpoint-help.py` script for endpoints redirection.


Diffs (updated)
-

  docs/endpoints/index.md d57239edb1d2708452ce2f687634227d50bbafc4 
  docs/endpoints/master/api/v1/scheduler.md 
f647dc0c0d9023ec9b21371db4fb8c28c64b0ef7 
  docs/endpoints/master/create-volumes.md 
5c86480b488e2722a12ab09b0a28399402410e2b 
  docs/endpoints/master/destroy-volumes.md 
f75dd52bd6b75d35aa62dca54148a41cfec36ded 
  docs/endpoints/master/frameworks.md 15ecabfdd87faee6a1f24895088a3c08edf62b96 
  docs/endpoints/master/machine/down.md 
f7e81417aa663e287a61124b351f2461b1c106e6 
  docs/endpoints/master/machine/up.md d96be726f5cf524937e3ebc5b046bf5f47886a1c 
  docs/endpoints/master/maintenance/schedule.md 
3d69900ffb376d64b463e7c01f349a4eb5d2f924 
  docs/endpoints/master/maintenance/status.md 
249dae2706ccf30d79e0cd2f19821acd6ada74b7 
  docs/endpoints/master/quota.md b1e3462bd35c5acfd65323db5d0660277b110b11 
  docs/endpoints/master/redirect.md 833d9935b8fb038977fe08d826d73f03f2059bab 
  docs/endpoints/master/reserve.md 8f38cc0f9c16c368ee3698ec0a6e7fd484110ebc 
  docs/endpoints/master/roles.json.md 01e265330c4c30de11301a2c4abbddfee10ac35d 
  docs/endpoints/master/roles.md 9c01d370a86d77eb95fcc157de70304e73f5f1e9 
  docs/endpoints/master/slaves.md 9e9fff7329d6bf05ae78997b1292657ff4dd10c3 
  docs/endpoints/master/state-summary.md 
a0da400b34c202602cb661a19c6199291da4ff4b 
  docs/endpoints/master/state.json.md 7998b80f29042f4b67565b03e2464b6e3a009335 
  docs/endpoints/master/state.md 59518d624a0c5e010f856c900a9d6512a35b21af 
  docs/endpoints/master/tasks.json.md 29bb9738b1519174eaeac4f4db423000fe48d0d3 
  docs/endpoints/master/tasks.md ab9bb09631400668adddeee363dd612c71e98e16 
  docs/endpoints/master/teardown.md 9b62b2656162b4160a9845152aecf7b75231fbae 
  docs/endpoints/master/unreserve.md 8e9a696a50f1821cfb11afae68cc4fb62b6f2a2c 
  docs/endpoints/master/weights.md 7c4517343aa61bf6156fa54547690930051616a3 
  docs/endpoints/slave/health.md 301a851035459c63a844c9a7ffd1f65ef39508af 
  docs/endpoints/slave/monitor/statistics.json.md 
f409a21315c99d0ee16ad73b83b9ddc4dcca4b8d 
  docs/endpoints/slave/monitor/statistics.md 
ec9f095456ac86ec31c65c7ca80b21553ec38edb 
  docs/endpoints/slave/state.json.md 92cd4e19ede2d52a6f0e25c7c08cc67632f30b60 
  docs/endpoints/slave/state.md 977746657b70aed531e2ef803c767d7e50561119 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 46220: Added documentation for Nvidia GPU support.

[Kevin Klues]

> On April 22, 2016, 9:55 a.m., Qian Zhang wrote:
> > docs/gpu-support.md, line 440
> > 
> >
> > Does this limitation mean that currently we do not support container 
> > with an image (e.g., Docker image, Appc image)? Instead we only support 
> > image-less container since such kind of containers will be able to directly 
> > access agent host filesystem so that they can access the needed GPU library 
> > to perform GPU-aware jobs.

Yes. This is what it means currently.  However, I am planning to remove this 
restriction before the 0.29 release.


- Kevin


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


On April 14, 2016, 8:33 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46220/
> ---
> 
> (Updated April 14, 2016, 8:33 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-5221
> https://issues.apache.org/jira/browse/MESOS-5221
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for Nvidia GPU support.
> 
> 
> Diffs
> -
> 
>   docs/gpu-support.md PRE-CREATION 
>   docs/home.md dc41fc4479e6c23650cd8ac78dcc4b9161d00721 
> 
> Diff: https://reviews.apache.org/r/46220/diff/
> 
> 
> Testing
> ---
> 
> Ran a local copy of the documentation site to make sure everything looks OK.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 46473: Updated `high-availability` and `operational-guide` docs.

[haosdent huang]

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

(Updated April 22, 2016, 4:40 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
Neil Conway, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

In this changes, we update the `high-availability` and
`operational-guide` documents about master http endpoints redirction
when current master is not the leader.


Diffs (updated)
-

  docs/high-availability.md b0e744e62081b56e2a24ef5f7304f424424fa3cc 
  docs/operational-guide.md 5ae7ede3f500380a78364d5c3da2c4cea75e04c5 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 34646: Redirected to the leading master when current master is not the leader.

[haosdent huang]

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

(Updated April 22, 2016, 4:39 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
Neil Conway, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

In this changes, we redirect to the leading master in those http
endpoints which depend on master elected status if current master is
not the leader.


Diffs (updated)
-

  src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 

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


Testing
---

localhost:5053 is the leading master. localhost:5054 is another master.

1) Test normal redirection.

In the leader.

```
curl -i http://localhost:5053/master/tasks
HTTP/1.1 200 OK
Date: Thu, 21 Apr 2016 06:57:50 GMT
Content-Length: 12
Content-Type: application/json

{"tasks":[]}
```

In non-leader.

```
curl -i http://localhost:5054/master/tasks
HTTP/1.1 307 Temporary Redirect
Date: Thu, 21 Apr 2016 07:00:06 GMT
Location: //localhost:5053/master/tasks
Content-Length: 0
```

2) Test the `/redirect`.

In the leader.

```
curl -i http://localhost:5053/master/redirect
HTTP/1.1 307 Temporary Redirect
Date: Thu, 21 Apr 2016 07:00:53 GMT
Location: //localhost:5053
Content-Length: 0
```

In the non-leader.

```
curl -i http://localhost:5054/master/redirect
HTTP/1.1 307 Temporary Redirect
Date: Thu, 21 Apr 2016 07:01:34 GMT
Location: //localhost:5053
Content-Length: 0
```

3) Test the `ServiceUnavailable` (By quit zookeeper).

In the non-leader(The leader is quit because could not connect to zookeeper).

```
curl -i http://localhost:5053/master/redirect
curl: (7) Failed connect to localhost:5053; Connection refused
curl -i http://localhost:5054/master/redirect
HTTP/1.1 503 Service Unavailable
Date: Thu, 21 Apr 2016 07:02:38 GMT
Content-Length: 17

No leader elected
```


Thanks,

haosdent huang

Re: Review Request 46472: Reran `generate-endpoint-help.py` script for endpoints redirection.

[haosdent huang]

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

(Updated April 22, 2016, 4:40 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
Neil Conway, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Reran `generate-endpoint-help.py` script for endpoints redirection.


Diffs (updated)
-

  docs/endpoints/index.md d57239edb1d2708452ce2f687634227d50bbafc4 
  docs/endpoints/master/api/v1/scheduler.md 
f647dc0c0d9023ec9b21371db4fb8c28c64b0ef7 
  docs/endpoints/master/create-volumes.md 
5c86480b488e2722a12ab09b0a28399402410e2b 
  docs/endpoints/master/destroy-volumes.md 
f75dd52bd6b75d35aa62dca54148a41cfec36ded 
  docs/endpoints/master/frameworks.md 15ecabfdd87faee6a1f24895088a3c08edf62b96 
  docs/endpoints/master/machine/down.md 
f7e81417aa663e287a61124b351f2461b1c106e6 
  docs/endpoints/master/machine/up.md d96be726f5cf524937e3ebc5b046bf5f47886a1c 
  docs/endpoints/master/maintenance/schedule.md 
3d69900ffb376d64b463e7c01f349a4eb5d2f924 
  docs/endpoints/master/maintenance/status.md 
249dae2706ccf30d79e0cd2f19821acd6ada74b7 
  docs/endpoints/master/quota.md b1e3462bd35c5acfd65323db5d0660277b110b11 
  docs/endpoints/master/redirect.md 833d9935b8fb038977fe08d826d73f03f2059bab 
  docs/endpoints/master/reserve.md 8f38cc0f9c16c368ee3698ec0a6e7fd484110ebc 
  docs/endpoints/master/roles.json.md 01e265330c4c30de11301a2c4abbddfee10ac35d 
  docs/endpoints/master/roles.md 9c01d370a86d77eb95fcc157de70304e73f5f1e9 
  docs/endpoints/master/slaves.md 9e9fff7329d6bf05ae78997b1292657ff4dd10c3 
  docs/endpoints/master/state-summary.md 
a0da400b34c202602cb661a19c6199291da4ff4b 
  docs/endpoints/master/state.json.md 7998b80f29042f4b67565b03e2464b6e3a009335 
  docs/endpoints/master/state.md 59518d624a0c5e010f856c900a9d6512a35b21af 
  docs/endpoints/master/tasks.json.md 29bb9738b1519174eaeac4f4db423000fe48d0d3 
  docs/endpoints/master/tasks.md ab9bb09631400668adddeee363dd612c71e98e16 
  docs/endpoints/master/teardown.md 9b62b2656162b4160a9845152aecf7b75231fbae 
  docs/endpoints/master/unreserve.md 8e9a696a50f1821cfb11afae68cc4fb62b6f2a2c 
  docs/endpoints/master/weights.md 7c4517343aa61bf6156fa54547690930051616a3 
  docs/endpoints/slave/health.md 301a851035459c63a844c9a7ffd1f65ef39508af 
  docs/endpoints/slave/monitor/statistics.json.md 
f409a21315c99d0ee16ad73b83b9ddc4dcca4b8d 
  docs/endpoints/slave/monitor/statistics.md 
ec9f095456ac86ec31c65c7ca80b21553ec38edb 
  docs/endpoints/slave/state.json.md 92cd4e19ede2d52a6f0e25c7c08cc67632f30b60 
  docs/endpoints/slave/state.md 977746657b70aed531e2ef803c767d7e50561119 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 46471: Updated descriptions for master endpoints which may return redirect.

[haosdent huang]

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

(Updated April 22, 2016, 4:40 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
Neil Conway, and Vinod Kone.


Changes
---

Address @neilc's comments.


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


Repository: mesos


Description
---

Update master endpoints descriptions about `TEMPORARY_REDIRECT` and
`SERVICE_UNAVAILABLE` when current master is not the leader.


Diffs (updated)
-

  src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 46325: Updated HTTP command executor to support kill policy in Kill event.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46321, 46491, 46322, 46323, 46325]

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

- Mesos ReviewBot


On April 22, 2016, 2:46 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46325/
> ---
> 
> (Updated April 22, 2016, 2:46 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Qian Zhang.
> 
> 
> Bugs: MESOS-4908
> https://issues.apache.org/jira/browse/MESOS-4908
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Kill policy can be provided in a kill event. In this case it should
> take precedence over kill policy specified when the task was launched.
> When kill event is issued multiple times during the task termination,
> the signal escalation timeout (the time a task has between SIGTERM
> and SIGKILL) may be reduced.
> 
> Since updating the delay timer (we use it for signal escalation delay)
> is currently not possible, we cancel the existing signal timer and set
> up a new one. `Clock::cancel()` guarantees that, if existed, the timer
> is removed before the function returns; hence we do not set up more
> than 1 timer for signal escalation delay.
> 
> 
> Diffs
> -
> 
>   src/launcher/http_command_executor.cpp 
> 2ce296740abac096aa85ad11dd4d191e42c1aa06 
> 
> Diff: https://reviews.apache.org/r/46325/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Tested manually using modified `mesos-execute` in a way that **two** extra 
> kill task requests are sent, 2s and 3s after receiving `TASK_RUNNING`. Each 
> kill task request specifies `KillPolicy` with 1s grace period. Together with 
> a kill *without* a kill policy scheduled 1s after the task is being launched, 
> the task receives **three** kill requests in total.
> 
> Master: `./bin/mesos-master.sh --work_dir=/tmp/w/m --ip=127.0.0.1`
> Agent: `./bin/mesos-slave.sh --work_dir=/tmp/w/s --master=127.0.0.1:5050 
> --http_command_executor`
> Mesos-execute: `./src/mesos-execute --master=127.0.0.1:5050 --name=test 
> --command="/Users/alex/bin/unresponsive_process" --env='{"GLOG_v": "2"}' 
> --kill_after=1secs`
> 
> HTTP command executor log:
> ```
> Received SUBSCRIBED event
> Subscribed executor on alexr.fritz.box
> Received LAUNCH event
> Starting task test
> sh -c '/Users/alex/bin/unresponsive_process'
> Forked command at 17475
> 14455919081943275466
> Received ACKNOWLEDGED event
> 17172602460659762152
> Received KILL event
> Received kill for task test
> Sending SIGTERM to process tree at pid 17475
> Sent SIGTERM to the following process trees:
> [ 
> --- 17475 /Users/alex/bin/unresponsive_process
> ]
> 4381544758593790168
> Scheduling escalation to SIGKILL in 3secs from now
> Received ACKNOWLEDGED event
> Received KILL event
> Received kill for task test
> Rescheduling escalation to SIGKILL in 1secs from now
> 10370891801885978953
> Process 17475 did not terminate after 1secs, sending SIGKILL to process tree 
> at 17475
> Killed the following process trees:
> [ 
> --- 17475 /Users/alex/bin/unresponsive_process
> ]
> Received KILL event
> Received kill for task test
> Command terminated with signal Killed: 9 (pid: 17475)
> ```
> 
> Excerpt from the agent log that shows all 3 kill task requests and that the 
> segnal escalation timeout was reduced from 3s to 1s:
> ```
> I0418 14:27:17.825070 244285440 slave.cpp:3605] Forwarding the update 
> TASK_RUNNING (UUID: 925e2d89-f6eb-464d-9a50-a74a8e07bc88) for task test of 
> framework ab374773-a018-4531-923b-899cf1e4f573- to master@127.0.0.1:5050
> I0418 14:27:17.831233 242139136 status_update_manager.cpp:392] Received 
> status update acknowledgement (UUID: 925e2d89-f6eb-464d-9a50-a74a8e07bc88) 
> for task test of framework ab374773-a018-4531-923b-899cf1e4f573-
> I0418 14:27:18.834309 244285440 slave.cpp:2046] Asked to kill task test of 
> framework ab374773-a018-4531-923b-899cf1e4f573-
> I0418 14:27:18.842150 244285440 http.cpp:178] HTTP POST for 
> /slave(1)/api/v1/executor from 192.168.178.24:54206
> I0418 14:27:18.842331 244285440 slave.cpp:3207] Handling status update 
> TASK_KILLING (UUID: a2f6eca7-b3e5-4e45-adcb-356f75355563) for task test of 
> framework ab374773-a018-4531-923b-899cf1e4f573-
> I0418 14:27:18.843214 242139136 status_update_manager.cpp:320] Received 
> status update TASK_KILLING (UUID: a2f6eca7-b3e5-4e45-adcb-356f75355563) for 
> task test of framework ab374773-a018-4531-923b-899cf1e4f573-
> I0418 14:27:18.843387 243748864 slave.cpp:3605] Forwarding the update 
> TASK_KILLING (UUID: 

Re: Review Request 46075: Update docs to reflect /containers endpoint.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 18, 2016, 3:39 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46075/
> ---
> 
> (Updated April 18, 2016, 3:39 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Docs are updated using support/generate-endpoint-help.py.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/index.md 4fc5583e27dda6df6dfc9f3cee72925a07cdd2b3 
>   docs/endpoints/monitor/statistics.json.md 
> 1830493d9b936279abcfc03fc5023b7e8b54888b 
>   docs/endpoints/monitor/statistics.md 
> 38dede06ac0a7cc9798a50ffd540b124eb5c1c84 
>   docs/endpoints/slave/containers.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46075/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 45375: Implemented cleanup() for docker volume isolator.

[Guangya Liu]

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

(Updated 四月 22, 2016, 4:13 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Implemented cleanup() for docker volume isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
bedc687cc280d0b721fb84801039fd3614364cca 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
915e5ae755a55a02b7dfcda88165f27346cad955 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46473: Updated `high-availability` and `operational-guide` docs.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On April 22, 2016, 4:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46473/
> ---
> 
> (Updated April 22, 2016, 4:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we update the `high-availability` and
> `operational-guide` documents about master http endpoints redirction
> when current master is not the leader.
> 
> 
> Diffs
> -
> 
>   docs/high-availability.md b0e744e62081b56e2a24ef5f7304f424424fa3cc 
>   docs/operational-guide.md 5ae7ede3f500380a78364d5c3da2c4cea75e04c5 
> 
> Diff: https://reviews.apache.org/r/46473/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46471: Updated descriptions for master endpoints which may return redirect.

[Neil Conway]

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


Fix it, then Ship it!




Ship It!


src/master/http.cpp (line 2339)


"was successful"



src/master/http.cpp (line 2455)


"was successful"


- Neil Conway


On April 22, 2016, 4:11 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46471/
> ---
> 
> (Updated April 22, 2016, 4:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update master endpoints descriptions about `TEMPORARY_REDIRECT` and
> `SERVICE_UNAVAILABLE` when current master is not the leader.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 
> 
> Diff: https://reviews.apache.org/r/46471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46472: Reran `generate-endpoint-help.py` script for endpoints redirection.

[haosdent huang]

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

(Updated April 22, 2016, 4:11 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
Neil Conway, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Reran `generate-endpoint-help.py` script for endpoints redirection.


Diffs (updated)
-

  docs/endpoints/index.md 4fc5583e27dda6df6dfc9f3cee72925a07cdd2b3 
  docs/endpoints/master/api/v1/scheduler.md 
f647dc0c0d9023ec9b21371db4fb8c28c64b0ef7 
  docs/endpoints/master/create-volumes.md 
5c86480b488e2722a12ab09b0a28399402410e2b 
  docs/endpoints/master/destroy-volumes.md 
f75dd52bd6b75d35aa62dca54148a41cfec36ded 
  docs/endpoints/master/frameworks.md 15ecabfdd87faee6a1f24895088a3c08edf62b96 
  docs/endpoints/master/machine/down.md 
f7e81417aa663e287a61124b351f2461b1c106e6 
  docs/endpoints/master/machine/up.md d96be726f5cf524937e3ebc5b046bf5f47886a1c 
  docs/endpoints/master/maintenance/schedule.md 
3d69900ffb376d64b463e7c01f349a4eb5d2f924 
  docs/endpoints/master/maintenance/status.md 
249dae2706ccf30d79e0cd2f19821acd6ada74b7 
  docs/endpoints/master/quota.md b1e3462bd35c5acfd65323db5d0660277b110b11 
  docs/endpoints/master/redirect.md 833d9935b8fb038977fe08d826d73f03f2059bab 
  docs/endpoints/master/reserve.md 8f38cc0f9c16c368ee3698ec0a6e7fd484110ebc 
  docs/endpoints/master/roles.json.md 01e265330c4c30de11301a2c4abbddfee10ac35d 
  docs/endpoints/master/roles.md 9c01d370a86d77eb95fcc157de70304e73f5f1e9 
  docs/endpoints/master/slaves.md 9e9fff7329d6bf05ae78997b1292657ff4dd10c3 
  docs/endpoints/master/state-summary.md 
a0da400b34c202602cb661a19c6199291da4ff4b 
  docs/endpoints/master/state.json.md 7998b80f29042f4b67565b03e2464b6e3a009335 
  docs/endpoints/master/state.md 59518d624a0c5e010f856c900a9d6512a35b21af 
  docs/endpoints/master/tasks.json.md 29bb9738b1519174eaeac4f4db423000fe48d0d3 
  docs/endpoints/master/tasks.md ab9bb09631400668adddeee363dd612c71e98e16 
  docs/endpoints/master/teardown.md 9b62b2656162b4160a9845152aecf7b75231fbae 
  docs/endpoints/master/unreserve.md 8e9a696a50f1821cfb11afae68cc4fb62b6f2a2c 
  docs/endpoints/master/weights.md 7c4517343aa61bf6156fa54547690930051616a3 
  docs/endpoints/monitor/statistics.json.md 
1830493d9b936279abcfc03fc5023b7e8b54888b 
  docs/endpoints/monitor/statistics.md 38dede06ac0a7cc9798a50ffd540b124eb5c1c84 
  docs/endpoints/slave/containers.md PRE-CREATION 
  docs/endpoints/slave/health.md 301a851035459c63a844c9a7ffd1f65ef39508af 
  docs/endpoints/slave/state.json.md 92cd4e19ede2d52a6f0e25c7c08cc67632f30b60 
  docs/endpoints/slave/state.md 977746657b70aed531e2ef803c767d7e50561119 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 46471: Updated descriptions for master endpoints which may return redirect.

[haosdent huang]

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

(Updated April 22, 2016, 4:11 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
Neil Conway, and Vinod Kone.


Changes
---

Address @neilc's comments.


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


Repository: mesos


Description
---

Update master endpoints descriptions about `TEMPORARY_REDIRECT` and
`SERVICE_UNAVAILABLE` when current master is not the leader.


Diffs (updated)
-

  src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 44454: Enabled Sequence mount for prepare().

[Guangya Liu]

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

(Updated 四月 22, 2016, 4:11 p.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description
---

Enabled Sequence mount for prepare().


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
bedc687cc280d0b721fb84801039fd3614364cca 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
915e5ae755a55a02b7dfcda88165f27346cad955 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46473: Updated `high-availability` and `operational-guide` docs.

[haosdent huang]

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

(Updated April 22, 2016, 4:10 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
Neil Conway, and Vinod Kone.


Changes
---

Address @neilc's comments.


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


Repository: mesos


Description
---

In this changes, we update the `high-availability` and
`operational-guide` documents about master http endpoints redirction
when current master is not the leader.


Diffs (updated)
-

  docs/high-availability.md b0e744e62081b56e2a24ef5f7304f424424fa3cc 
  docs/operational-guide.md 5ae7ede3f500380a78364d5c3da2c4cea75e04c5 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 46471: Updated descriptions for master endpoints which may return redirect.

[haosdent huang]

> On April 22, 2016, 3:11 p.m., Neil Conway wrote:
> >
> 
> haosdent huang wrote:
> Hi, @neilc. Thank you very much for your detail comments! I saw you use 
> `is xxed` in some comments while use `was xxed` in others. Should I change 
> all of them to `was xxed` to keep consistent?
> 
> Neil Conway wrote:
> Good point -- I don't have a strong preference on using "is" vs. "was", 
> but it would be good to pick one and use it consistently. I suppose "was" is 
> slightly more accurate?

Got it. Appreciated your comments again(you are the real author of this patch)!


- haosdent


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


On April 22, 2016, 3:48 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46471/
> ---
> 
> (Updated April 22, 2016, 3:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update master endpoints descriptions about `TEMPORARY_REDIRECT` and
> `SERVICE_UNAVAILABLE` when current master is not the leader.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 
> 
> Diff: https://reviews.apache.org/r/46471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46472: Reran `generate-endpoint-help.py` script for endpoints redirection.

[haosdent huang]

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

(Updated April 22, 2016, 3:48 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
Neil Conway, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Reran `generate-endpoint-help.py` script for endpoints redirection.


Diffs (updated)
-

  docs/endpoints/index.md 4fc5583e27dda6df6dfc9f3cee72925a07cdd2b3 
  docs/endpoints/master/api/v1/scheduler.md 
f647dc0c0d9023ec9b21371db4fb8c28c64b0ef7 
  docs/endpoints/master/create-volumes.md 
5c86480b488e2722a12ab09b0a28399402410e2b 
  docs/endpoints/master/destroy-volumes.md 
f75dd52bd6b75d35aa62dca54148a41cfec36ded 
  docs/endpoints/master/frameworks.md 15ecabfdd87faee6a1f24895088a3c08edf62b96 
  docs/endpoints/master/machine/down.md 
f7e81417aa663e287a61124b351f2461b1c106e6 
  docs/endpoints/master/machine/up.md d96be726f5cf524937e3ebc5b046bf5f47886a1c 
  docs/endpoints/master/maintenance/schedule.md 
3d69900ffb376d64b463e7c01f349a4eb5d2f924 
  docs/endpoints/master/maintenance/status.md 
249dae2706ccf30d79e0cd2f19821acd6ada74b7 
  docs/endpoints/master/quota.md b1e3462bd35c5acfd65323db5d0660277b110b11 
  docs/endpoints/master/redirect.md 833d9935b8fb038977fe08d826d73f03f2059bab 
  docs/endpoints/master/reserve.md 8f38cc0f9c16c368ee3698ec0a6e7fd484110ebc 
  docs/endpoints/master/roles.json.md 01e265330c4c30de11301a2c4abbddfee10ac35d 
  docs/endpoints/master/roles.md 9c01d370a86d77eb95fcc157de70304e73f5f1e9 
  docs/endpoints/master/slaves.md 9e9fff7329d6bf05ae78997b1292657ff4dd10c3 
  docs/endpoints/master/state-summary.md 
a0da400b34c202602cb661a19c6199291da4ff4b 
  docs/endpoints/master/state.json.md 7998b80f29042f4b67565b03e2464b6e3a009335 
  docs/endpoints/master/state.md 59518d624a0c5e010f856c900a9d6512a35b21af 
  docs/endpoints/master/tasks.json.md 29bb9738b1519174eaeac4f4db423000fe48d0d3 
  docs/endpoints/master/tasks.md ab9bb09631400668adddeee363dd612c71e98e16 
  docs/endpoints/master/teardown.md 9b62b2656162b4160a9845152aecf7b75231fbae 
  docs/endpoints/master/unreserve.md 8e9a696a50f1821cfb11afae68cc4fb62b6f2a2c 
  docs/endpoints/master/weights.md 7c4517343aa61bf6156fa54547690930051616a3 
  docs/endpoints/monitor/statistics.json.md 
1830493d9b936279abcfc03fc5023b7e8b54888b 
  docs/endpoints/monitor/statistics.md 38dede06ac0a7cc9798a50ffd540b124eb5c1c84 
  docs/endpoints/slave/containers.md PRE-CREATION 
  docs/endpoints/slave/health.md 301a851035459c63a844c9a7ffd1f65ef39508af 
  docs/endpoints/slave/state.json.md 92cd4e19ede2d52a6f0e25c7c08cc67632f30b60 
  docs/endpoints/slave/state.md 977746657b70aed531e2ef803c767d7e50561119 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 46471: Updated descriptions for master endpoints which may return redirect.

[Neil Conway]

> On April 22, 2016, 3:11 p.m., Neil Conway wrote:
> >
> 
> haosdent huang wrote:
> Hi, @neilc. Thank you very much for your detail comments! I saw you use 
> `is xxed` in some comments while use `was xxed` in others. Should I change 
> all of them to `was xxed` to keep consistent?

Good point -- I don't have a strong preference on using "is" vs. "was", but it 
would be good to pick one and use it consistently. I suppose "was" is slightly 
more accurate?


- Neil


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


On April 22, 2016, 3:48 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46471/
> ---
> 
> (Updated April 22, 2016, 3:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update master endpoints descriptions about `TEMPORARY_REDIRECT` and
> `SERVICE_UNAVAILABLE` when current master is not the leader.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 
> 
> Diff: https://reviews.apache.org/r/46471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 34646: Redirected to the leading master when current master is not the leader.

[haosdent huang]

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

(Updated April 22, 2016, 3:47 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
Neil Conway, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

In this changes, we redirect to the leading master in those http
endpoints which depend on master elected status if current master is
not the leader.


Diffs (updated)
-

  src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 

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


Testing
---

localhost:5053 is the leading master. localhost:5054 is another master.

1) Test normal redirection.

In the leader.

```
curl -i http://localhost:5053/master/tasks
HTTP/1.1 200 OK
Date: Thu, 21 Apr 2016 06:57:50 GMT
Content-Length: 12
Content-Type: application/json

{"tasks":[]}
```

In non-leader.

```
curl -i http://localhost:5054/master/tasks
HTTP/1.1 307 Temporary Redirect
Date: Thu, 21 Apr 2016 07:00:06 GMT
Location: //localhost:5053/master/tasks
Content-Length: 0
```

2) Test the `/redirect`.

In the leader.

```
curl -i http://localhost:5053/master/redirect
HTTP/1.1 307 Temporary Redirect
Date: Thu, 21 Apr 2016 07:00:53 GMT
Location: //localhost:5053
Content-Length: 0
```

In the non-leader.

```
curl -i http://localhost:5054/master/redirect
HTTP/1.1 307 Temporary Redirect
Date: Thu, 21 Apr 2016 07:01:34 GMT
Location: //localhost:5053
Content-Length: 0
```

3) Test the `ServiceUnavailable` (By quit zookeeper).

In the non-leader(The leader is quit because could not connect to zookeeper).

```
curl -i http://localhost:5053/master/redirect
curl: (7) Failed connect to localhost:5053; Connection refused
curl -i http://localhost:5054/master/redirect
HTTP/1.1 503 Service Unavailable
Date: Thu, 21 Apr 2016 07:02:38 GMT
Content-Length: 17

No leader elected
```


Thanks,

haosdent huang

Re: Review Request 46471: Updated descriptions for master endpoints which may return redirect.

[haosdent huang]

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

(Updated April 22, 2016, 3:48 p.m.)


Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
Neil Conway, and Vinod Kone.


Changes
---

Address @neilc's comments.


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


Repository: mesos


Description
---

Update master endpoints descriptions about `TEMPORARY_REDIRECT` and
`SERVICE_UNAVAILABLE` when current master is not the leader.


Diffs (updated)
-

  src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 46471: Updated descriptions for master endpoints which may return redirect.

[Neil Conway]

> On April 22, 2016, 3:11 p.m., Neil Conway wrote:
> > src/master/http.cpp, line 1307
> > 
> >
> > "Returns 200 OK when the weight update was successful."
> 
> haosdent huang wrote:
> Should it be `the weights update` here?

Yes, thanks!


- Neil


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


On April 22, 2016, 7:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46471/
> ---
> 
> (Updated April 22, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update master endpoints descriptions about `TEMPORARY_REDIRECT` and
> `SERVICE_UNAVAILABLE` when current master is not the leader.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 
> 
> Diff: https://reviews.apache.org/r/46471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46471: Updated descriptions for master endpoints which may return redirect.

[haosdent huang]

> On April 22, 2016, 3:11 p.m., Neil Conway wrote:
> >

Hi, @neilc. Thank you very much for your detail comments! I saw you use `is 
xxed` in some comments while use `was xxed` in others. Should I change all of 
them to `was xxed` to keep consistent?


- haosdent


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


On April 22, 2016, 7:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46471/
> ---
> 
> (Updated April 22, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update master endpoints descriptions about `TEMPORARY_REDIRECT` and
> `SERVICE_UNAVAILABLE` when current master is not the leader.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 
> 
> Diff: https://reviews.apache.org/r/46471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46471: Updated descriptions for master endpoints which may return redirect.

[haosdent huang]

> On April 22, 2016, 3:11 p.m., Neil Conway wrote:
> > src/master/http.cpp, line 1307
> > 
> >
> > "Returns 200 OK when the weight update was successful."

Should it be `the weights update` here?


- haosdent


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


On April 22, 2016, 7:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46471/
> ---
> 
> (Updated April 22, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update master endpoints descriptions about `TEMPORARY_REDIRECT` and
> `SERVICE_UNAVAILABLE` when current master is not the leader.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 
> 
> Diff: https://reviews.apache.org/r/46471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46391: Clarified several agent log messages.

[Neil Conway]

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

(Updated April 22, 2016, 3:28 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Address review comment.


Repository: mesos


Description
---

When diagnosing problems, it can be useful to distinguish
between executors that are "terminating" vs. "terminated".


Diffs (updated)
-

  src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 46473: Updated `high-availability` and `operational-guide` docs.

[Neil Conway]

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




docs/high-availability.md (line 29)


Can we link to the endpoint docs for `/redirect` here? i.e., 
[/redirect](endpoint/master/redirect.md)



docs/high-availability.md (line 30)


"For HTTP endpoints that only work at the leading master, requests made to 
endpoints at a non-leading master will result in an HTTP 307 redirect to the 
current leading master."



docs/operational-guide.md (line 69)


Can we link to the endpoint docs for `/state` and `/metrics/snapshot`?


- Neil Conway


On April 22, 2016, 7:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46473/
> ---
> 
> (Updated April 22, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we update the `high-availability` and
> `operational-guide` documents about master http endpoints redirction
> when current master is not the leader.
> 
> 
> Diffs
> -
> 
>   docs/high-availability.md b0e744e62081b56e2a24ef5f7304f424424fa3cc 
>   docs/operational-guide.md 5ae7ede3f500380a78364d5c3da2c4cea75e04c5 
> 
> Diff: https://reviews.apache.org/r/46473/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46469: Added support for credentials to mesos-execute.

[Tim Anderegg]

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

(Updated April 22, 2016, 3:15 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Added support for credentials to mesos-execute.


Diffs (updated)
-

  src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 

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


Testing
---

$ ./mesos-execute --master=127.0.0.1:5050 --name=auth_test --command="while 
true; do echo 'Hello'; sleep 2; done"
I0420 21:55:11.347501  2751 scheduler.cpp:177] Version: 0.29.0
Received an ERROR event: Received unexpected '401 Unauthorized' () for SUBSCRIBE

$ ./mesos-execute --master=127.0.0.1:5050 --name=auth_test --command="while 
true; do echo 'Hello'; sleep 2; done" --principal=principal --secret=secret
I0420 21:54:02.924027  2587 scheduler.cpp:177] Version: 0.29.0
Subscribed with ID '81fe5703-ccae-4c6e-8e39-a246109d812e-0001'
Submitted task 'auth_test' to agent '9edb64fe-5b97-43d3-81fe-81f0cdcea0b4-S0'
Received status update TASK_RUNNING for task 'auth_test'
  source: SOURCE_EXECUTOR


Thanks,

Tim Anderegg

Re: Review Request 46471: Updated descriptions for master endpoints which may return redirect.

[Neil Conway]

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




src/master/http.cpp (line 335)


Can we use the same phrasing ("if the leading master cannot be found") here 
as we do below?



src/master/http.cpp (line 690)


Lowercase "r" in "redirect", here and below.



src/master/http.cpp (line 795)


"when the frameworks info is queried successfully."



src/master/http.cpp (line 1175)


"Returns 200 OK when the request is processed successfully."



src/master/http.cpp (line 1259)


"Returns 200 OK when the quota has been changed successfully."



src/master/http.cpp (line 1307)


"Returns 200 OK when the weight update was successful."



src/master/http.cpp (line 1350)


"Returns 200 OK when the state of the master was queried successfully."



src/master/http.cpp (line 1706)


"Returns 200 OK when a summary of the master's state was queried 
successfully."



src/master/http.cpp (line 1836)


"Returns 200 OK when information about roles was queried successfully."



src/master/http.cpp (line 1957)


Not years, but "Returns 200 OK if the framework was torn down successfully."



src/master/http.cpp (line 2054)


"Returns 200 OK when task information was queried successfully."



src/master/http.cpp (line 2188)


"when maintenance successfully" is ungrammatical. "when the requested 
maintenance operation was performed successfully."



src/master/http.cpp (line 2337)


"Returns 200 OK when the operation is successful."



src/master/http.cpp (line 2453)


"Returns 200 OK when the operation is successful."



src/master/http.cpp (line 2568)


"Returns 200 OK when the maintenance status was queried successfully."


- Neil Conway


On April 22, 2016, 7:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46471/
> ---
> 
> (Updated April 22, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update master endpoints descriptions about `TEMPORARY_REDIRECT` and
> `SERVICE_UNAVAILABLE` when current master is not the leader.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 
> 
> Diff: https://reviews.apache.org/r/46471/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46505: Removed MethodNotAllowed error string creation.

[Jacob Janco]

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

(Updated April 22, 2016, 3:11 p.m.)


Review request for mesos and Alexander Rukletsov.


Summary (updated)
-

Removed MethodNotAllowed error string creation.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
  src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c 

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


Testing
---


Thanks,

Jacob Janco

Re: Review Request 34646: Redirected to the leading master when current master is not the leader.

[haosdent huang]

> On April 19, 2016, 3:37 p.m., Neil Conway wrote:
> > Can we update the docs to describe this behavior? e.g., add a note to 
> > https://mesos.apache.org/documentation/latest/endpoints/ describing the 
> > redirect behavior -- I suppose it is worth adding a note to every master 
> > endpoint's doc page describing the redirect behavior, as well as a note to 
> > the "Master Endpoints" section of the top-level endpoints doc. Might also 
> > be worth adding a note to the "Operational Guide" or "High Availability" 
> > pages. We also need to update the docs for the "master/redirect/" endpoint.
> > 
> > This change requires one or more test cases. For example: (a) normal 
> > redirect case (b) return ServiceUnavailable when there is no leading master 
> > (c) avoiding redirect loop for `/redirect` endpoint.
> 
> haosdent huang wrote:
> Thank you very much for your review. Let me update.
> 
> haosdent huang wrote:
> @neilc, I just updated the testing done field. Because we could not 
> create multiple masters test cases now, I test this manually. May you review 
> this again? Thank you in advance.
> 
> Neil Conway wrote:
> @haosdent: Ah, right, I remember now why it isn't easy to add test cases. 
> How painful would it be to test this by forking multiple processes and 
> running one master per child process?

Multiple processes should be doable, I would try this in following days.


- haosdent


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


On April 22, 2016, 7:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34646/
> ---
> 
> (Updated April 22, 2016, 7:18 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we redirect to the leading master in those http
> endpoints which depend on master elected status if current master is
> not the leader.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp de06985cafc93022fffc0d13812a119ae43b7e57 
> 
> Diff: https://reviews.apache.org/r/34646/diff/
> 
> 
> Testing
> ---
> 
> localhost:5053 is the leading master. localhost:5054 is another master.
> 
> 1) Test normal redirection.
> 
> In the leader.
> 
> ```
> curl -i http://localhost:5053/master/tasks
> HTTP/1.1 200 OK
> Date: Thu, 21 Apr 2016 06:57:50 GMT
> Content-Length: 12
> Content-Type: application/json
> 
> {"tasks":[]}
> ```
> 
> In non-leader.
> 
> ```
> curl -i http://localhost:5054/master/tasks
> HTTP/1.1 307 Temporary Redirect
> Date: Thu, 21 Apr 2016 07:00:06 GMT
> Location: //localhost:5053/master/tasks
> Content-Length: 0
> ```
> 
> 2) Test the `/redirect`.
> 
> In the leader.
> 
> ```
> curl -i http://localhost:5053/master/redirect
> HTTP/1.1 307 Temporary Redirect
> Date: Thu, 21 Apr 2016 07:00:53 GMT
> Location: //localhost:5053
> Content-Length: 0
> ```
> 
> In the non-leader.
> 
> ```
> curl -i http://localhost:5054/master/redirect
> HTTP/1.1 307 Temporary Redirect
> Date: Thu, 21 Apr 2016 07:01:34 GMT
> Location: //localhost:5053
> Content-Length: 0
> ```
> 
> 3) Test the `ServiceUnavailable` (By quit zookeeper).
> 
> In the non-leader(The leader is quit because could not connect to zookeeper).
> 
> ```
> curl -i http://localhost:5053/master/redirect
> curl: (7) Failed connect to localhost:5053; Connection refused
> curl -i http://localhost:5054/master/redirect
> HTTP/1.1 503 Service Unavailable
> Date: Thu, 21 Apr 2016 07:02:38 GMT
> Content-Length: 17
> 
> No leader elected
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46501: Updated authorization.md to reflect current changes.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46501]

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

- Mesos ReviewBot


On April 22, 2016, 12:13 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46501/
> ---
> 
> (Updated April 22, 2016, 12:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, and Neil Conway.
> 
> 
> Bugs: MESOS-4785
> https://issues.apache.org/jira/browse/MESOS-4785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The API of the authorization has been changing constantly over the
> last few versions. This patch attempts to update the documentation to
> the those changes into account.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
> 
> Diff: https://reviews.apache.org/r/46501/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 46325: Updated HTTP command executor to support kill policy in Kill event.

[Alexander Rukletsov]

> On April 21, 2016, 9:56 p.m., Ben Mahler wrote:
> > Could you pull out the 'killed' bug fix? Any reason it's bundled in this 
> > patch?

Will do.


- Alexander


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


On April 22, 2016, 2:46 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46325/
> ---
> 
> (Updated April 22, 2016, 2:46 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Qian Zhang.
> 
> 
> Bugs: MESOS-4908
> https://issues.apache.org/jira/browse/MESOS-4908
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Kill policy can be provided in a kill event. In this case it should
> take precedence over kill policy specified when the task was launched.
> When kill event is issued multiple times during the task termination,
> the signal escalation timeout (the time a task has between SIGTERM
> and SIGKILL) may be reduced.
> 
> Since updating the delay timer (we use it for signal escalation delay)
> is currently not possible, we cancel the existing signal timer and set
> up a new one. `Clock::cancel()` guarantees that, if existed, the timer
> is removed before the function returns; hence we do not set up more
> than 1 timer for signal escalation delay.
> 
> 
> Diffs
> -
> 
>   src/launcher/http_command_executor.cpp 
> 2ce296740abac096aa85ad11dd4d191e42c1aa06 
> 
> Diff: https://reviews.apache.org/r/46325/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Tested manually using modified `mesos-execute` in a way that **two** extra 
> kill task requests are sent, 2s and 3s after receiving `TASK_RUNNING`. Each 
> kill task request specifies `KillPolicy` with 1s grace period. Together with 
> a kill *without* a kill policy scheduled 1s after the task is being launched, 
> the task receives **three** kill requests in total.
> 
> Master: `./bin/mesos-master.sh --work_dir=/tmp/w/m --ip=127.0.0.1`
> Agent: `./bin/mesos-slave.sh --work_dir=/tmp/w/s --master=127.0.0.1:5050 
> --http_command_executor`
> Mesos-execute: `./src/mesos-execute --master=127.0.0.1:5050 --name=test 
> --command="/Users/alex/bin/unresponsive_process" --env='{"GLOG_v": "2"}' 
> --kill_after=1secs`
> 
> HTTP command executor log:
> ```
> Received SUBSCRIBED event
> Subscribed executor on alexr.fritz.box
> Received LAUNCH event
> Starting task test
> sh -c '/Users/alex/bin/unresponsive_process'
> Forked command at 17475
> 14455919081943275466
> Received ACKNOWLEDGED event
> 17172602460659762152
> Received KILL event
> Received kill for task test
> Sending SIGTERM to process tree at pid 17475
> Sent SIGTERM to the following process trees:
> [ 
> --- 17475 /Users/alex/bin/unresponsive_process
> ]
> 4381544758593790168
> Scheduling escalation to SIGKILL in 3secs from now
> Received ACKNOWLEDGED event
> Received KILL event
> Received kill for task test
> Rescheduling escalation to SIGKILL in 1secs from now
> 10370891801885978953
> Process 17475 did not terminate after 1secs, sending SIGKILL to process tree 
> at 17475
> Killed the following process trees:
> [ 
> --- 17475 /Users/alex/bin/unresponsive_process
> ]
> Received KILL event
> Received kill for task test
> Command terminated with signal Killed: 9 (pid: 17475)
> ```
> 
> Excerpt from the agent log that shows all 3 kill task requests and that the 
> segnal escalation timeout was reduced from 3s to 1s:
> ```
> I0418 14:27:17.825070 244285440 slave.cpp:3605] Forwarding the update 
> TASK_RUNNING (UUID: 925e2d89-f6eb-464d-9a50-a74a8e07bc88) for task test of 
> framework ab374773-a018-4531-923b-899cf1e4f573- to master@127.0.0.1:5050
> I0418 14:27:17.831233 242139136 status_update_manager.cpp:392] Received 
> status update acknowledgement (UUID: 925e2d89-f6eb-464d-9a50-a74a8e07bc88) 
> for task test of framework ab374773-a018-4531-923b-899cf1e4f573-
> I0418 14:27:18.834309 244285440 slave.cpp:2046] Asked to kill task test of 
> framework ab374773-a018-4531-923b-899cf1e4f573-
> I0418 14:27:18.842150 244285440 http.cpp:178] HTTP POST for 
> /slave(1)/api/v1/executor from 192.168.178.24:54206
> I0418 14:27:18.842331 244285440 slave.cpp:3207] Handling status update 
> TASK_KILLING (UUID: a2f6eca7-b3e5-4e45-adcb-356f75355563) for task test of 
> framework ab374773-a018-4531-923b-899cf1e4f573-
> I0418 14:27:18.843214 242139136 status_update_manager.cpp:320] Received 
> status update TASK_KILLING (UUID: a2f6eca7-b3e5-4e45-adcb-356f75355563) for 
> task test of framework ab374773-a018-4531-923b-899cf1e4f573-
> I0418 14:27:18.843387 243748864 slave.cpp:3605] Forwarding the update 
> TASK_KILLING (UUID: a2f6eca7-b3e5-4e45-adcb-356f75355563) for task test of 
> framework 

Re: Review Request 46325: Updated HTTP command executor to support kill policy in Kill event.

[Alexander Rukletsov]

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

(Updated April 22, 2016, 2:46 p.m.)


Review request for mesos, Ben Mahler and Qian Zhang.


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


Repository: mesos


Description
---

Kill policy can be provided in a kill event. In this case it should
take precedence over kill policy specified when the task was launched.
When kill event is issued multiple times during the task termination,
the signal escalation timeout (the time a task has between SIGTERM
and SIGKILL) may be reduced.

Since updating the delay timer (we use it for signal escalation delay)
is currently not possible, we cancel the existing signal timer and set
up a new one. `Clock::cancel()` guarantees that, if existed, the timer
is removed before the function returns; hence we do not set up more
than 1 timer for signal escalation delay.


Diffs (updated)
-

  src/launcher/http_command_executor.cpp 
2ce296740abac096aa85ad11dd4d191e42c1aa06 

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


Testing
---

`make check`

Tested manually using modified `mesos-execute` in a way that **two** extra kill 
task requests are sent, 2s and 3s after receiving `TASK_RUNNING`. Each kill 
task request specifies `KillPolicy` with 1s grace period. Together with a kill 
*without* a kill policy scheduled 1s after the task is being launched, the task 
receives **three** kill requests in total.

Master: `./bin/mesos-master.sh --work_dir=/tmp/w/m --ip=127.0.0.1`
Agent: `./bin/mesos-slave.sh --work_dir=/tmp/w/s --master=127.0.0.1:5050 
--http_command_executor`
Mesos-execute: `./src/mesos-execute --master=127.0.0.1:5050 --name=test 
--command="/Users/alex/bin/unresponsive_process" --env='{"GLOG_v": "2"}' 
--kill_after=1secs`

HTTP command executor log:
```
Received SUBSCRIBED event
Subscribed executor on alexr.fritz.box
Received LAUNCH event
Starting task test
sh -c '/Users/alex/bin/unresponsive_process'
Forked command at 17475
14455919081943275466
Received ACKNOWLEDGED event
17172602460659762152
Received KILL event
Received kill for task test
Sending SIGTERM to process tree at pid 17475
Sent SIGTERM to the following process trees:
[ 
--- 17475 /Users/alex/bin/unresponsive_process
]
4381544758593790168
Scheduling escalation to SIGKILL in 3secs from now
Received ACKNOWLEDGED event
Received KILL event
Received kill for task test
Rescheduling escalation to SIGKILL in 1secs from now
10370891801885978953
Process 17475 did not terminate after 1secs, sending SIGKILL to process tree at 
17475
Killed the following process trees:
[ 
--- 17475 /Users/alex/bin/unresponsive_process
]
Received KILL event
Received kill for task test
Command terminated with signal Killed: 9 (pid: 17475)
```

Excerpt from the agent log that shows all 3 kill task requests and that the 
segnal escalation timeout was reduced from 3s to 1s:
```
I0418 14:27:17.825070 244285440 slave.cpp:3605] Forwarding the update 
TASK_RUNNING (UUID: 925e2d89-f6eb-464d-9a50-a74a8e07bc88) for task test of 
framework ab374773-a018-4531-923b-899cf1e4f573- to master@127.0.0.1:5050
I0418 14:27:17.831233 242139136 status_update_manager.cpp:392] Received status 
update acknowledgement (UUID: 925e2d89-f6eb-464d-9a50-a74a8e07bc88) for task 
test of framework ab374773-a018-4531-923b-899cf1e4f573-
I0418 14:27:18.834309 244285440 slave.cpp:2046] Asked to kill task test of 
framework ab374773-a018-4531-923b-899cf1e4f573-
I0418 14:27:18.842150 244285440 http.cpp:178] HTTP POST for 
/slave(1)/api/v1/executor from 192.168.178.24:54206
I0418 14:27:18.842331 244285440 slave.cpp:3207] Handling status update 
TASK_KILLING (UUID: a2f6eca7-b3e5-4e45-adcb-356f75355563) for task test of 
framework ab374773-a018-4531-923b-899cf1e4f573-
I0418 14:27:18.843214 242139136 status_update_manager.cpp:320] Received status 
update TASK_KILLING (UUID: a2f6eca7-b3e5-4e45-adcb-356f75355563) for task test 
of framework ab374773-a018-4531-923b-899cf1e4f573-
I0418 14:27:18.843387 243748864 slave.cpp:3605] Forwarding the update 
TASK_KILLING (UUID: a2f6eca7-b3e5-4e45-adcb-356f75355563) for task test of 
framework ab374773-a018-4531-923b-899cf1e4f573- to master@127.0.0.1:5050
I0418 14:27:18.846459 242675712 status_update_manager.cpp:392] Received status 
update acknowledgement (UUID: a2f6eca7-b3e5-4e45-adcb-356f75355563) for task 
test of framework ab374773-a018-4531-923b-899cf1e4f573-
I0418 14:27:19.836699 240529408 slave.cpp:2046] Asked to kill task test of 
framework ab374773-a018-4531-923b-899cf1e4f573-
I0418 14:27:20.850658 240529408 slave.cpp:2046] Asked to kill task test of 
framework ab374773-a018-4531-923b-899cf1e4f573-
I0418 14:27:20.927338 241602560 http.cpp:178] HTTP POST for 
/slave(1)/api/v1/executor from 192.168.178.24:54206
I0418 14:27:20.927465 241602560 

Re: Review Request 46325: Updated HTTP command executor to support kill policy in Kill event.

[Alexander Rukletsov]

> On April 21, 2016, 9:56 p.m., Ben Mahler wrote:
> > src/launcher/http_command_executor.cpp, lines 172-176
> > 
> >
> > Wrap at the paren:
> > 
> > ```
> >   kill(event.kill().task_id(),
> >event.kill().has_kill_policy()
> >  ? Option(event.kill().kill_policy())
> >  : None());
> > ```
> > 
> > Any way to avoid the explicit Option construction? Does this compile?
> > 
> > ```
> >   kill(event.kill().task_id(),
> >event.kill().has_kill_policy()
> > ? Some(event.kill().kill_policy())
> > : None());
> > ```
> > 
> > If not, consider pulling out the kill policy as an option variable.

Unfortunately, `Some(event.kill().kill_policy())` does not compile. Passing 
`Event::Kill` seems unfortunate as well: if we want to call `kill()` from 
another place in the code, we have to prepare the proto message. I'll go with a 
local variable.


> On April 21, 2016, 9:56 p.m., Ben Mahler wrote:
> > src/launcher/http_command_executor.cpp, line 665
> > 
> >
> > Leaving this unimplemented as a TODO seems to have implications on the 
> > documentation in your previous patch:
> > 
> > https://reviews.apache.org/r/46322/
> > 
> > It seems like all you would need to do to implement the TODO is to 
> > remove this if condition and make the logging more clear? Otherwise, if you 
> > leave this out we really need to document it within the Kill Call. My hunch 
> > is that in writing that documentation we'll realize we should just do an 
> > override always.

Yeah, we can remove the check.


- Alexander


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


On April 21, 2016, 2:28 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46325/
> ---
> 
> (Updated April 21, 2016, 2:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Qian Zhang.
> 
> 
> Bugs: MESOS-4908
> https://issues.apache.org/jira/browse/MESOS-4908
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Kill policy can be provided in a kill event. In this case it should
> take precedence over kill policy specified when the task was launched.
> When kill event is issued multiple times during the task termination,
> the signal escalation timeout (the time a task has between SIGTERM
> and SIGKILL) may be reduced.
> 
> Since updating the delay timer (we use it for signal escalation delay)
> is currently not possible, we cancel the existing signal timer and set
> up a new one. `Clock::cancel()` guarantees that, if existed, the timer
> is removed before the function returns; hence we do not set up more
> than 1 timer for signal escalation delay.
> 
> 
> Diffs
> -
> 
>   src/launcher/http_command_executor.cpp 
> 9dfe48108cababeb2d4a6af74434880d79245c21 
> 
> Diff: https://reviews.apache.org/r/46325/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Tested manually using modified `mesos-execute` in a way that **two** extra 
> kill task requests are sent, 2s and 3s after receiving `TASK_RUNNING`. Each 
> kill task request specifies `KillPolicy` with 1s grace period. Together with 
> a kill *without* a kill policy scheduled 1s after the task is being launched, 
> the task receives **three** kill requests in total.
> 
> Master: `./bin/mesos-master.sh --work_dir=/tmp/w/m --ip=127.0.0.1`
> Agent: `./bin/mesos-slave.sh --work_dir=/tmp/w/s --master=127.0.0.1:5050 
> --http_command_executor`
> Mesos-execute: `./src/mesos-execute --master=127.0.0.1:5050 --name=test 
> --command="/Users/alex/bin/unresponsive_process" --env='{"GLOG_v": "2"}' 
> --kill_after=1secs`
> 
> HTTP command executor log:
> ```
> Received SUBSCRIBED event
> Subscribed executor on alexr.fritz.box
> Received LAUNCH event
> Starting task test
> sh -c '/Users/alex/bin/unresponsive_process'
> Forked command at 17475
> 14455919081943275466
> Received ACKNOWLEDGED event
> 17172602460659762152
> Received KILL event
> Received kill for task test
> Sending SIGTERM to process tree at pid 17475
> Sent SIGTERM to the following process trees:
> [ 
> --- 17475 /Users/alex/bin/unresponsive_process
> ]
> 4381544758593790168
> Scheduling escalation to SIGKILL in 3secs from now
> Received ACKNOWLEDGED event
> Received KILL event
> Received kill for task test
> Rescheduling escalation to SIGKILL in 1secs from now
> 10370891801885978953
> Process 17475 did not terminate after 1secs, sending 

Re: Review Request 46323: Propagated KillPolicy in kill task from scheduler to executor.

[Alexander Rukletsov]

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

(Updated April 22, 2016, 2:38 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

BenM's comment.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/internal/evolve.hpp cff6b657a6d69ff9e7f7dedc965122a03a200da1 
  src/internal/evolve.cpp 4ca2abacd75523de8fb4ffe69a9edd8627f512af 
  src/master/master.cpp 6dacf5fbd73771e5c31ffb0e8723cd2905dcefb3 
  src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
  src/slave/slave.cpp a365e8f5f8d63bf80614fccf6adf35a4fee07526 
  src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
  src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb 
  src/tests/slave_tests.cpp 3f653354869987dce3f5fbc4513b6f3466a718cb 

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


Testing
---

The whole chain is tested in https://reviews.apache.org/r/46325/


Thanks,

Alexander Rukletsov

Re: Review Request 46322: Added KillPolicy to scheduler and executor Kill protobuf messages.

[Alexander Rukletsov]

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

(Updated April 22, 2016, 2:37 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Clarified note in CHANGELOG.


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


Repository: mesos


Description
---

A framework may want to override the `KillPolicy` set in `TaskInfo`
when killing a task, for example to forcefully kill a task which is
already being killed.


Diffs (updated)
-

  CHANGELOG d2e902f8295644c527964123e409be460c2a5789 
  include/mesos/executor/executor.proto 
338b3638f986244122c2d39c9aca7905c12008ce 
  include/mesos/scheduler/scheduler.proto 
078c6550f24a3d8ac675251168434130fc3eeef3 
  include/mesos/v1/executor/executor.proto 
4552fb5d3f9d53affd8fad0abf122fce548973b7 
  include/mesos/v1/scheduler/scheduler.proto 
8ed9e19a9e5aa19a518b708b0e0d9cfdc038cd11 
  src/messages/messages.proto e0f1fca92d3ea8c29c095da31653c317873a934c 

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


Testing
---

The whole chain is tested in https://reviews.apache.org/r/46325/


Thanks,

Alexander Rukletsov

Re: Review Request 46491: Ensured escalated() is not called after reaped() in command executor.

[Alexander Rukletsov]

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

(Updated April 22, 2016, 2:36 p.m.)


Review request for mesos, Benjamin Bannier and Ben Mahler.


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


Repository: mesos


Description
---

In command executor, capture the state when a task is killed (i.e.,
reaped) in a flag and use this flag to prevent calls to `killTask()`
and `escalated()` when they are executed after the task is killed.


Diffs (updated)
-

  src/launcher/executor.cpp 9f1d2168bc4ddbce1bcd25ff38dc1c34714eb28b 
  src/launcher/http_command_executor.cpp 
2ce296740abac096aa85ad11dd4d191e42c1aa06 

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


Testing
---

`make check` on Mac OS 10.10.4


Thanks,

Alexander Rukletsov