Re: Review Request 39018: Added JSON parsing for Resources.

2015-10-06 Thread haosdent huang

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



src/common/resources.cpp (line 362)


Is it would more clear to split three functions, one is for parse old form, 
one is for parse json object from and third one is for parse json array form.


- haosdent huang


On Oct. 5, 2015, 9:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 5, 2015, 9:51 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 37024: Exposes mesos version information in components.

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [37024]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 2:02 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37024/
> ---
> 
> (Updated Oct. 6, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1841
> https://issues.apache.org/jira/browse/MESOS-1841
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add an endpoint exposes Apache Mesos build informations and version 
> information.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/exec/exec.cpp 7b51baaa8c08d248918974a3a22b6217e388bcb1 
>   src/local/main.cpp 18b2f0187637cd425d55c220f73faac5a1218f0f 
>   src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
>   src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
>   src/slave/main.cpp 364dc7fc7ab2e3cef01aea7267dafa014b60e2b9 
>   src/version/version.hpp PRE-CREATION 
>   src/version/version.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37024/diff/
> 
> 
> Testing
> ---
> 
> Manual test result:
> 
> ```
> $ curl http://localhost:5050/version 2>/dev/null|jq .
> 
> {
>   "version": "0.24.0",
>   "build_user": "haosdent",
>   "build_time": 1439702338,
>   "build_date": "2015-08-16 13:18:58"
> }
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 37996: Added property manager

2015-10-06 Thread Alexander Rojas


> On Oct. 6, 2015, 1:14 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp, lines 
> > 30-52
> > 
> >
> > Hm.. what will this abstraction be used for? I have a hard time 
> > understanding what this is abstracting, is this general enough to belong in 
> > stout?

To your first question, while performing HTTP Authentication, it will be used 
to check the authentication realm of the endpoint. I also though that it will 
allow the firewall rules "disable_endpoints" to be able to cover children 
endpoints and not only the full path given.

Which leads to the second question, the use cases are all built in libprocess, 
but given that this is a header only, template class, it makes sense to be in 
stout. 

And finally yes, I can think on multiple cases in which this abstraction is 
general enough, think of setting ownership of a directory, using this 
abstraction allows you to set ownership to one folder, and all its children 
automatically inherit the ownership unless explicitly stated otherwise. I 
already mention the one of setting authentication realms for endpoints or other 
rules that are passed to children. You can also set permissions in directories 
or files without having to create a node for each, etc.


- Alexander


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


On Oct. 5, 2015, 6:02 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 5, 2015, 6:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 38579: Refactored registry client

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [39013, 38443, 38579]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 2:33 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38579/
> ---
> 
> (Updated Oct. 6, 2015, 2:33 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Cleanup and broke big methods into smaller chunks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> c2040b48ea43fdb29766994c244273d3fa9ee3cd 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38579/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Review Request 39042: Updated comment referencing deprecated and removed /master/shutdown endpoint.

2015-10-06 Thread Joerg Schad

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

Review request for mesos and Bernd Mathiske.


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


Repository: mesos


Description
---

Updated comment referencing deprecated and removed /master/shutdown endpoint.


Diffs
-

  src/master/master.hpp 4bb65f0b6b77ea7324b0dee943602cfdb0f6a11c 

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


Testing
---


Thanks,

Joerg Schad



Re: Review Request 39042: Updated comment referencing deprecated and removed /master/shutdown endpoint.

2015-10-06 Thread Joerg Schad

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

(Updated Oct. 6, 2015, 8:18 a.m.)


Review request for mesos and Bernd Mathiske.


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


Repository: mesos


Description (updated)
---

/master/shutdown endpoint has been fully removed with 36732.


Diffs
-

  src/master/master.hpp 4bb65f0b6b77ea7324b0dee943602cfdb0f6a11c 

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


Testing
---


Thanks,

Joerg Schad



Re: Review Request 39042: Updated comment referencing deprecated and removed /master/shutdown endpoint.

2015-10-06 Thread Bernd Mathiske

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

Ship it!


Ship It!

- Bernd Mathiske


On Oct. 6, 2015, 1:18 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39042/
> ---
> 
> (Updated Oct. 6, 2015, 1:18 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-2697
> https://issues.apache.org/jira/browse/MESOS-2697
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> /master/shutdown endpoint has been fully removed with 36732.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 4bb65f0b6b77ea7324b0dee943602cfdb0f6a11c 
> 
> Diff: https://reviews.apache.org/r/39042/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 37996: Added property manager

2015-10-06 Thread Alexander Rojas

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

(Updated Oct. 6, 2015, 10:24 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Minor comment changes.


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


Repository: mesos


Description
---

Introduces the `InheritanceTree` class which allows to create a tree where 
nodes can be tag with some property. This property is then inherited by 
children nodes.

Two behaviors are implemented, overriding, i.e. Adding a property to the child 
node of another node with a property already will result in the ancestor 
property being lost. The second behavior, accumulating, takes a function and 
accumulates
properties of all ancestors.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
76e1674e08bbe65a4fdf86731823a61f231d6d12 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
94292f8a46ec31bbaf6e52f48109322bbe123f70 
  3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
PRE-CREATION 

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


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 39042: Updated comment referencing deprecated and removed /master/shutdown endpoint.

2015-10-06 Thread Guangya Liu

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

Ship it!


Ship It!

- Guangya Liu


On 十月 6, 2015, 8:18 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39042/
> ---
> 
> (Updated 十月 6, 2015, 8:18 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-2697
> https://issues.apache.org/jira/browse/MESOS-2697
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> /master/shutdown endpoint has been fully removed with 36732.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 4bb65f0b6b77ea7324b0dee943602cfdb0f6a11c 
> 
> Diff: https://reviews.apache.org/r/39042/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 38000: Added an API for libprocess users to interact with http::AuthenticatorManager

2015-10-06 Thread Alexander Rojas

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

(Updated Oct. 6, 2015, 10:49 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Adds functions which allows libprocess users to add handlers which require 
authentication as well as functions to install and remove authenticators.

Includes tests.


Diffs (updated)
-

  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/process.hpp 
8b086f296c80a43be2edaf496a04dadf0c64251a 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 
  3rdparty/libprocess/src/tests/http_tests.cpp 
38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 

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


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 38950: Http Authenticators can be loaded as modules from mesos.

2015-10-06 Thread Alexander Rojas

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

(Updated Oct. 6, 2015, 10:52 a.m.)


Review request for mesos, Adam B, Bernd Mathiske, and Till Toenshoff.


Changes
---

Forcing buildbot rebuild.


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


Repository: mesos


Description
---

Adds support for modularization of HTTP Authenticators. 

It includes an example of how to do it with the Basic HTTP Authenticator.


Diffs (updated)
-

  include/mesos/authentication/http/basic_authenticator_factory.hpp 
PRE-CREATION 
  include/mesos/module/http_authenticator.hpp PRE-CREATION 
  src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
  src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION 
  src/examples/test_http_authenticator_module.cpp PRE-CREATION 
  src/master/constants.hpp 51d477444c4d1a2c9dd2f32164ebffbb1fd5c8c2 
  src/master/constants.cpp 2b66b27783f18930010ee912e9977ea1647eba09 
  src/module/manager.cpp f9a0643a70bc9de1484599629041650493842c69 
  src/tests/http_authentication_tests.cpp PRE-CREATION 
  src/tests/module.hpp 0820978441aede18dae6d1701433bff705b8c3c2 
  src/tests/module.cpp edab0b37dcf0bd8e15d439726354039c1bbcd51f 

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


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 36180: Avoid multi writers write to same file in PortMappingIsolatorTests.

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [36180]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 3:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36180/
> ---
> 
> (Updated Oct. 6, 2015, 3:13 a.m.)
> 
> 
> Review request for mesos, Adam B, Ian Downes, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-2765
> https://issues.apache.org/jira/browse/MESOS-2765
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid multi writers write to same file in PortMappingIsolatorTests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/port_mapping_tests.cpp 
> feca2043503436ac9abac6017ae9059b3fcbed21 
> 
> Diff: https://reviews.apache.org/r/36180/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-10-06 Thread Joerg Schad

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

(Updated Oct. 6, 2015, 9:31 a.m.)


Review request for mesos, Alexander Rukletsov and Bernd Mathiske.


Changes
---

rebased


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


Repository: mesos


Description
---

Added /quota HTTP Endpoint for Quota handling.


Diffs (updated)
-

  src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
  src/master/http.cpp 4b9f9ed005a4af2897171659d15168955cc60660 
  src/master/master.hpp 9d957519bb0f717526af9b2717dc870fae93c20f 
  src/master/master.cpp 6bee4f351c3fd0fb72f64bbc863968e4786b318b 
  src/master/quota_handler.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Joerg Schad



Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

2015-10-06 Thread Joerg Schad


> On Sept. 8, 2015, 7:14 a.m., Qian Zhang wrote:
> > src/master/master.hpp, line 901
> > 
> >
> > Why we add CALL_HELP back?

resolved.


- Joerg


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


On Oct. 6, 2015, 9:31 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36913/
> ---
> 
> (Updated Oct. 6, 2015, 9:31 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Bernd Mathiske.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added /quota HTTP Endpoint for Quota handling.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/master/http.cpp 4b9f9ed005a4af2897171659d15168955cc60660 
>   src/master/master.hpp 9d957519bb0f717526af9b2717dc870fae93c20f 
>   src/master/master.cpp 6bee4f351c3fd0fb72f64bbc863968e4786b318b 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36913/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>



Re: Review Request 37023: Add an endpoint that exposes component flags.

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [39037, 37023]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 4:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37023/
> ---
> 
> (Updated Oct. 6, 2015, 4:15 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add an endpoint that exposes component flags.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4b9f9ed005a4af2897171659d15168955cc60660 
>   src/master/master.hpp 4bb65f0b6b77ea7324b0dee943602cfdb0f6a11c 
>   src/master/master.cpp 6bee4f351c3fd0fb72f64bbc863968e4786b318b 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/37023/diff/
> 
> 
> Testing
> ---
> 
> manual test wich mesos-local.
> ```
> $ curl http://localhost:5050/master/flags 2>/dev/null|jq .
> {
>   "flags": {
> "allocation_interval": "1secs",
> "allocator": "HierarchicalDRF",
> "authenticate": "false",
> "authenticate_slaves": "false",
> "authenticators": "crammd5",
> "framework_sorter": "drf",
> "help": "true",
> "initialize_driver_logging": "true",
> "log_auto_initialize": "true",
> "logbufsecs": "0",
> "logging_level": "INFO",
> "max_slave_ping_timeouts": "5",
> "quiet": "false",
> "recovery_slave_removal_limit": "100%",
> "registry": "replicated_log",
> "registry_fetch_timeout": "1mins",
> "registry_store_timeout": "5secs",
> "registry_strict": "false",
> "root_submissions": "true",
> "slave_ping_timeout": "15secs",
> "slave_reregister_timeout": "10mins",
> "user_sorter": "drf",
> "version": "false",
> "webui_dir": "/home/haosdent/mesos/build/../src/webui",
> "work_dir": "/tmp/mesos",
> "zk_session_timeout": "10secs"
>   }
> }
> ```
> 
> ```
> $ curl http://localhost:5050/slave(1)/flags 2>/dev/null|jq .
> {
>   "flags": {
> "authenticatee": "crammd5",
> "cgroups_cpu_enable_pids_and_tids_count": "false",
> "cgroups_enable_cfs": "false",
> "cgroups_hierarchy": "/sys/fs/cgroup",
> "cgroups_limit_swap": "false",
> "cgroups_root": "mesos",
> "container_disk_watch_interval": "15secs",
> "containerizers": "mesos",
> "default_role": "*",
> "disk_watch_interval": "1mins",
> "docker": "docker",
> "docker_kill_orphans": "true",
> "docker_remove_delay": "6hrs",
> "docker_socket": "/var/run/docker.sock",
> "docker_stop_timeout": "0ns",
> "enforce_container_disk_quota": "false",
> "executor_registration_timeout": "1mins",
> "executor_shutdown_grace_period": "5secs",
> "fetcher_cache_dir": "/tmp/mesos/fetch",
> "fetcher_cache_size": "2GB",
> "frameworks_home": "",
> "gc_delay": "1weeks",
> "gc_disk_headroom": "0.1",
> "hadoop_home": "",
> "help": "false",
> "initialize_driver_logging": "true",
> "isolation": "posix/cpu,posix/mem",
> "launcher_dir": "/home/haosdent/mesos/build/src",
> "logbufsecs": "0",
> "logging_level": "INFO",
> "oversubscribed_resources_interval": "15secs",
> "perf_duration": "10secs",
> "perf_interval": "1mins",
> "qos_correction_interval_min": "0ns",
> "quiet": "false",
> "recover": "reconnect",
> "recovery_timeout": "15mins",
> "registration_backoff_factor": "1secs",
> "resource_monitoring_interval": "1secs",
> "revocable_cpu_low_priority": "true",
> "sandbox_directory": "/mnt/mesos/sandbox",
> "strict": "true",
> "switch_user": "true",
> "version": "false",
> "work_dir": "/tmp/mesos/0"
>   }
> }
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38950: Http Authenticators can be loaded as modules from mesos.

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [37714, 37996, 37997, 37998, 37999, 38000, 38094, 38627, 38950]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 8:52 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38950/
> ---
> 
> (Updated Oct. 6, 2015, 8:52 a.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds support for modularization of HTTP Authenticators. 
> 
> It includes an example of how to do it with the Basic HTTP Authenticator.
> 
> 
> Diffs
> -
> 
>   include/mesos/authentication/http/basic_authenticator_factory.hpp 
> PRE-CREATION 
>   include/mesos/module/http_authenticator.hpp PRE-CREATION 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/authentication/http/basic_authenticator_factory.cpp PRE-CREATION 
>   src/examples/test_http_authenticator_module.cpp PRE-CREATION 
>   src/master/constants.hpp 51d477444c4d1a2c9dd2f32164ebffbb1fd5c8c2 
>   src/master/constants.cpp 2b66b27783f18930010ee912e9977ea1647eba09 
>   src/module/manager.cpp f9a0643a70bc9de1484599629041650493842c69 
>   src/tests/http_authentication_tests.cpp PRE-CREATION 
>   src/tests/module.hpp 0820978441aede18dae6d1701433bff705b8c3c2 
>   src/tests/module.cpp edab0b37dcf0bd8e15d439726354039c1bbcd51f 
> 
> Diff: https://reviews.apache.org/r/38950/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 37853: Overlay filesystem provisioning backend

2015-10-06 Thread Mei Wan

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

(Updated Oct. 6, 2015, 12:35 p.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Jie Yu.


Changes
---

Changing branch


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


Repository: mesos


Description
---

Implemented the overlay filesystem backend by layering the images as a 
read-only filesystem.


Diffs
-

  src/Makefile.am e698927 
  src/slave/containerizer/provisioner/backend.cpp b5d9670 
  src/slave/containerizer/provisioner/backends/overlay.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/backends/overlay.cpp PRE-CREATION 

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


Testing
---

I haven't done any official testing. When I was working off Ian's branch, I 
tested it manually and the provisioning works.


Thanks,

Mei Wan



Re: Review Request 37853: Overlay filesystem provisioning backend

2015-10-06 Thread Mei Wan

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

(Updated Oct. 6, 2015, 12:36 p.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Jie Yu.


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


Repository: mesos


Description
---

Implemented the overlay filesystem backend by layering the images as a 
read-only filesystem.


Diffs
-

  src/Makefile.am e698927 
  src/slave/containerizer/provisioner/backend.cpp b5d9670 
  src/slave/containerizer/provisioner/backends/overlay.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/backends/overlay.cpp PRE-CREATION 

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


Testing
---

I haven't done any official testing. When I was working off Ian's branch, I 
tested it manually and the provisioning works.


Thanks,

Mei Wan



Re: Review Request 37853: Overlay filesystem provisioning backend

2015-10-06 Thread Mei Wan

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

(Updated Oct. 6, 2015, 12:38 p.m.)


Review request for mesos, Ben Mahler, Chi Zhang, Ian Downes, and Jie Yu.


Changes
---

Sorry the diff isn't uploading for some reason.


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


Repository: mesos


Description
---

Implemented the overlay filesystem backend by layering the images as a 
read-only filesystem.


Diffs
-

  src/Makefile.am e698927 
  src/slave/containerizer/provisioner/backend.cpp b5d9670 
  src/slave/containerizer/provisioner/backends/overlay.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/backends/overlay.cpp PRE-CREATION 

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


Testing
---

I haven't done any official testing. When I was working off Ian's branch, I 
tested it manually and the provisioning works.


Thanks,

Mei Wan



Re: Review Request 37996: Added property manager

2015-10-06 Thread Bernd Mathiske

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

Ship it!


Ship It!

- Bernd Mathiske


On Oct. 6, 2015, 1:24 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 6, 2015, 1:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 38337: Extract gz file in fetcher.

2015-10-06 Thread Bernd Mathiske

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


I am wondering whether gzip is always available on all platforms. Can we rely 
on this? Even on CoreOS? And if we cannot, do we give an appropriate error 
message that is informative enough?

- Bernd Mathiske


On Oct. 5, 2015, 2:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38337/
> ---
> 
> (Updated Oct. 5, 2015, 2:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, and Timothy Chen.
> 
> 
> Bugs: MESOS-3407
> https://issues.apache.org/jira/browse/MESOS-3407
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract gz file in fetcher.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0f1533a0d7dc453e143a15e988d04ca6e55446ff 
>   src/tests/fetcher_tests.cpp 8d13352d0d3f8fb80581e7913c9416b543cfd009 
> 
> Diff: https://reviews.apache.org/r/38337/diff/
> 
> 
> Testing
> ---
> 
> sudo GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="FetcherTest.ExtractGzipFile" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38337: Extract gz file in fetcher.

2015-10-06 Thread haosdent huang


> On Oct. 6, 2015, 12:54 p.m., Bernd Mathiske wrote:
> > I am wondering whether gzip is always available on all platforms. Can we 
> > rely on this? Even on CoreOS? And if we cannot, do we give an appropriate 
> > error message that is informative enough?

gzip and tar are available in fresh CoreOS/CentOS/Ubuntu. When use extract 
`*.tar.gz` package, tar would call `gzip` to get `*.tar` first.


- haosdent


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


On Oct. 5, 2015, 9:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38337/
> ---
> 
> (Updated Oct. 5, 2015, 9:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, and Timothy Chen.
> 
> 
> Bugs: MESOS-3407
> https://issues.apache.org/jira/browse/MESOS-3407
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract gz file in fetcher.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0f1533a0d7dc453e143a15e988d04ca6e55446ff 
>   src/tests/fetcher_tests.cpp 8d13352d0d3f8fb80581e7913c9416b543cfd009 
> 
> Diff: https://reviews.apache.org/r/38337/diff/
> 
> 
> Testing
> ---
> 
> sudo GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="FetcherTest.ExtractGzipFile" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38337: Extract gz file in fetcher.

2015-10-06 Thread Bernd Mathiske

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

Ship it!


Ship It!

- Bernd Mathiske


On Oct. 5, 2015, 2:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38337/
> ---
> 
> (Updated Oct. 5, 2015, 2:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, and Timothy Chen.
> 
> 
> Bugs: MESOS-3407
> https://issues.apache.org/jira/browse/MESOS-3407
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract gz file in fetcher.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0f1533a0d7dc453e143a15e988d04ca6e55446ff 
>   src/tests/fetcher_tests.cpp 8d13352d0d3f8fb80581e7913c9416b543cfd009 
> 
> Diff: https://reviews.apache.org/r/38337/diff/
> 
> 
> Testing
> ---
> 
> sudo GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="FetcherTest.ExtractGzipFile" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38337: Extract gz file in fetcher.

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38337]

All tests passed.

- Mesos ReviewBot


On Oct. 5, 2015, 9:26 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38337/
> ---
> 
> (Updated Oct. 5, 2015, 9:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, and Timothy Chen.
> 
> 
> Bugs: MESOS-3407
> https://issues.apache.org/jira/browse/MESOS-3407
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract gz file in fetcher.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0f1533a0d7dc453e143a15e988d04ca6e55446ff 
>   src/tests/fetcher_tests.cpp 8d13352d0d3f8fb80581e7913c9416b543cfd009 
> 
> Diff: https://reviews.apache.org/r/38337/diff/
> 
> 
> Testing
> ---
> 
> sudo GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="FetcherTest.ExtractGzipFile" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Review Request 39043: Added support for HTTP Authentication in Mesos.

2015-10-06 Thread Alexander Rojas

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

Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, and Till 
Toenshoff.


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


Repository: mesos


Description
---

1. Adds a flag to load an HTTP Authenticator module from the flags.
2. If provided, uses the credentials file to initialize the default HTTP 
Authenticator.
3. Updates the existing endpoints which implement their own basic HTTP 
authenticator with the libprocess one.
4. Updates one test which expected the wrong results since now credentials are 
checked before the body of the request.


Diffs
-

  src/master/constants.hpp 51d477444c4d1a2c9dd2f32164ebffbb1fd5c8c2 
  src/master/constants.cpp 2b66b27783f18930010ee912e9977ea1647eba09 
  src/master/flags.hpp 5fd5d502697b2edc22ae98a5a8e361bf62bf8bb6 
  src/master/flags.cpp 0285ce70cefca09e81ef7137968d024e297fec87 
  src/master/http.cpp 4b9f9ed005a4af2897171659d15168955cc60660 
  src/master/master.hpp 9d957519bb0f717526af9b2717dc870fae93c20f 
  src/master/master.cpp 6bee4f351c3fd0fb72f64bbc863968e4786b318b 
  src/tests/teardown_tests.cpp 2eeead74b877e282660c4009910c42d93ef3ff46 

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


Testing
---

make check


Thanks,

Alexander Rojas



Review Request 39005: stout: Added thread-safe replacement for strerror.

2015-10-06 Thread Benjamin Bannier

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Summary (updated)
-

stout: Added thread-safe replacement for strerror.


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


Repository: mesos


Description (updated)
---

This adds a thread-safe wrapper around strerror_r which has semantics similar 
to strerror. We plan to use this at call sites currently relying on strerror.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp 
12ba1ca861114e60f8276c0ee91c543abcfc2519 
  3rdparty/libprocess/3rdparty/stout/tests/error_tests.cpp 
9e7605c53e6636e7fea32e4f69fbaff9100a979f 

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


Testing (updated)
---

make check


Thanks,

Benjamin Bannier



Review Request 39006: stout: Used thread-safe replacement for strerror.

2015-10-06 Thread Benjamin Bannier

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Summary (updated)
-

stout: Used thread-safe replacement for strerror.


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


Repository: mesos


Description (updated)
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
d1e2df6151149e03ffb4a76e2c24ff78b891e016 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
7eb51e8771e95f57548fc35753e75c6d56cd97cd 
  3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp 
e740d5bc0f0cc5cf8e99b2064c1e39c08282da67 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
e6d36ec1bf414b52d0899f0edf83e0ad8910dd0e 

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


Testing (updated)
---

make check


Thanks,

Benjamin Bannier



Review Request 39008: Used thread-safe replacement for strerror.

2015-10-06 Thread Benjamin Bannier

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Summary (updated)
-

Used thread-safe replacement for strerror.


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


Repository: mesos


Description (updated)
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  src/cli/mesos.cpp 80c3c1a7e30e7e148e17c379ec6824ab7e4c0f12 
  src/files/files.cpp 08e76b95b632b6fb9c82666550d0ae3c4e1a1a89 
  src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
  src/linux/routing/link/internal.hpp 015c0ef5be516d7786c96a96437cced1ae8487fa 
  src/linux/routing/link/link.cpp 8ea3e31e0f64c7b653f208ec74bb389a702b357a 
  src/slave/containerizer/docker.cpp 6c975f904178e01797b67628a2d471ec7b3b1fbf 
  src/slave/containerizer/isolators/filesystem/linux.cpp 
8823b7850a1ac17fc4f4868aadf1b04428d2381b 
  src/slave/containerizer/isolators/filesystem/posix.cpp 
eec510c4f7655d67b33ad90210eeb57fcc910684 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
73804ca5a8a3bf03e13c74a247b5c21e9af5f040 
  src/slave/containerizer/mesos/containerizer.cpp 
b904b2d88e9b62fa4ba312c4569a4d89b0dc6052 
  src/slave/containerizer/mesos/launch.cpp 
09d4d8f4d6837e93a82deef76ca07e2167d6a405 
  src/slave/containerizer/provisioner/backends/bind.cpp 
1fe1746c0bc1c9c12e1378e6438122a91b58316b 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
  src/tests/containerizer/memory_test_helper.cpp 
8109a4314c0dcf17c5fe124d9b87ac856b3a922a 
  src/tests/script.cpp bcc1fab912410237dfe903d7a36cad9323d625a0 

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


Testing (updated)
---

make check


Thanks,

Benjamin Bannier



Review Request 39007: libprocess: Used thread-safe replacement for strerror.

2015-10-06 Thread Benjamin Bannier

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

Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.


Summary (updated)
-

libprocess: Used thread-safe replacement for strerror.


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


Repository: mesos


Description (updated)
---

Switch call sites to using safe strerror_r wrapper.


Diffs (updated)
-

  3rdparty/libprocess/src/io.cpp 26686e1a96484e3f09d41a7292f38b7579ce9c48 
  3rdparty/libprocess/src/poll_socket.cpp 
28ed102972a9d8f88048aea4046ed837b6a25b35 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 
  3rdparty/libprocess/src/profiler.cpp 0c515568880aa6b7a65cfe2955eb7132bdfb3baf 
  3rdparty/libprocess/src/subprocess.cpp 
a457cbe35ad33531c49f74550cd570cf28758f5d 

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


Testing (updated)
---

make check


Thanks,

Benjamin Bannier



Re: Review Request 39043: Added support for HTTP Authentication in Mesos.

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [37714, 37996, 37997, 37998, 37999, 38000, 38094, 38627, 
38950, 39043]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 2:56 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39043/
> ---
> 
> (Updated Oct. 6, 2015, 2:56 p.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, Isabel Jimenez, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 1. Adds a flag to load an HTTP Authenticator module from the flags.
> 2. If provided, uses the credentials file to initialize the default HTTP 
> Authenticator.
> 3. Updates the existing endpoints which implement their own basic HTTP 
> authenticator with the libprocess one.
> 4. Updates one test which expected the wrong results since now credentials 
> are checked before the body of the request.
> 
> 
> Diffs
> -
> 
>   src/master/constants.hpp 51d477444c4d1a2c9dd2f32164ebffbb1fd5c8c2 
>   src/master/constants.cpp 2b66b27783f18930010ee912e9977ea1647eba09 
>   src/master/flags.hpp 5fd5d502697b2edc22ae98a5a8e361bf62bf8bb6 
>   src/master/flags.cpp 0285ce70cefca09e81ef7137968d024e297fec87 
>   src/master/http.cpp 4b9f9ed005a4af2897171659d15168955cc60660 
>   src/master/master.hpp 9d957519bb0f717526af9b2717dc870fae93c20f 
>   src/master/master.cpp 6bee4f351c3fd0fb72f64bbc863968e4786b318b 
>   src/tests/teardown_tests.cpp 2eeead74b877e282660c4009910c42d93ef3ff46 
> 
> Diff: https://reviews.apache.org/r/39043/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

2015-10-06 Thread Anand Mazumdar


> On Oct. 6, 2015, 6:20 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2479
> > 
> >
> > Seems this line is incomplete.

It was intentionally left this way to avoid warning as error issues. This would 
be fixed when `MESOS-3476` is resolved. (Read comments before)


- Anand


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


On Sept. 30, 2015, 6:06 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Sept. 30, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

2015-10-06 Thread Anand Mazumdar

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

(Updated Oct. 6, 2015, 4:39 p.m.)


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


Changes
---

Moved `HTTP` executor checks outside else.


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


Repository: mesos


Description
---

This change adds the functionality for executors to `Subscribe` via the 
`api/v1/executor` endpoint. It also stores a marker file as part of the 
`Subscribe` call if framework `checkpointing` is enabled. This can then be used 
by the agent when recovering to wait for reconnecting back with the executor.

Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
`CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
the `Subscribe` call.


Diffs (updated)
-

  src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
  src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
  src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 

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


Testing
---

make check


Thanks,

Anand Mazumdar



Review Request 39053: RegistryClient refactor: priv method const'ness

2015-10-06 Thread Jojy Varghese

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

RegistryClient refactor: priv method const'ness


Diffs
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
c2040b48ea43fdb29766994c244273d3fa9ee3cd 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38580: Added docker registry RemotePuller

2015-10-06 Thread Jojy Varghese

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

(Updated Oct. 6, 2015, 4:44 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

Fixed test.


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
4a0b7d11f013941084571f2d89d835a4668a3d8b 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
c2040b48ea43fdb29766994c244273d3fa9ee3cd 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
cbb67686d45513f0395a0cf1bc5c43cb4935adae 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38570: Change documentation image links to absolute paths.

2015-10-06 Thread Joseph Wu

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

(Updated Oct. 6, 2015, 9:45 a.m.)


Review request for mesos, Adam B, Dave Lester, Artem Harutyunyan, Kapil Arya, 
and Vinod Kone.


Changes
---

Update image URL of the recently added networking doc.


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


Repository: mesos


Description
---

Also removes extraneous `?raw=true` from the links.


Diffs (updated)
-

  docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
  docs/fetcher-cache-internals.md e8a68d1230420d1afca61f92c1ab6be12a70dbf2 
  docs/maintenance.md a5831ffa092a9ea6decbe2e640bae637c759a308 
  docs/networking-for-mesos-managed-containers.md 
33568a8233523ff3805f962371c94346e608208b 
  docs/oversubscription.md 5a31b1ff7f003307817732a71f3b0a7c7d60cd24 

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


Testing
---

Patched `Rakefile` to include images in the website (See JIRA for the patch).
Then rendered with: https://github.com/mesosphere/mesos-website-container

Confirmed that images show up on the modified docs.


Thanks,

Joseph Wu



Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

2015-10-06 Thread haosdent huang


> On Oct. 6, 2015, 1:55 a.m., Cong Wang wrote:
> > src/tests/containerizer/isolator_tests.cpp, line 1206
> > 
> >
> > Looks like you are replicating the same code from 
> > ContainerizerTest::SetUp(), why that code is not 
> > enough and is it possible to reuse that code?

I am not sure UserCgroupIsolatorTest should extends from ContainerizerTest or 
not before. Let me make UserCgroupIsolatorTest extends ContainerizerTest.


- haosdent


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


On Oct. 3, 2015, 4:15 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38527/
> ---
> 
> (Updated Oct. 3, 2015, 4:15 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-3470
> https://issues.apache.org/jira/browse/MESOS-3470
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix UserCgroupIsolatorTest failed on CentOS 6.6.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 237f3f27722b01ff92d0dcbaba7910613542a1a7 
> 
> Diff: https://reviews.apache.org/r/38527/diff/
> 
> 
> Testing
> ---
> 
> # In CentOS 6.6
> sudo ./bin/mesos-tests.sh --gtest_filter="UserCgroupIsolatorTest*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

2015-10-06 Thread haosdent huang

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

(Updated Oct. 6, 2015, 4:47 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.


Changes
---

Update according @wangcong's review.


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


Repository: mesos


Description
---

Fix UserCgroupIsolatorTest failed on CentOS 6.6.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
237f3f27722b01ff92d0dcbaba7910613542a1a7 
  src/tests/mesos.hpp 3e58b454c75a2ab9f8b4a29785fa823afefd0c8a 
  src/tests/mesos.cpp ab2d85b091d121113931e4190a5b496901dcd7a5 

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


Testing
---

# In CentOS 6.6
sudo ./bin/mesos-tests.sh --gtest_filter="UserCgroupIsolatorTest*" --verbose


Thanks,

haosdent huang



Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

2015-10-06 Thread Anand Mazumdar


> On Oct. 6, 2015, 6:35 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2420
> > 
> >
> > Should use return here directly instead of break?

What is the advantage of one over the other here ? I was being consistent with 
`(re-)registerExecutor(...)` functions in the same file.


> On Oct. 6, 2015, 6:35 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 4244
> > 
> >
> > I just suggest if it possible to change it like this pattern.
> > 
> > ```
> > if (executor->pid.isSome() && executor->pid.get()) {
> > // Normal executor
> > } else if (executor->pid.isNone()) {
> > // Http executor
> > } else {
> > // Other cases
> > }
> > ```
> > 
> > My concern is `else` maybe have more exception cases in the future, 
> > assume the only case in `else` is http executor seems not always match.

Good suggestion. Fixed.


> On Oct. 6, 2015, 6:35 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2545
> > 
> >
> > When execute is in unexpected state, we keep http connection open and 
> > not close it?

We are not leaking any descriptors. The OS would reclaim these after the crash 
on this line.


- Anand


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


On Sept. 30, 2015, 6:06 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Sept. 30, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

2015-10-06 Thread Anand Mazumdar


> On Oct. 6, 2015, 1:04 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2389
> > 
> >
> > I think just print a number is not easy to understand when troubleshoot 
> > error happens.

Can you elaborate a bit more on this ?


- Anand


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


On Sept. 30, 2015, 6:06 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Sept. 30, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

2015-10-06 Thread haosdent huang


> On Oct. 6, 2015, 6:20 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2479
> > 
> >
> > Seems this line is incomplete.
> 
> Anand Mazumdar wrote:
> It was intentionally left this way to avoid warning as error issues. This 
> would be fixed when `MESOS-3476` is resolved. (Read comments before)

got it, thx for explanation. I did not realize compiler would show error when 
update not used.


- haosdent


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


On Oct. 6, 2015, 4:39 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Oct. 6, 2015, 4:39 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

2015-10-06 Thread haosdent huang


> On Oct. 6, 2015, 1:04 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2389
> > 
> >
> > I think just print a number is not easy to understand when troubleshoot 
> > error happens.
> 
> Anand Mazumdar wrote:
> Can you elaborate a bit more on this ?

never mind, I just now found other places also just print `state`.


- haosdent


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


On Oct. 6, 2015, 4:39 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> ---
> 
> (Updated Oct. 6, 2015, 4:39 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
> https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the functionality for executors to `Subscribe` via the 
> `api/v1/executor` endpoint. It also stores a marker file as part of the 
> `Subscribe` call if framework `checkpointing` is enabled. This can then be 
> used by the agent when recovering to wait for reconnecting back with the 
> executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a 
> `CHECK` if a executor tries to send a list of unacknowledged tasks as part of 
> the `Subscribe` call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 36180: Avoid multi writers write to same file in PortMappingIsolatorTests.

2015-10-06 Thread haosdent huang


> On Oct. 6, 2015, 3:20 a.m., Cong Wang wrote:
> > src/tests/containerizer/port_mapping_tests.cpp, line 270
> > 
> >
> > traffic_invalid_via_loopback is not good either, because there is no 
> > traffic from an invalid port

how about `traffice_empty_via_loopback` and `traffic_empty_via_public`


- haosdent


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


On Oct. 6, 2015, 3:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36180/
> ---
> 
> (Updated Oct. 6, 2015, 3:13 a.m.)
> 
> 
> Review request for mesos, Adam B, Ian Downes, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-2765
> https://issues.apache.org/jira/browse/MESOS-2765
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoid multi writers write to same file in PortMappingIsolatorTests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/port_mapping_tests.cpp 
> feca2043503436ac9abac6017ae9059b3fcbed21 
> 
> Diff: https://reviews.apache.org/r/36180/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



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

2015-10-06 Thread Joseph Wu

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



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(lines 41 - 43)


Is this necessary?  (Do the calls to `FindNextFile`, `FindClose`, or 
`FindFirstFile` require C or something?)



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


I'm not really sure how necessary this comment is.  (And the two identical 
notes below.)

Presumably, we already know this because:
1) This is Stout.
2) This file is in the `internal` folder.



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


Seems like this will give a negative array access if you set `path = "";`.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(lines 165 - 166)


Opening curly brace should be moved up.


- Joseph Wu


On Oct. 5, 2015, 3:12 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39019/
> ---
> 
> (Updated Oct. 5, 2015, 3:12 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dirent compat code for non-Unix systems.
> 
> 
> Diffs
> -
> 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 50e35f43d87c69a83a9e7d039d1881404ea8be38 
> 
> Diff: https://reviews.apache.org/r/39019/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39005: stout: Added thread-safe replacement for strerror.

2015-10-06 Thread Cong Wang

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



3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp (line 36)


When strerror_r() returns EINVAL, you return an empty string?


- Cong Wang


On Oct. 6, 2015, 3:07 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39005/
> ---
> 
> (Updated Oct. 6, 2015, 3:07 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a thread-safe wrapper around strerror_r which has semantics similar 
> to strerror. We plan to use this at call sites currently relying on strerror.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp 
> 12ba1ca861114e60f8276c0ee91c543abcfc2519 
>   3rdparty/libprocess/3rdparty/stout/tests/error_tests.cpp 
> 9e7605c53e6636e7fea32e4f69fbaff9100a979f 
> 
> Diff: https://reviews.apache.org/r/39005/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 39005: stout: Added thread-safe replacement for strerror.

2015-10-06 Thread Cong Wang


> On Oct. 6, 2015, 5:09 p.m., Cong Wang wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp, line 37
> > 
> >
> > When strerror_r() returns EINVAL, you return an empty string?

Also, you should force the XSI-compliant one rather than the GNU one which 
returns a pointer.


- Cong


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


On Oct. 6, 2015, 3:07 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39005/
> ---
> 
> (Updated Oct. 6, 2015, 3:07 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
> https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a thread-safe wrapper around strerror_r which has semantics similar 
> to strerror. We plan to use this at call sites currently relying on strerror.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp 
> 12ba1ca861114e60f8276c0ee91c543abcfc2519 
>   3rdparty/libprocess/3rdparty/stout/tests/error_tests.cpp 
> 9e7605c53e6636e7fea32e4f69fbaff9100a979f 
> 
> Diff: https://reviews.apache.org/r/39005/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



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

2015-10-06 Thread Gilbert Song

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

(Updated Oct. 6, 2015, 10:29 a.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

Serialize Docker Image Spec as Protobuf


Diffs (updated)
-

  src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
  src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
  src/slave/containerizer/provisioner/docker/message.proto 
bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (ubuntu 14.04 + clang++-3.6)


Thanks,

Gilbert Song



Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Mandeep Chadha

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

Review request for mesos.


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


Repository: mesos


Description
---

Check failed due to double comparison : MESOS-3552.


Diffs
-

  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 

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


Testing
---

Added unit test.
make check successful.


Thanks,

Mandeep Chadha



Re: Review Request 38570: Change documentation image links to absolute paths.

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38570]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 4:45 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38570/
> ---
> 
> (Updated Oct. 6, 2015, 4:45 p.m.)
> 
> 
> Review request for mesos, Adam B, Dave Lester, Artem Harutyunyan, Kapil Arya, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3183
> https://issues.apache.org/jira/browse/MESOS-3183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also removes extraneous `?raw=true` from the links.
> 
> 
> Diffs
> -
> 
>   docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
>   docs/fetcher-cache-internals.md e8a68d1230420d1afca61f92c1ab6be12a70dbf2 
>   docs/maintenance.md a5831ffa092a9ea6decbe2e640bae637c759a308 
>   docs/networking-for-mesos-managed-containers.md 
> 33568a8233523ff3805f962371c94346e608208b 
>   docs/oversubscription.md 5a31b1ff7f003307817732a71f3b0a7c7d60cd24 
> 
> Diff: https://reviews.apache.org/r/38570/diff/
> 
> 
> Testing
> ---
> 
> Patched `Rakefile` to include images in the website (See JIRA for the patch).
> Then rendered with: https://github.com/mesosphere/mesos-website-container
> 
> Confirmed that images show up on the modified docs.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Neil Conway

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



src/common/resources.cpp (line 870)


This change should also be applied to Resources::apply() in 
src/v1/resources.cpp



src/common/resources.cpp (line 871)


Capitalize "check"; also, maybe include a brief explanation of why we might 
see some error in the floating point calculation.



src/common/resources.cpp (line 872)


Should be "constexpr", maybe "static" too.



src/tests/reservation_tests.cpp (line 175)


I think the comment here should explain why using a double value is 
relevant.



src/tests/reservation_tests.cpp (line 245)


Whitespace/capitalization.


- Neil Conway


On Oct. 6, 2015, 5:59 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 5:59 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 38919: Validation of Docker Image Manifests

2015-10-06 Thread Gilbert Song

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

(Updated Oct. 6, 2015, 12:08 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

Validation of Docker Image Manifests


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (Ubuntu14.04 + clang++-3.6)


Thanks,

Gilbert Song



Re: Review Request 39037: Allow description empty in help information.

2015-10-06 Thread Ben Mahler

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



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


Please use an Option :)


- Ben Mahler


On Oct. 6, 2015, 4:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> ---
> 
> (Updated Oct. 6, 2015, 4:15 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow description empty in help information.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> e7dc670648e8abd2fef7be79835f0b71e3e91258 
>   3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7 
> 
> Diff: https://reviews.apache.org/r/39037/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38527]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 4:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38527/
> ---
> 
> (Updated Oct. 6, 2015, 4:47 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-3470
> https://issues.apache.org/jira/browse/MESOS-3470
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix UserCgroupIsolatorTest failed on CentOS 6.6.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 237f3f27722b01ff92d0dcbaba7910613542a1a7 
>   src/tests/mesos.hpp 3e58b454c75a2ab9f8b4a29785fa823afefd0c8a 
>   src/tests/mesos.cpp ab2d85b091d121113931e4190a5b496901dcd7a5 
> 
> Diff: https://reviews.apache.org/r/38527/diff/
> 
> 
> Testing
> ---
> 
> # In CentOS 6.6
> sudo ./bin/mesos-tests.sh --gtest_filter="UserCgroupIsolatorTest*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 37024: Exposes mesos version information in components.

2015-10-06 Thread Ben Mahler

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


Would be great to add a unit test for VersionProcess as a follow up, not in 
this patch.


src/version/version.hpp (lines 40 - 41)


Looks like this should be static?



src/version/version.cpp (lines 56 - 58)


Couple of things:

(1) Can you use a 2 space indent instead of 4?

(2) Can you add the git fields here as well?

```
"{",
"  \"build_user\":\"username\",",
"  \"build_time\":1443894750,",
"  \"build_date\":\"2015-10-04 01:52:30\",",
"  \"git_branch\":\"branch\", // Optional",
"  \"git_sha\":\"2199a599d4e57cce0c9209660e488f530156e07b\", // Optional",
"  \"git_tag\":\"0.26.0-rc1\", // Optional",
"  \"version\":\"0.26.0\"",
"}",
```

(3) Ideally we could just make VersionProcess::version static and then just 
pretty print the json object directly here instead of hard coding an example. 
However, we don't currently have pretty printing, so feel free to just leave 
this example here and have a TODO for generating the example automatically.


- Ben Mahler


On Oct. 6, 2015, 2:02 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37024/
> ---
> 
> (Updated Oct. 6, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1841
> https://issues.apache.org/jira/browse/MESOS-1841
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add an endpoint exposes Apache Mesos build informations and version 
> information.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/exec/exec.cpp 7b51baaa8c08d248918974a3a22b6217e388bcb1 
>   src/local/main.cpp 18b2f0187637cd425d55c220f73faac5a1218f0f 
>   src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
>   src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
>   src/slave/main.cpp 364dc7fc7ab2e3cef01aea7267dafa014b60e2b9 
>   src/version/version.hpp PRE-CREATION 
>   src/version/version.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37024/diff/
> 
> 
> Testing
> ---
> 
> Manual test result:
> 
> ```
> $ curl http://localhost:5050/version 2>/dev/null|jq .
> 
> {
>   "version": "0.24.0",
>   "build_user": "haosdent",
>   "build_time": 1439702338,
>   "build_date": "2015-08-16 13:18:58"
> }
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 39013: RegistryClient refactor: Fixed comments style.

2015-10-06 Thread Ben Mahler

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

Ship it!


Thanks for pulling away the dependencies, I'll commit this shortly without the 
const change.


src/slave/containerizer/provisioner/docker/registry_client.hpp (line 53)


Ah, please avoid slipping in other changes like this, looks like this 
belongs in your layer id patch.


- Ben Mahler


On Oct. 6, 2015, 2:30 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39013/
> ---
> 
> (Updated Oct. 6, 2015, 2:30 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RegistryClient refactor: Fixed comments style.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> c2040b48ea43fdb29766994c244273d3fa9ee3cd 
> 
> Diff: https://reviews.apache.org/r/39013/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 38919: Validation of Docker Image Manifests

2015-10-06 Thread Timothy Chen

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



src/slave/containerizer/provisioner/docker/spec.cpp (line 62)


Align this next to (



src/slave/containerizer/provisioner/docker/spec.cpp (line 75)


I don't think we need to check SHA length, or signature length


- Timothy Chen


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



Review Request 39060: Create master detector per url & not per framework

2015-10-06 Thread Mandeep Chadha

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

Review request for mesos.


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


Repository: mesos


Description
---

If the number of framework created exceeds the lib process
threads then during master failover the zookeeper updates can
cause deadlock.


Diffs
-

  src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 

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


Testing
---

make check successful 
Created 100 framework instances on a 24 CPU machine. Master failover detected 
by the framework process and continue to work as expected.


Thanks,

Mandeep Chadha



Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

2015-10-06 Thread Ben Mahler

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


I would have liked to ship this but it looks like the fsLayers rename related 
changes need adjustement. Can you please pull that out into a separate patch 
please?

Let's have this patch focus only on the renaming of ManifestResponse.


src/slave/containerizer/provisioner/docker/registry_client.hpp (line 67)


Please let's avoid conflating independent changes in these patches, as it 
makes it much easier to go through review when we're doing one thing at a time.

We should pull out naming cleanup into a seperate patch, it looks like 
there is a lot of naming cleanup we need to do on these files outside of just 
fsLayers. Also, how about s/fsLayers/layers/?



src/tests/containerizer/provisioner_docker_tests.cpp (line 67)


Can you do a sweep to remove all of the namespace aliases? If you want to 
pull them out of the RegistryClient let's use another patch.



src/tests/containerizer/provisioner_docker_tests.cpp (line 429)


In general please try to use succict, non-redundant names:

s/manifestResponseFuture/manifest

It's clear here that this is a future from the type and now that we've 
removed response from the type we should remove it from the name as well.


- Ben Mahler


On Oct. 5, 2015, 8:57 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39014/
> ---
> 
> (Updated Oct. 5, 2015, 8:57 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RegistryClient refactor: renamed ManifestResponse as per review comments.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> c2040b48ea43fdb29766994c244273d3fa9ee3cd 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/39014/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Mesos ReviewBot

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Oct. 6, 2015, 5:59 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 5:59 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 38779: Use new HTTP status code check in scheduler.

2015-10-06 Thread Timothy Chen

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

(Updated Oct. 6, 2015, 8:27 p.m.)


Review request for mesos, Anand Mazumdar and Ben Mahler.


Repository: mesos


Description
---

Use new HTTP status code check in scheduler.


Diffs (updated)
-

  src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 

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


Testing
---

make check


Thanks,

Timothy Chen



Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-10-06 Thread Timothy Chen

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

(Updated Oct. 6, 2015, 8:27 p.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
and Jiang Yan Xu.


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


Repository: mesos


Description
---

Allow HTTP response codes to be checked with code.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
591c1a959057155e1bf0f5bd73352e78d1c15cb3 
  3rdparty/libprocess/src/decoder.hpp 53b8079968a0145651bad9d11aa4be2b504de57a 
  3rdparty/libprocess/src/http.cpp ebf76093d654397260fc8da84c4f2b0fd6541483 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
  3rdparty/libprocess/src/tests/http_tests.cpp 
38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
  3rdparty/libprocess/src/tests/process_tests.cpp 
ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 

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


Testing
---

make


Thanks,

Timothy Chen



Re: Review Request 38779: Use new HTTP status code check in scheduler.

2015-10-06 Thread Gilbert Song

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

Ship it!


Ship It!

- Gilbert Song


On Oct. 6, 2015, 1:27 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38779/
> ---
> 
> (Updated Oct. 6, 2015, 1:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use new HTTP status code check in scheduler.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 
> 
> Diff: https://reviews.apache.org/r/38779/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 38570: Change documentation image links to absolute paths.

2015-10-06 Thread Ben Mahler

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



docs/external-containerizer.md (line 92)


How will this work when we add multiple versions of documents to the 
website? Now this hardcodes "latest" as the version?


- Ben Mahler


On Oct. 6, 2015, 4:45 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38570/
> ---
> 
> (Updated Oct. 6, 2015, 4:45 p.m.)
> 
> 
> Review request for mesos, Adam B, Dave Lester, Artem Harutyunyan, Kapil Arya, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3183
> https://issues.apache.org/jira/browse/MESOS-3183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also removes extraneous `?raw=true` from the links.
> 
> 
> Diffs
> -
> 
>   docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
>   docs/fetcher-cache-internals.md e8a68d1230420d1afca61f92c1ab6be12a70dbf2 
>   docs/maintenance.md a5831ffa092a9ea6decbe2e640bae637c759a308 
>   docs/networking-for-mesos-managed-containers.md 
> 33568a8233523ff3805f962371c94346e608208b 
>   docs/oversubscription.md 5a31b1ff7f003307817732a71f3b0a7c7d60cd24 
> 
> Diff: https://reviews.apache.org/r/38570/diff/
> 
> 
> Testing
> ---
> 
> Patched `Rakefile` to include images in the website (See JIRA for the patch).
> Then rendered with: https://github.com/mesosphere/mesos-website-container
> 
> Confirmed that images show up on the modified docs.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Mandeep Chadha

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

(Updated Oct. 6, 2015, 9:33 p.m.)


Review request for mesos and Neil Conway.


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


Repository: mesos


Description
---

Check failed due to double comparison : MESOS-3552.


Diffs (updated)
-

  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
  src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 

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


Testing
---

Added unit test.
make check successful.


Thanks,

Mandeep Chadha



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Neil Conway

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

Ship it!


Ship It!


src/tests/reservation_tests.cpp (line 173)


"a Reserved"


- Neil Conway


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Jie Yu

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



src/common/resources.cpp (line 876)


Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to

https://code.google.com/p/googletest/wiki/AdvancedGuide#Floating-Point_Macros

To choose a default epsilon, you probably want to check the above link. 
Gtest uses "Units in the Last Place (ULPs)" as the default.



src/v1/resources.cpp (lines 876 - 882)


Not yours, but too bad we need to duplicate the logic here. I am now sure 
what will be the long term plan here. If we have 10 versions, do we need to 
copy the code 10 times... That's not good :(


- Jie Yu


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 38919: Validation of Docker Image Manifests

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38901, 38919]

All tests passed.

- Mesos ReviewBot


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



Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-10-06 Thread Ben Mahler

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



3rdparty/libprocess/include/process/http.hpp (lines 146 - 188)


What is the value of this being an enum? We can't store the enum anywhere 
so every time we want to use these we have to do explicit casting, like you 
have in Response below.

How about making this a class called 'Status' which holds each of these as 
a static member variable?

That way we can still do:

```
Status::OK
```

And no explicit casting is necessary.



3rdparty/libprocess/include/process/http.hpp (lines 414 - 417)


Rather than adding this to try to cope with the casting from the enum, 
let's avoid the enum per my suggestion above and remove this helper.



3rdparty/libprocess/src/decoder.hpp (line 439)


newline after this?



3rdparty/libprocess/src/decoder.hpp (line 441)


Can you put this above the if, or just remove it?



3rdparty/libprocess/src/http.cpp (line 76)


We avoid static non-POD types due to destruction issues. Can you put this 
on the heap?

Per my suggestion above, perhaps we can have `Status::string(uint16_t 
code)` as a static function on the `Status` class that provides the string 
version of the status.

This should clean up the tests as well.


- Ben Mahler


On Oct. 6, 2015, 8:27 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38416/
> ---
> 
> (Updated Oct. 6, 2015, 8:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Jie Yu, Jojy Varghese, 
> and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3429
> https://issues.apache.org/jira/browse/MESOS-3429
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow HTTP response codes to be checked with code.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 591c1a959057155e1bf0f5bd73352e78d1c15cb3 
>   3rdparty/libprocess/src/decoder.hpp 
> 53b8079968a0145651bad9d11aa4be2b504de57a 
>   3rdparty/libprocess/src/http.cpp ebf76093d654397260fc8da84c4f2b0fd6541483 
>   3rdparty/libprocess/src/process.cpp 
> d1c81f1d244f02bf42cab97198587ce1b8c7c407 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> bb9ced8933bf2bb97ae6b3cffdb5528676e53c11 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> ffd260a3fa2e49b3de183ba7b392b71afaaba2e5 
> 
> Diff: https://reviews.apache.org/r/38416/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 38814: add test cases for sha512 digest verifier

2015-10-06 Thread Gilbert Song

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

(Updated Oct. 6, 2015, 2:56 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


Repository: mesos


Description
---

add test cases for sha512 digest verifier


Diffs (updated)
-

  3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 

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


Testing
---

make check (ubuntu 14.04)


Thanks,

Gilbert Song



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Mandeep Chadha


> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote:
> > src/common/resources.cpp, line 880
> > 
> >
> > Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to
> > 
> > https://code.google.com/p/googletest/wiki/AdvancedGuide#Floating-Point_Macros
> > 
> > To choose a default epsilon, you probably want to check the above link. 
> > Gtest uses "Units in the Last Place (ULPs)" as the default.

CHECK_DOUBLE_EQ will also fail. 

F0930 18:15:38.169140 26984 resources.cpp:874] Check failed: 
(result.cpus().get()) >= (cpus().get())-0.001L (24 vs. 24) *** 
Check failure stack trace: *** F0930 18:15:38.169322 26991 resources.cpp:874] 
Check failed: (result.cpus().get()) >= (cpus().get())-0.001L (24 
vs. 24) *** Check failure stack trace: ***

CHECK_NEAR() is the right Macro to use. 


But we also need to check (isNone() and isNone()) equality and hence the 
present implementation. I think the long term fix is to wrap double into Double 
and have the opeartor== to the right thing.


> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote:
> > src/v1/resources.cpp, lines 880-886
> > 
> >
> > Not yours, but too bad we need to duplicate the logic here. I am now 
> > sure what will be the long term plan here. If we have 10 versions, do we 
> > need to copy the code 10 times... That's not good :(

I agree !! I already missed src/v1/resources.cpp in my first review request :) 
Thanks Neil.


- Mandeep


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


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Mandeep Chadha


> On Oct. 6, 2015, 6:34 p.m., Neil Conway wrote:
> > src/common/resources.cpp, line 874
> > 
> >
> > This change should also be applied to Resources::apply() in 
> > src/v1/resources.cpp

Thanks Neil.


- Mandeep


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


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Anand Mazumdar


> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote:
> > src/v1/resources.cpp, lines 880-886
> > 
> >
> > Not yours, but too bad we need to duplicate the logic here. I am now 
> > sure what will be the long term plan here. If we have 10 versions, do we 
> > need to copy the code 10 times... That's not good :(
> 
> Mandeep Chadha wrote:
> I agree !! I already missed src/v1/resources.cpp in my first review 
> request :) 
> Thanks Neil.

Jie, this is a tech-debt that should not hopefully exist across 10 version 
iterations :)

On a more serious note, this is the related JIRA :  
https://issues.apache.org/jira/browse/MESOS-3404 
We intend to work on it as part of the HTTP API epic.


- Anand


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


On Oct. 6, 2015, 9:33 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 9:33 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Mandeep Chadha

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

(Updated Oct. 6, 2015, 10:07 p.m.)


Review request for mesos and Neil Conway.


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


Repository: mesos


Description
---

Check failed due to double comparison : MESOS-3552.


Diffs (updated)
-

  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
  src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 

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


Testing
---

Added unit test.
make check successful.


Thanks,

Mandeep Chadha



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Jie Yu


> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote:
> > src/common/resources.cpp, line 880
> > 
> >
> > Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to
> > 
> > https://code.google.com/p/googletest/wiki/AdvancedGuide#Floating-Point_Macros
> > 
> > To choose a default epsilon, you probably want to check the above link. 
> > Gtest uses "Units in the Last Place (ULPs)" as the default.
> 
> Mandeep Chadha wrote:
> CHECK_DOUBLE_EQ will also fail. 
> 
> F0930 18:15:38.169140 26984 resources.cpp:874] Check failed: 
> (result.cpus().get()) >= (cpus().get())-0.001L (24 vs. 24) *** 
> Check failure stack trace: *** F0930 18:15:38.169322 26991 resources.cpp:874] 
> Check failed: (result.cpus().get()) >= (cpus().get())-0.001L (24 
> vs. 24) *** Check failure stack trace: ***
> 
> CHECK_NEAR() is the right Macro to use. 
> 
> 
> But we also need to check (isNone() and isNone()) equality and hence the 
> present implementation. I think the long term fix is to wrap double into 
> Double and have the opeartor== to the right thing.

How do you implemente CHECK_DOUBLE_EQ? You copied from gtest code?


- Jie


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


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Mandeep Chadha


> On Oct. 6, 2015, 9:47 p.m., Jie Yu wrote:
> > src/common/resources.cpp, line 880
> > 
> >
> > Can you introduce a `CHECK_DOUBLE_EQ` in `stout/check.hpp`, similar to
> > 
> > https://code.google.com/p/googletest/wiki/AdvancedGuide#Floating-Point_Macros
> > 
> > To choose a default epsilon, you probably want to check the above link. 
> > Gtest uses "Units in the Last Place (ULPs)" as the default.
> 
> Mandeep Chadha wrote:
> CHECK_DOUBLE_EQ will also fail. 
> 
> F0930 18:15:38.169140 26984 resources.cpp:874] Check failed: 
> (result.cpus().get()) >= (cpus().get())-0.001L (24 vs. 24) *** 
> Check failure stack trace: *** F0930 18:15:38.169322 26991 resources.cpp:874] 
> Check failed: (result.cpus().get()) >= (cpus().get())-0.001L (24 
> vs. 24) *** Check failure stack trace: ***
> 
> CHECK_NEAR() is the right Macro to use. 
> 
> 
> But we also need to check (isNone() and isNone()) equality and hence the 
> present implementation. I think the long term fix is to wrap double into 
> Double and have the opeartor== to the right thing.
> 
> Jie Yu wrote:
> How do you implemente CHECK_DOUBLE_EQ? You copied from gtest code?

you can just call it:

CHECK_DOUBLE_EQ(result.cpus.get(), cpus.get());

src/common/resources.cpp ( includes the glog/logging.h and all the Macros are 
available).

#include 


- Mandeep


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


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 37996: Added property manager

2015-10-06 Thread Ben Mahler


> On Oct. 6, 2015, 12:39 p.m., Bernd Mathiske wrote:
> > Ship It!

I don't think we should introduce this into stout in its current form. I 
realize you're planning to use this for authentication stuff, but looking at 
this on its own, it seems like a confusing abstraction. Why would we couple the 
notion of a Tree with the semantics around properties and property inheritance?


- Ben


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


On Oct. 6, 2015, 8:24 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 6, 2015, 8:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 39060: Create master detector per url & not per framework

2015-10-06 Thread Mesos ReviewBot

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


Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Oct. 6, 2015, 7:54 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39060/
> ---
> 
> (Updated Oct. 6, 2015, 7:54 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3595
> https://issues.apache.org/jira/browse/MESOS-3595
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the number of framework created exceeds the lib process
> threads then during master failover the zookeeper updates can
> cause deadlock.
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
> 
> Diff: https://reviews.apache.org/r/39060/diff/
> 
> 
> Testing
> ---
> 
> make check successful 
> Created 100 framework instances on a 24 CPU machine. Master failover detected 
> by the framework process and continue to work as expected.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 38919: Validation of Docker Image Manifests

2015-10-06 Thread Gilbert Song

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

(Updated Oct. 6, 2015, 3:35 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


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


Repository: mesos


Description
---

Validation of Docker Image Manifests


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check (Ubuntu14.04 + clang++-3.6)


Thanks,

Gilbert Song



Re: Review Request 39018: Added JSON parsing for Resources.

2015-10-06 Thread Greg Mann

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

(Updated Oct. 6, 2015, 11:02 p.m.)


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


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

This includes code changes necessary for JSON parsing of Resources. 
Documentation changes will be posted soon in another review.


Diffs (updated)
-

  include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
  include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
  src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
  src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
  src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 

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


Testing
---

`make check`


Thanks,

Greg Mann



Re: Review Request 39018: Added JSON parsing for Resources.

2015-10-06 Thread Greg Mann


> On Oct. 6, 2015, 7:09 a.m., haosdent huang wrote:
> > src/common/resources.cpp, line 362
> > 
> >
> > Is it would more clear to split three functions, one is for parse old 
> > form, one is for parse json object from and third one is for parse json 
> > array form.

Good idea, thanks haosdent!


- Greg


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


On Oct. 6, 2015, 11:02 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 6, 2015, 11:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 38779: Use new HTTP status code check in scheduler.

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38416, 38779]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 8:27 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38779/
> ---
> 
> (Updated Oct. 6, 2015, 8:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use new HTTP status code check in scheduler.
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 56801ca6ffc9f9f0e4bd12dbf535e9c5251c2712 
> 
> Diff: https://reviews.apache.org/r/38779/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 38570: Change documentation image links to absolute paths.

2015-10-06 Thread Joseph Wu


> On Oct. 6, 2015, 2:29 p.m., Ben Mahler wrote:
> > docs/external-containerizer.md, line 92
> > 
> >
> > How will this work when we add multiple versions of documents to the 
> > website? Now this hardcodes "latest" as the version?

I'm not sure.

>From what I've seen of the website generation code, we currently wipe out 
>older versions of the docs during generation.  We'd first have to change the 
>site's Rakefile (and probably the release process) to keep older versions.

After that, we'd still have to deal with the problem of relative URLs and 
trailing slashes:
http://stackoverflow.com/questions/5457885/relative-urls-and-trailing-slashes

Perhaps we could change the markdown parser to replace image links with a 
versioned absolute path.  (I'm not sure what that would entail.)

OR

We could change all the image links to relative links (i.e. 
`images/ec_launch_seqdiag.png`) and remove trailing slashes from all 
documentation links.  (Maybe add redirects from trailing slash pages to 
non-trailing slash pages.)


What do you think?


- Joseph


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


On Oct. 6, 2015, 9:45 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38570/
> ---
> 
> (Updated Oct. 6, 2015, 9:45 a.m.)
> 
> 
> Review request for mesos, Adam B, Dave Lester, Artem Harutyunyan, Kapil Arya, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3183
> https://issues.apache.org/jira/browse/MESOS-3183
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also removes extraneous `?raw=true` from the links.
> 
> 
> Diffs
> -
> 
>   docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
>   docs/fetcher-cache-internals.md e8a68d1230420d1afca61f92c1ab6be12a70dbf2 
>   docs/maintenance.md a5831ffa092a9ea6decbe2e640bae637c759a308 
>   docs/networking-for-mesos-managed-containers.md 
> 33568a8233523ff3805f962371c94346e608208b 
>   docs/oversubscription.md 5a31b1ff7f003307817732a71f3b0a7c7d60cd24 
> 
> Diff: https://reviews.apache.org/r/38570/diff/
> 
> 
> Testing
> ---
> 
> Patched `Rakefile` to include images in the website (See JIRA for the patch).
> Then rendered with: https://github.com/mesosphere/mesos-website-container
> 
> Confirmed that images show up on the modified docs.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

2015-10-06 Thread Jojy Varghese


> On Oct. 6, 2015, 7:57 p.m., Ben Mahler wrote:
> > src/slave/containerizer/provisioner/docker/registry_client.hpp, line 67
> > 
> >
> > Please let's avoid conflating independent changes in these patches, as 
> > it makes it much easier to go through review when we're doing one thing at 
> > a time.
> > 
> > We should pull out naming cleanup into a seperate patch, it looks like 
> > there is a lot of naming cleanup we need to do on these files outside of 
> > just fsLayers. Also, how about s/fsLayers/layers/?

I would like to keep fsLayers as it reflects the language of manifest.


> On Oct. 6, 2015, 7:57 p.m., Ben Mahler wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 67
> > 
> >
> > Can you do a sweep to remove all of the namespace aliases? If you want 
> > to pull them out of the RegistryClient let's use another patch.

Its already removed in a later patch in the series 
(https://reviews.apache.org/r/38941)


> On Oct. 6, 2015, 7:57 p.m., Ben Mahler wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 429
> > 
> >
> > In general please try to use succict, non-redundant names:
> > 
> > s/manifestResponseFuture/manifest
> > 
> > It's clear here that this is a future from the type and now that we've 
> > removed response from the type we should remove it from the name as well.

The idea was to make it obvious 50 lines down from initialization. I can change 
it and create another patch for replacing all xxxFuture with xxx for variable 
names.


- Jojy


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


On Oct. 5, 2015, 8:57 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39014/
> ---
> 
> (Updated Oct. 5, 2015, 8:57 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> RegistryClient refactor: renamed ManifestResponse as per review comments.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/provisioner/docker/registry_client.hpp 
> 9d5d15455192e2d75fe5cd7fa8755fb8cc67e185 
>   src/slave/containerizer/provisioner/docker/registry_client.cpp 
> c2040b48ea43fdb29766994c244273d3fa9ee3cd 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/39014/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>



Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

2015-10-06 Thread Jojy Varghese

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

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


Review request for mesos and Ben Mahler.


Changes
---

separating out other changes.


Repository: mesos


Description
---

RegistryClient refactor: renamed ManifestResponse as per review comments.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Review Request 39068: RegistryClient refactor: Renamed fsLayerInfoList

2015-10-06 Thread Jojy Varghese

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

Review request for mesos.


Repository: mesos


Description
---

RegistryClient refactor: Renamed fsLayerInfoList


Diffs
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38941: Moved structs outside RegistryClient

2015-10-06 Thread Jojy Varghese

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

(Updated Oct. 7, 2015, 12:19 a.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

rebased after reordering patches.


Repository: mesos


Description
---

Moved:
  - ManifestResponse
  - FilesystemLayerInfo


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 37996: Added property manager

2015-10-06 Thread Till Toenshoff


> On Oct. 6, 2015, 12:39 p.m., Bernd Mathiske wrote:
> > Ship It!
> 
> Ben Mahler wrote:
> I don't think we should introduce this into stout in its current form. I 
> realize you're planning to use this for authentication stuff, but looking at 
> this on its own, it seems like a confusing abstraction. Why would we couple 
> the notion of a Tree with the semantics around properties and property 
> inheritance?

This code is being used in libprocess. So the options are libprocess or stout 
for introducing it. I believe it would be a better fit for stout than for 
libprocess as it is a data structure implementation that has no threading or 
process specifics. Given that it already is in a reusable state, I think that 
we should go as proposed by this RR.


- Till


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


On Oct. 6, 2015, 8:24 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 6, 2015, 8:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 38910: Added `-v` flag to `docker rm`.

2015-10-06 Thread Greg Mann


> On Oct. 1, 2015, 4:31 a.m., Jojy Varghese wrote:
> > src/docker/docker.cpp, line 681
> > 
> >
> > wondering this behavior should be defaulted or not. We might be 
> > overloading stop with more than what it should be doing isnt it? Do we 
> > always want to force remove the volumes when docker stops?
> 
> Greg Mann wrote:
> My & Tim's original thought was that because persistent volumes aren't 
> explicitly implemented for the Docker containerizer currently, we could 
> safely make `-v` the default behavior. However, after considering a bit more, 
> I'm thinking that existing user workflows might establish a Docker volume on 
> an agent with the intention of returning to it later with a subsequent task, 
> in which case defaulting to `-v` would break this workflow, so making this an 
> optional argument may be a good idea. I'll have to think about what is the 
> best way to pass this option to the agent, any thoughts on that matter would 
> be appreciated!

After further review, I do think that we should make the default behavior 
include the `-v` flag. By default, Docker will create directories in 
`/var/lib/docker/aufs/`, `/var/lib/docker/vfs`, etc. depending on the 
filesystem used. These directories contain information related to the Docker 
image itself, and they will not be deleted if the `-v` flag isn't used. Thus, 
if an agent runs long enough and runs enough new Docker images, it can fill up 
the disk with these default volumes.

While it's possible that a user might hack together a "persistent volume" using 
Docker volumes as they're currently implemented, we should discourage this in 
favor of utilizing Mesos-monitored persistent volumes. Always using `docker rm 
-v` allows us to properly clean the agent, leaving it in a good state after the 
containerized task has completed.

It's also worth noting that `docker rm -v` will NOT remove volumes where a host 
path has been explicitly mounted into the container (i.e. using `docker run -v 
/host/path:/container/path mycontainer`). So, in fact, some volumes will be 
unaffected by this change. The primary purpose of this patch is to clean the 
agent by removing the default volumes created in `/var/lib/docker/`.


- Greg


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


On Sept. 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated Sept. 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [39056]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 39068: RegistryClient refactor: Renamed fsLayerInfoList

2015-10-06 Thread Jojy Varghese

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

(Updated Oct. 7, 2015, 12:39 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

fixed braced initializer


Repository: mesos


Description (updated)
---

RegistryClient refactor: renamed fsLayerInfoList


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38941: Moved structs outside RegistryClient

2015-10-06 Thread Jojy Varghese

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

(Updated Oct. 7, 2015, 12:40 a.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

rebased after updating patches.


Repository: mesos


Description
---

Moved:
  - ManifestResponse
  - FilesystemLayerInfo


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39015: RegistryClient refactor: expanded abbreviated names.

2015-10-06 Thread Jojy Varghese

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

(Updated Oct. 7, 2015, 12:41 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased after updating patches.


Repository: mesos


Description
---

RegistryClient refactor: expanded abbreviated names.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39017: RegistryClient refactor: encapsulated Manifest

2015-10-06 Thread Jojy Varghese

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

(Updated Oct. 7, 2015, 12:41 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased after updating patches.


Repository: mesos


Description
---

Wrapped Manifest with ManifestResponse. getManifest returns the encapsulated 
ManifestResponse.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 39016: RegistryClient refactor: refactored lambdas

2015-10-06 Thread Jojy Varghese

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

(Updated Oct. 7, 2015, 12:41 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased after updating patches.


Repository: mesos


Description
---

RegistryClient refactor: refactored lambdas as per review comments.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 

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


Testing
---

Make check.


Thanks,

Jojy Varghese



Re: Review Request 39053: RegistryClient refactor: priv method const'ness

2015-10-06 Thread Jojy Varghese

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

(Updated Oct. 7, 2015, 12:42 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased after updating patches.


Repository: mesos


Description
---

RegistryClient refactor: priv method const'ness


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 

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


Testing
---

make check.


Thanks,

Jojy Varghese



Re: Review Request 38873: Added helper functions for evolving old style executor messages to V1 Executor Events

2015-10-06 Thread Ben Mahler

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

Ship it!


Will get this committed shortly.


src/internal/evolve.hpp (lines 100 - 101)


Why did you wrap?

```
>>> len('v1::executor::Event evolve(const 
StatusUpdateAcknowledgementMessage& message);')
78
```



src/internal/evolve.cpp (lines 304 - 305)


Why did you wrap?

```
>>> len('  kill->mutable_task_id()->CopyFrom(evolve(message.task_id()));')
63
```



src/internal/evolve.cpp (lines 317 - 318)


Ditto about wrapping here and elsewhere


- Ben Mahler


On Oct. 5, 2015, 11:09 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38873/
> ---
> 
> (Updated Oct. 5, 2015, 11:09 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3480
> https://issues.apache.org/jira/browse/MESOS-3480
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change implements helper functions for evolving old style Executor 
> messages in `src/messages/messages.proto` to V1 Executor Events defined in 
> `include/mesos/v1/executor/executor.proto`. This is needed for MESOS-3480
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 9babac3ccbfb2bf9a3989a3ae20cf96e5f3a2903 
>   src/internal/evolve.cpp 625706e089984b32d8298a2eacf2f8af2bca931e 
> 
> Diff: https://reviews.apache.org/r/38873/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Klaus Ma

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



src/common/resources.cpp (line 875)


This fix is ok for this ticket; but how to handle other part about cpu()? 
Here's some question from me:
1. if `epsilon` is 0.01 here, does it mean the min cpu is 0.01?
2. is this the only code that need `epsilon`? It seems not

@jieyu/@mcypark, show we start a EPIC to include all precision related 
ticket? so, we can 
1. unify the min cpu/men/disk to user
2. unify the operator/code within allocator
3. unify the precision between backend and UI
4. clarify the requirement to cpu/mem, e.g. whether accept empty cpu/mem

Any comments?


- Klaus Ma


On Oct. 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated Oct. 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

2015-10-06 Thread Benjamin Mahler
This patch still depends on https://reviews.apache.org/r/38579/, are you
planning to remove the dependency? I can't ship these smaller changes since
they depend on the large refactor and your layer id patch:
https://reviews.apache.org/r/38443/

On Tue, Oct 6, 2015 at 5:16 PM, Jojy Varghese  wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39014/
> Review request for mesos and Ben Mahler.
> By Jojy Varghese.
>
> *Updated Oct. 7, 2015, 12:16 a.m.*
> Changes
>
> separating out other changes.
>
> *Repository: * mesos
> Description
>
> RegistryClient refactor: renamed ManifestResponse as per review comments.
>
> Testing
>
> make check.
>
> Diffs (updated)
>
>- src/slave/containerizer/provisioner/docker/registry_client.hpp
>(fdb68b675582f603ffb3e96f31c1c9405e238567)
>- src/slave/containerizer/provisioner/docker/registry_client.cpp
>(4931ae8869a697b1e9d8d4cbc0a871e7cd506285)
>- src/tests/containerizer/provisioner_docker_tests.cpp
>(d895eb9d0723e52cff8b21ef2deeaef1911d019c)
>
> View Diff 
>


Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Guangya Liu

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



src/common/resources.cpp (line 875)


The meos is now using 0.01 as the MIN_CPUS


- Guangya Liu


On 十月 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated 十月 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 39056: Fix for Mesos master crash due to check failure.

2015-10-06 Thread Guangya Liu


> On 十月 7, 2015, 1:03 a.m., Guangya Liu wrote:
> > src/common/resources.cpp, line 879
> > 
> >
> > The meos is now using 0.01 as the MIN_CPUS

As the mesos is using 0.01 as the MIN_CPUS, I think it is OK using 0.01 as the 
epsilon


- Guangya


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


On 十月 6, 2015, 10:07 p.m., Mandeep Chadha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39056/
> ---
> 
> (Updated 十月 6, 2015, 10:07 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check failed due to double comparison : MESOS-3552.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/reservation_tests.cpp 6b7c43c8b5c64618249dbee926383242320c111e 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39056/diff/
> 
> 
> Testing
> ---
> 
> Added unit test.
> make check successful.
> 
> 
> Thanks,
> 
> Mandeep Chadha
> 
>



Re: Review Request 38910: Added `-v` flag to `docker rm`.

2015-10-06 Thread Guangya Liu


> On 十月 1, 2015, 4:31 a.m., Jojy Varghese wrote:
> > src/docker/docker.cpp, line 681
> > 
> >
> > wondering this behavior should be defaulted or not. We might be 
> > overloading stop with more than what it should be doing isnt it? Do we 
> > always want to force remove the volumes when docker stops?
> 
> Greg Mann wrote:
> My & Tim's original thought was that because persistent volumes aren't 
> explicitly implemented for the Docker containerizer currently, we could 
> safely make `-v` the default behavior. However, after considering a bit more, 
> I'm thinking that existing user workflows might establish a Docker volume on 
> an agent with the intention of returning to it later with a subsequent task, 
> in which case defaulting to `-v` would break this workflow, so making this an 
> optional argument may be a good idea. I'll have to think about what is the 
> best way to pass this option to the agent, any thoughts on that matter would 
> be appreciated!
> 
> Greg Mann wrote:
> After further review, I do think that we should make the default behavior 
> include the `-v` flag. By default, Docker will create directories in 
> `/var/lib/docker/aufs/`, `/var/lib/docker/vfs`, etc. depending on the 
> filesystem used. These directories contain information related to the Docker 
> image itself, and they will not be deleted if the `-v` flag isn't used. Thus, 
> if an agent runs long enough and runs enough new Docker images, it can fill 
> up the disk with these default volumes.
> 
> While it's possible that a user might hack together a "persistent volume" 
> using Docker volumes as they're currently implemented, we should discourage 
> this in favor of utilizing Mesos-monitored persistent volumes. Always using 
> `docker rm -v` allows us to properly clean the agent, leaving it in a good 
> state after the containerized task has completed.
> 
> It's also worth noting that `docker rm -v` will NOT remove volumes where 
> a host path has been explicitly mounted into the container (i.e. using 
> `docker run -v /host/path:/container/path mycontainer`). So, in fact, some 
> volumes will be unaffected by this change. The primary purpose of this patch 
> is to clean the agent by removing the default volumes created in 
> `/var/lib/docker/`.

Greg, so if I was using -v /host/path:/container/path when create a container, 
I will always need to remove the files explictly on the docker server when the 
container stopped?


- Guangya


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


On 九月 30, 2015, 11:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38910/
> ---
> 
> (Updated 九月 30, 2015, 11:51 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-2613
> https://issues.apache.org/jira/browse/MESOS-2613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `-v` flag to `docker rm`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
> 
> Diff: https://reviews.apache.org/r/38910/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> DockerContainerizerTest.ROOT_DOCKER_Launch_Executor fails on my Ubuntu 14.04 
> VM, but this is a known issue: MESOS-3123
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



  1   2   >