Review Request 43474: Speed up MasterDetectorExpireSlaveZKSession by advance Clock.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up MasterDetectorExpireSlaveZKSession by advance Clock.


Diffs
-

  src/tests/master_contender_detector_tests.cpp 
6375586c31b1fd406529bf299dad6e321b945de8 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 43472: Speed up ZooKeeperTest.LeaderContender by advance Clock.

[haosdent huang]

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

(Updated Feb. 11, 2016, 8:14 a.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up ZooKeeperTest.LeaderContender by advance Clock.


Diffs
-

  src/tests/zookeeper_tests.cpp 0665d916760da7550dae9a85e797cee337a0490d 

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


Testing (updated)
---

Before

```
[   OK ] ZooKeeperTest.LeaderContender (3385 ms)
```

After

```
[   OK ] ZooKeeperTest.LeaderContender (694 ms)
```


Thanks,

haosdent huang

Re: Review Request 43293: Ignored invalid env vars.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Feb. 7, 2016, 1:40 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43293/
> ---
> 
> (Updated Feb. 7, 2016, 1:40 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4607
> https://issues.apache.org/jira/browse/MESOS-4607
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored invalid env vars when creating docker image.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
> 
> Diff: https://reviews.apache.org/r/43293/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> $ GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="DockerImageTest.ParseInspectonImage" --verbose
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from DockerImageTest
> [ RUN  ] DockerImageTest.ParseInspectonImage
> I0207 10:30:01.894125 2034615040 process.cpp:2489] Spawned process 
> files@192.168.0.100:49551
> I0207 10:30:01.894136 211529728 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.894163968+00:00
> I0207 10:30:01.894317 210456576 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.894332928+00:00
> I0207 10:30:01.898862 2034615040 docker.cpp:397] Skipping duplicate 
> environment variable 'JAVA_VERSION'
> I0207 10:30:01.898892 2034615040 docker.cpp:390] Skipping invalid environment 
> variable 'JAVA_VERSION+8u66' for 'ContainerConfig.Env'
> I0207 10:30:01.899473 20992 process.cpp:2499] Resuming 
> AuthenticationRouter(1)@192.168.0.100:49551 at 2016-02-07 
> 02:30:01.899522048+00:00
> I0207 10:30:01.899983 213139456 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.9+00:00
> I0207 10:30:01.900032 213139456 process.cpp:2604] Cleaning up 
> files@192.168.0.100:49551
> I0207 10:30:01.900244 211529728 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.900256000+00:00
> [   OK ] DockerImageTest.ParseInspectonImage (8 ms)
> [--] 1 test from DockerImageTest (8 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (18 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 43473: Speed up ContenderDetectorShutdownNetwork by advance Clock.

[haosdent huang]

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

(Updated Feb. 11, 2016, 8:13 a.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up ContenderDetectorShutdownNetwork by advance Clock.


Diffs
-

  src/tests/master_contender_detector_tests.cpp 
6375586c31b1fd406529bf299dad6e321b945de8 

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


Testing (updated)
---

Before

```
[   OK ] 
ZooKeeperMasterContenderDetectorTest.ContenderDetectorShutdownNetwork (3390 ms)
```

After

```
[   OK ] 
ZooKeeperMasterContenderDetectorTest.ContenderDetectorShutdownNetwork (546 ms)
```


Thanks,

haosdent huang

Re: Review Request 43469: Added a parameter to specify the maximum number of tokens for tokenize.

[Guangya Liu]

> On 二月 11, 2016, 6:14 a.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp, line 230
> > 
> >
> > Align the indent here would be better.

Done.


- Guangya


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


On 二月 11, 2016, 8:32 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43469/
> ---
> 
> (Updated 二月 11, 2016, 8:32 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3833
> https://issues.apache.org/jira/browse/MESOS-3833
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a parameter to specify the maximum number of tokens for tokenize.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 
> c20e2ffe128059fe63ac2630a2c73e512fc6 
>   3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
> 8e7d3eeb22645c7600df1ee215a9de6a26166096 
> 
> Diff: https://reviews.apache.org/r/43469/diff/
> 
> 
> Testing
> ---
> 
> LiuGuangyas-MacBook-Pro:3rdparty gyliu$ ./stout-tests   
> --gtest_filter="StringsTest.Tokenize*" --verbose
> Note: Google Test filter = StringsTest.Tokenize*
> [==] Running 15 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 15 tests from StringsTest
> [ RUN  ] StringsTest.Tokenize
> [   OK ] StringsTest.Tokenize (0 ms)
> [ RUN  ] StringsTest.TokenizeStringWithDelimsAtStart
> [   OK ] StringsTest.TokenizeStringWithDelimsAtStart (0 ms)
> [ RUN  ] StringsTest.TokenizeStringWithDelimsAtEnd
> [   OK ] StringsTest.TokenizeStringWithDelimsAtEnd (0 ms)
> [ RUN  ] StringsTest.TokenizeStringWithDelimsAtStartAndEnd
> [   OK ] StringsTest.TokenizeStringWithDelimsAtStartAndEnd (0 ms)
> [ RUN  ] StringsTest.TokenizeWithMultipleDelims
> [   OK ] StringsTest.TokenizeWithMultipleDelims (0 ms)
> [ RUN  ] StringsTest.TokenizeEmptyString
> [   OK ] StringsTest.TokenizeEmptyString (0 ms)
> [ RUN  ] StringsTest.TokenizeDelimOnlyString
> [   OK ] StringsTest.TokenizeDelimOnlyString (0 ms)
> [ RUN  ] StringsTest.TokenizeNullByteDelim
> [   OK ] StringsTest.TokenizeNullByteDelim (0 ms)
> [ RUN  ] StringsTest.TokenizeNZero
> [   OK ] StringsTest.TokenizeNZero (0 ms)
> [ RUN  ] StringsTest.TokenizeNDelimOnlyString
> [   OK ] StringsTest.TokenizeNDelimOnlyString (0 ms)
> [ RUN  ] StringsTest.TokenizeN
> [   OK ] StringsTest.TokenizeN (0 ms)
> [ RUN  ] StringsTest.TokenizeNStringWithDelimsAtStart
> [   OK ] StringsTest.TokenizeNStringWithDelimsAtStart (0 ms)
> [ RUN  ] StringsTest.TokenizeNStringWithDelimsAtEnd
> [   OK ] StringsTest.TokenizeNStringWithDelimsAtEnd (0 ms)
> [ RUN  ] StringsTest.TokenizeNStringWithDelimsAtStartAndEnd
> [   OK ] StringsTest.TokenizeNStringWithDelimsAtStartAndEnd (0 ms)
> [ RUN  ] StringsTest.TokenizeNWithMultipleDelims
> [   OK ] StringsTest.TokenizeNWithMultipleDelims (0 ms)
> [--] 15 tests from StringsTest (0 ms total)
> 
> [--] Global test environment tear-down
> [==] 15 tests from 1 test case ran. (0 ms total)
> [  PASSED  ] 15 tests.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 43469: Added a parameter to specify the maximum number of tokens for tokenize.

[Guangya Liu]

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

(Updated 二月 11, 2016, 8:32 a.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Added a parameter to specify the maximum number of tokens for tokenize.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 
c20e2ffe128059fe63ac2630a2c73e512fc6 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
8e7d3eeb22645c7600df1ee215a9de6a26166096 

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


Testing
---

LiuGuangyas-MacBook-Pro:3rdparty gyliu$ ./stout-tests   
--gtest_filter="StringsTest.Tokenize*" --verbose
Note: Google Test filter = StringsTest.Tokenize*
[==] Running 15 tests from 1 test case.
[--] Global test environment set-up.
[--] 15 tests from StringsTest
[ RUN  ] StringsTest.Tokenize
[   OK ] StringsTest.Tokenize (0 ms)
[ RUN  ] StringsTest.TokenizeStringWithDelimsAtStart
[   OK ] StringsTest.TokenizeStringWithDelimsAtStart (0 ms)
[ RUN  ] StringsTest.TokenizeStringWithDelimsAtEnd
[   OK ] StringsTest.TokenizeStringWithDelimsAtEnd (0 ms)
[ RUN  ] StringsTest.TokenizeStringWithDelimsAtStartAndEnd
[   OK ] StringsTest.TokenizeStringWithDelimsAtStartAndEnd (0 ms)
[ RUN  ] StringsTest.TokenizeWithMultipleDelims
[   OK ] StringsTest.TokenizeWithMultipleDelims (0 ms)
[ RUN  ] StringsTest.TokenizeEmptyString
[   OK ] StringsTest.TokenizeEmptyString (0 ms)
[ RUN  ] StringsTest.TokenizeDelimOnlyString
[   OK ] StringsTest.TokenizeDelimOnlyString (0 ms)
[ RUN  ] StringsTest.TokenizeNullByteDelim
[   OK ] StringsTest.TokenizeNullByteDelim (0 ms)
[ RUN  ] StringsTest.TokenizeNZero
[   OK ] StringsTest.TokenizeNZero (0 ms)
[ RUN  ] StringsTest.TokenizeNDelimOnlyString
[   OK ] StringsTest.TokenizeNDelimOnlyString (0 ms)
[ RUN  ] StringsTest.TokenizeN
[   OK ] StringsTest.TokenizeN (0 ms)
[ RUN  ] StringsTest.TokenizeNStringWithDelimsAtStart
[   OK ] StringsTest.TokenizeNStringWithDelimsAtStart (0 ms)
[ RUN  ] StringsTest.TokenizeNStringWithDelimsAtEnd
[   OK ] StringsTest.TokenizeNStringWithDelimsAtEnd (0 ms)
[ RUN  ] StringsTest.TokenizeNStringWithDelimsAtStartAndEnd
[   OK ] StringsTest.TokenizeNStringWithDelimsAtStartAndEnd (0 ms)
[ RUN  ] StringsTest.TokenizeNWithMultipleDelims
[   OK ] StringsTest.TokenizeNWithMultipleDelims (0 ms)
[--] 15 tests from StringsTest (0 ms total)

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


Thanks,

Guangya Liu

Review Request 43477: Speed up GroupTest.* test cases by advance clock.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up GroupTest.* test cases by advance clock.


Diffs
-

  src/tests/group_tests.cpp 6344fade519f47cd4ac3a2dc36c3237d9adf4cf8 

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


Testing
---

Before

```
[   OK ] GroupTest.GroupJoinWithDisconnect (3352 ms)
[   OK ] GroupTest.GroupDataWithDisconnect (3350 ms)
[   OK ] GroupTest.GroupCancelWithDisconnect (2013 ms)
[   OK ] GroupTest.GroupPathWithRestrictivePerms (13368 ms)
[   OK ] GroupTest.RetryableErrors (26720 ms)
```

After 

```
[   OK ] GroupTest.GroupJoinWithDisconnect (405 ms)
[   OK ] GroupTest.GroupDataWithDisconnect (192 ms)
[   OK ] GroupTest.GroupCancelWithDisconnect (250 ms)
[   OK ] GroupTest.GroupPathWithRestrictivePerms (334 ms)
[   OK ] GroupTest.RetryableErrors (341 ms)
```


Thanks,

haosdent huang

Review Request 43475: Speed up MasterDetectorExpireSlaveZKSessionNewMaster by advance Clock.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up MasterDetectorExpireSlaveZKSessionNewMaster by advance Clock.


Diffs
-

  src/tests/master_contender_detector_tests.cpp 
6375586c31b1fd406529bf299dad6e321b945de8 

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


Testing
---

Before
```
[   OK ] 
ZooKeeperMasterContenderDetectorTest.MasterDetectorExpireSlaveZKSessionNewMaster
 (3359 ms)
```

After

```
[   OK ] 
ZooKeeperMasterContenderDetectorTest.MasterDetectorExpireSlaveZKSessionNewMaster
 (613 ms)
```


Thanks,

haosdent huang

Re: Review Request 43474: Speed up MasterDetectorExpireSlaveZKSession by advance Clock.

[haosdent huang]

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

(Updated Feb. 11, 2016, 8:11 a.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up MasterDetectorExpireSlaveZKSession by advance Clock.


Diffs
-

  src/tests/master_contender_detector_tests.cpp 
6375586c31b1fd406529bf299dad6e321b945de8 

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


Testing (updated)
---

Before

```
[   OK ] 
ZooKeeperMasterContenderDetectorTest.MasterDetectorExpireSlaveZKSession (3358 
ms)
```

After 

```
[   OK ] 
ZooKeeperMasterContenderDetectorTest.MasterDetectorExpireSlaveZKSession (448 ms)
```


Thanks,

haosdent huang

Re: Review Request 43434: Constrain one of Option's constructors to 'constructible' types.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Feb. 10, 2016, 11:35 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43434/
> ---
> 
> (Updated Feb. 10, 2016, 11:35 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Michael Park.
> 
> 
> Bugs: MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4633
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
> 6b66bce76838046d7c955188ab9ac05e12756a83 
> 
> Diff: https://reviews.apache.org/r/43434/diff/
> 
> 
> Testing
> ---
> 
> Note: All credit for this patch, including the below example, goes to MPark :)
> 
> Lets us compile something like this:
> ```
> struct Foo {};
> 
> void F(const std::string&) {}
> void F(const Option&) {}
> 
> TEST(OptionTest, ConditionalConstruct)
> {
>   F("hello");
> }
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39968: Enabled http endpoint include nested paths.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43469, 39968]

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

- Mesos ReviewBot


On Feb. 11, 2016, 5:37 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39968/
> ---
> 
> (Updated Feb. 11, 2016, 5:37 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3833
> https://issues.apache.org/jira/browse/MESOS-3833
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled http endpoint include nested paths.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/help.cpp be3f7b74d92bd0626d111fd4ddd4cacd74e36e68 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 410551cd4bda145bc491c46b1c64e6ed14fb4f13 
> 
> Diff: https://reviews.apache.org/r/39968/diff/
> 
> 
> Testing
> ---
> 
> LiuGuangyas-MacBook-Pro:~ gyliu$ curl 
> "http://192.168.0.100:5050/help/master/api/v1/scheduler; 
> ### USAGE ###
> >/master/api/v1/scheduler
> 
> ### TL;DR; ###
> Endpoint for schedulers to make Calls against the master.
> 
> ### DESCRIPTION ###
> Returns 202 Accepted iff the request is accepted.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Klaus Ma]

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




src/docker/docker.cpp (line 529)


Also check whether `network_name()` is empty string `""`.


please add unit test cases for this patches; refer to `bridge` test cases.

- Klaus Ma


On Feb. 10, 2016, 4:34 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 4:34 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

> On Feb. 11, 2016, 9:34 a.m., Klaus Ma wrote:
> > src/docker/docker.cpp, line 529
> > 
> >
> > Also check whether `network_name()` is empty string `""`.
> 
> Ezra Silvera wrote:
> I think we already covered this issue in previous review comments. Is 
> there anything special you think is missing ?
> 
> Ezra Silvera 3 weeks ago (Jan. 20, 2016, 12:47 p.m.)
> 
> Guangya Liu, I went through the testing functions and I'm not sure we 
> actually need to add test for this.  Currently there are 3 possible values 
> that can be passed BRIDGE, HOST, NONE. There isn't any test that check the 
> NONE and HOST values. In fact there is  not even a functionality test for 
> BRIDGE value but simply when creating containers the BRIDGE value is used.
> We added a new value "USER" which is identical in terms of code flow 
> to HOST and NONE so I'm not sure we should add a special test for USER while 
> all other values are not tested.
> Further more in order to create a container with USER we will need to 
> create the actual user-network on the docker engine - this will require us to 
> add the functionality for  "network create" into docker.cpp. This 
> functionality is not needed at all for the mesos and will be used only  for 
> this test.
> The same is true for port mapping - maybe we had to better clarify it 
> on the comments - we didn't change any functionality at all and didn't touch 
> any code related to port mapping or network functionality. We are simply 
> passing a new value for the  --net parameter to the docker engine, nothing 
> else has changed.
> What do you think?

My comment was for teh testing. We will add a test for "" string (we actually 
started with that and switch to the current test due to review comments. :-)


- Ezra


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


On Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

> On Feb. 11, 2016, 9:34 a.m., Klaus Ma wrote:
> > src/docker/docker.cpp, line 529
> > 
> >
> > Also check whether `network_name()` is empty string `""`.
> 
> Ezra Silvera wrote:
> I think we already covered this issue in previous review comments. Is 
> there anything special you think is missing ?
> 
> Ezra Silvera 3 weeks ago (Jan. 20, 2016, 12:47 p.m.)
> 
> Guangya Liu, I went through the testing functions and I'm not sure we 
> actually need to add test for this.  Currently there are 3 possible values 
> that can be passed BRIDGE, HOST, NONE. There isn't any test that check the 
> NONE and HOST values. In fact there is  not even a functionality test for 
> BRIDGE value but simply when creating containers the BRIDGE value is used.
> We added a new value "USER" which is identical in terms of code flow 
> to HOST and NONE so I'm not sure we should add a special test for USER while 
> all other values are not tested.
> Further more in order to create a container with USER we will need to 
> create the actual user-network on the docker engine - this will require us to 
> add the functionality for  "network create" into docker.cpp. This 
> functionality is not needed at all for the mesos and will be used only  for 
> this test.
> The same is true for port mapping - maybe we had to better clarify it 
> on the comments - we didn't change any functionality at all and didn't touch 
> any code related to port mapping or network functionality. We are simply 
> passing a new value for the  --net parameter to the docker engine, nothing 
> else has changed.
> What do you think?
> 
> Ezra Silvera wrote:
> My comment was for teh testing. We will add a test for "" string (we 
> actually started with that and switch to the current test due to review 
> comments. :-)
> 
> Klaus Ma wrote:
> For the test, using user-defined network is fine (similar to test using 
> BRIDGE). For the empty string, current code is only check whether 
> `network_name()` is set; if it's set to `""`, I'm not sure the behaviour but 
> I think we it's better to return error before launching docker command line 
> with empty string.

I agree.


- Ezra


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


On Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Review Request 43472: Speed up ZooKeeperTest.LeaderContender by advance Clock.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up ZooKeeperTest.LeaderContender by advance Clock.


Diffs
-

  src/tests/zookeeper_tests.cpp 0665d916760da7550dae9a85e797cee337a0490d 

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


Testing
---


Thanks,

haosdent huang

Review Request 43471: Add the zookeeper patch for the allow add_auth calls.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Add the zookeeper patch for the allow add_auth calls.


Diffs
-

  3rdparty/zookeeper-06d3f3f.patch 8f8f72cb712c4bca328bf865ab082926106fbd94 
  3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 

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


Testing
---


Thanks,

haosdent huang

Review Request 43473: Speed up ContenderDetectorShutdownNetwork by advance Clock.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up ContenderDetectorShutdownNetwork by advance Clock.


Diffs
-

  src/tests/master_contender_detector_tests.cpp 
6375586c31b1fd406529bf299dad6e321b945de8 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 43159: Removed the duplicate "active" field in json schema of `Framework`.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Feb. 4, 2016, 12:11 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43159/
> ---
> 
> (Updated Feb. 4, 2016, 12:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4582
> https://issues.apache.org/jira/browse/MESOS-4582
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `jsonify` library is a writer-based approach, and does not keep track 
> of the fields that have been written out so far.
> The previous version of `summarize(framework)` and `model(framework)` had a 
> duplicate `"active"` field which was de-duplicated since they simply get 
> inserted to a `std::map`, overriding the previous value.
> 
> In the `jsonify` case, this pattern results in duplicate key in the JSON 
> output. Although the presence of duplicate keys is technically not 
> __invalid__ according to the JSON specification, some JSON libraries disallow 
> them.
> As such, we should generate JSON outputs without duplicate keys.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 3d7a624b78fd85a8d99bce609e37411ed660678c 
> 
> Diff: https://reviews.apache.org/r/43159/diff/
> 
> 
> Testing
> ---
> 
> Verified that `make check` __with__ https://reviews.apache.org/r/43160/ + 
> https://reviews.apache.org/r/43161/ and __without__ this patch breaks.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 43471: Add the zookeeper patch for the allow add_auth calls.

[haosdent huang]

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

(Updated Feb. 11, 2016, 8:15 a.m.)


Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Add the zookeeper patch for the allow add_auth calls.


Diffs
-

  3rdparty/zookeeper-06d3f3f.patch 8f8f72cb712c4bca328bf865ab082926106fbd94 
  3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 

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


Testing (updated)
---

Before

```
[   OK ] ZooKeeperMasterContenderDetectorTest.NonRetryableFrrors (10053 ms)
[   OK ] MasterZooKeeperTest.MasterInfoAddress (11282 ms)
[   OK ] ZooKeeperTest.Auth (6688 ms)
[   OK ] ZooKeeperTest.Create (6690 ms)
```

After

```
[   OK ] ZooKeeperMasterContenderDetectorTest.NonRetryableFrrors (321 ms)
[   OK ] MasterZooKeeperTest.MasterInfoAddress (447 ms)
[   OK ] ZooKeeperTest.Auth (233 ms)
[   OK ] ZooKeeperTest.Create (275 ms)
```


Thanks,

haosdent huang

Re: Review Request 42992: Support sharing of resources through reference counting of resources.

[Guangya Liu]

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




src/common/resources.cpp (lines 202 - 205)


Does this require that the num_consumers should be same?



src/common/resources.cpp (lines 335 - 338)


does this require the num_consumers be same when subtract?



src/common/resources.cpp (line 729)


add a period to the end.



src/common/resources.cpp (line 734)


s/Resource type/Resoure

`Resource type` is usally specifying if the resources is `SCALAR`, `SET` or 
`RANGES`.



src/common/resources.cpp (lines 1653 - 1656)


Can you please add some comments to clarify the difference between `None` 
and `0` for `num_consumers`?


- Guangya Liu


On 二月 5, 2016, 10:57 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42992/
> ---
> 
> (Updated 二月 5, 2016, 10:57 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new Offer::Operation of SHARE and UNSHARE for resources.
> Added ShareInfo within Resources protobuf to allow for sharing of resources
> and keep track of consumers of such resources.
> Shareable resources keeps track of its consumers and makes decision based on
> that during task launch and task termination.
> Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af 
>   src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
>   src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2 
>   src/master/allocator/mesos/hierarchical.cpp 
> a9d2c23162892e0f97d89a076d2311091d91 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 18797e42a9c2bd20392020237cfae600a5ffe12c 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca 
>   src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7 
> 
> Diff: https://reviews.apache.org/r/42992/diff/
> 
> 
> Testing
> ---
> 
> make check done.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Review Request 43517: Speed up MasterTest.OfferTimeout.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up MasterTest.OfferTimeout.


Diffs
-

  src/tests/master_tests.cpp 393a6f5fe3744d6ba743f362b7e309d1ee75a303 

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


Testing
---


Thanks,

haosdent huang

Review Request 43514: Speed up MasterTest.RecoverResources.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up MasterTest.RecoverResources.


Diffs
-

  src/tests/master_tests.cpp 393a6f5fe3744d6ba743f362b7e309d1ee75a303 

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


Testing
---


Thanks,

haosdent huang

Review Request 43515: Speed up MasterTest.MasterInfoOnReElection.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up MasterTest.MasterInfoOnReElection.


Diffs
-

  src/tests/master_tests.cpp 393a6f5fe3744d6ba743f362b7e309d1ee75a303 

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


Testing
---


Thanks,

haosdent huang

Review Request 43516: Speed up MasterTest.LaunchCombinedOfferTest.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up MasterTest.LaunchCombinedOfferTest.


Diffs
-

  src/tests/master_tests.cpp 393a6f5fe3744d6ba743f362b7e309d1ee75a303 

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


Testing
---


Thanks,

haosdent huang

Review Request 43518: Speed up MasterMaintenanceTest.EnterMaintenanceMode.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up MasterMaintenanceTest.EnterMaintenanceMode.


Diffs
-

  src/tests/master_maintenance_tests.cpp 
b98eedec388813ee795dd83ccc5ff27338209475 

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


Testing
---


Thanks,

haosdent huang

Review Request 43519: Speed up MasterMaintenanceTest.InverseOffers.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up MasterMaintenanceTest.InverseOffers.


Diffs
-

  src/tests/master_maintenance_tests.cpp 
b98eedec388813ee795dd83ccc5ff27338209475 

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


Testing
---


Thanks,

haosdent huang

Review Request 43520: Speed up GarbageCollectorIntegrationTest.Restart.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up GarbageCollectorIntegrationTest.Restart.


Diffs
-

  src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 

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


Testing
---


Thanks,

haosdent huang

Review Request 43522: Speed up OversubscriptionTest.RemoveCapabilitiesOnSchedulerFailover.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up OversubscriptionTest.RemoveCapabilitiesOnSchedulerFailover.


Diffs
-

  src/tests/oversubscription_tests.cpp d4ae81972fd218c58a413d1968a4e9acbee52fd3 

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


Testing
---


Thanks,

haosdent huang

Review Request 43521: Speed up OversubscriptionTest.UpdateAllocatorOnSchedulerFailover.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Speed up OversubscriptionTest.UpdateAllocatorOnSchedulerFailover.


Diffs
-

  src/tests/oversubscription_tests.cpp d4ae81972fd218c58a413d1968a4e9acbee52fd3 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 43517: Speed up MasterTest.OfferTimeout.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43514, 43515, 43516, 43517]

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

- Mesos ReviewBot


On Feb. 12, 2016, 6:36 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43517/
> ---
> 
> (Updated Feb. 12, 2016, 6:36 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4167
> https://issues.apache.org/jira/browse/MESOS-4167
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Speed up MasterTest.OfferTimeout.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 393a6f5fe3744d6ba743f362b7e309d1ee75a303 
> 
> Diff: https://reviews.apache.org/r/43517/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 43438: Update containerizer tests to pass shared_ptrs to 'StartSlave'.

[Joseph Wu]

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

(Updated Feb. 11, 2016, 11:33 a.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Move a FUTURE_DISPATCH on the MesosContainerizer after the destruction of a 
prior containerizer.


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


Repository: mesos


Description
---

Makes the following changes:

* All objects passed to `StartSlave` are transformed into `shared_ptr`.
* References to the above objects are updated to reflect the different type.
* Removes `Shutdown()` in at the end of all tests, except for master tests that 
have a stack-allocated dependency.
* Removes manual `delete`s inside tests.
* Generally renames test variables to be more consistent (i.e. `exec` -> 
`executor` and `dockerContainerizer` -> `containerizer`).
* Removes extraneous spaces in `Try` in touched lines.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 
  src/tests/containerizer/external_containerizer_test.cpp 
8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
d8c3c81c3d5a4c4093b3d4b27bb5c123c77accfc 
  src/tests/containerizer/isolator_tests.cpp 
653b037c489072f43e53dec01a811a9249dcd660 
  src/tests/containerizer/memory_pressure_tests.cpp 
4a03af2c9c0643d964b1d76e2096341b59bf5dce 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
  src/tests/containerizer/port_mapping_tests.cpp 
aa9846097feda1a82131b67aa4c782ec3625d049 

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


Testing
---

Tests are run at the very, very end of this review chain.


Thanks,

Joseph Wu

Re: Review Request 43437: Update slave tests to pass shared_ptrs to 'StartSlave'.

[Joseph Wu]

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

(Updated Feb. 11, 2016, 11:33 a.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Move some FUTURE_DISPATCH's on the MesosContainerizer after the destruction of 
a prior containerizer.


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


Repository: mesos


Description
---

Makes the following changes:

* All objects passed to `StartSlave` are transformed into `shared_ptr`.
* References to the above objects are updated to reflect the different type.
* Removes `Shutdown()` in at the end of all tests, except for master tests that 
have a stack-allocated dependency.
* Removes manual `delete`s inside tests.
* Generally renames test variables to be more consistent (i.e. `exec` -> 
`executor` and `dockerContainerizer` -> `containerizer`).
* Removes extraneous spaces in `Try` in touched lines.


Diffs (updated)
-

  src/tests/slave_recovery_tests.cpp e2a78a0f55b7657057ee351a747caff51024fd67 
  src/tests/slave_tests.cpp c7f5a701eff2c2f9aa3df5722583a131bf2c072a 

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


Testing
---

Tests are run at the very, very end of this review chain.


Thanks,

Joseph Wu

Re: Review Request 43493: Corrected links to slides.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43493]

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

- Mesos ReviewBot


On Feb. 11, 2016, 7:25 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43493/
> ---
> 
> (Updated Feb. 11, 2016, 7:25 p.m.)
> 
> 
> Review request for mesos, Guangya Liu and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/presentations.md 2a581b2acdbbaa526764bfb685f41d651485876e 
> 
> Diff: https://reviews.apache.org/r/43493/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 43336: Introduced Appc image fetcher.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [41958, 41959, 42156, 42157, 43127, 43198]

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

Error:
2016-02-11 19:06:40 URL:https://reviews.apache.org/r/43198/diff/raw/ 
[4884/4884] -> "43198.patch" [1]
src/appc/spec.cpp:146:  Redundant blank line at the end of a code block should 
be deleted.  [whitespace/blank_line] [3]
Total errors found: 1
Checking 2 files

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

- Mesos ReviewBot


On Feb. 11, 2016, 5:52 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43336/
> ---
> 
> (Updated Feb. 11, 2016, 5:52 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4596
> https://issues.apache.org/jira/browse/MESOS-4596
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation for simple image discovery for Appc images.
> 
> TODO: Add tests.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 4a2954498efa48a4eb43f82827ff1d6f5f65d389 
>   src/Makefile.am 22f51319a1c06da02cac5822d42315e3027cf500 
>   src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp PRE-CREATION 
>   src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
>   src/slave/flags.cpp 24a23325cc255d0d7b7af7ed096b6d3012ad75c7 
> 
> Diff: https://reviews.apache.org/r/43336/diff/
> 
> 
> Testing
> ---
> 
> Tested with local http(s) server.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 43493: Corrected links to slides.

[Alexander Rukletsov]

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

Review request for mesos, Guangya Liu and Michael Park.


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/presentations.md 2a581b2acdbbaa526764bfb685f41d651485876e 

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


Testing
---

None.


Thanks,

Alexander Rukletsov

Re: Review Request 43483: Added logs for better debugging of allocation of `net_cls` handles.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 415)


No need for the quote.

```
Updating container status with net_cls.classid " << 
hexify(info.handle->get()).
```

Also, I think we should add a streaming function for NetClsHandle so that 
we can do:
```
<< info.handle.get()
```
here.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 473)


s/Allocating/Allocated/



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 474)


Remove ':'.


- Jie Yu


On Feb. 11, 2016, 3:35 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43483/
> ---
> 
> (Updated Feb. 11, 2016, 3:35 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4657
> https://issues.apache.org/jira/browse/MESOS-4657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added logs for better debugging of allocation of `net_cls` handles.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> 2fc79973524d3776d39d378f22ac694030347c73 
> 
> Diff: https://reviews.apache.org/r/43483/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43466: Added const qualifier for uri::Fetcher::fetch.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 11, 2016, 1:16 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43466/
> ---
> 
> (Updated Feb. 11, 2016, 1:16 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added const qualifier for uri::Fetcher::fetch.
> 
> 
> Diffs
> -
> 
>   include/mesos/uri/fetcher.hpp baa32d3a1157308af33ea06a7a65e2751d970115 
>   src/uri/fetcher.cpp dfda732348fec3b686cf82b55ad94fda4829469b 
> 
> Diff: https://reviews.apache.org/r/43466/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 43499: Improved docs for writing HA frameworks.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Improved docs for writing HA frameworks.


Diffs
-

  docs/high-availability-framework-guide.md 
faae27d2c11d5fa8f3c55d3a6ba5fa751cea057a 
  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 

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


Testing
---

Previewed in site-docker.


Thanks,

Neil Conway

Re: Review Request 43444: Update disk tests to pass shared_ptrs to 'StartSlave'.

[Joseph Wu]

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

(Updated Feb. 11, 2016, 11:34 a.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Remove ownership of `containerizer` from FetcherCache tests.


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


Repository: mesos


Description
---

Makes the following changes:

* All objects passed to `StartSlave` are transformed into `shared_ptr`.
* References to the above objects are updated to reflect the different type.
* Removes `Shutdown()` in at the end of all tests, except for master tests that 
have a stack-allocated dependency.
* Removes manual `delete`s inside tests.
* Generally renames test variables to be more consistent (i.e. `exec` -> 
`executor` and `dockerContainerizer` -> `containerizer`).
* Removes extraneous spaces in `Try` in touched lines.


Diffs (updated)
-

  src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
  src/tests/fetcher_cache_tests.cpp 1cf45660691860793ac600363f7934e13a2e7ddf 
  src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
  src/tests/persistent_volume_tests.cpp 
e169e1b141a38dc389eefd42c11a078c413123d5 

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


Testing
---

Tests are run at the very, very end of this review chain.


Thanks,

Joseph Wu

Re: Review Request 43411: Windows: Added dynamic library loading tests to build.

[M Lawindi]

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


Ship it!




Ship It!

- M Lawindi


On Feb. 10, 2016, 6:47 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> ---
> 
> (Updated Feb. 10, 2016, 6:47 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dynamic library loading tests to build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 
> 27626ae28db090f1a002239ff5c674b82e8fc9a8 
> 
> Diff: https://reviews.apache.org/r/43411/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43413: CMake:[1/2] Allow downloading third-party dependencies from mirror.

[M Lawindi]

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


Fix it, then Ship it!




Ship It!


CMakeLists.txt (line 48)


Make sure you trim this to 80 columns or less


- M Lawindi


On Feb. 10, 2016, 7:03 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43413/
> ---
> 
> (Updated Feb. 10, 2016, 7:03 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[1/2] Allow downloading third-party dependencies from mirror.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 3253da73aa517a335be94148d567510147dae08d 
>   CMakeLists.txt 7f83dc84997d3b824d1f63012894bd9fc5284053 
> 
> Diff: https://reviews.apache.org/r/43413/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

> On Feb. 11, 2016, 9:34 a.m., Klaus Ma wrote:
> > src/docker/docker.cpp, line 529
> > 
> >
> > Also check whether `network_name()` is empty string `""`.

I think we already covered this issue in previous review comments. Is there 
anything special you think is missing ?

Ezra Silvera 3 weeks ago (Jan. 20, 2016, 12:47 p.m.)

Guangya Liu, I went through the testing functions and I'm not sure we 
actually need to add test for this.  Currently there are 3 possible values that 
can be passed BRIDGE, HOST, NONE. There isn't any test that check the NONE and 
HOST values. In fact there is  not even a functionality test for BRIDGE value 
but simply when creating containers the BRIDGE value is used.
We added a new value "USER" which is identical in terms of code flow to 
HOST and NONE so I'm not sure we should add a special test for USER while all 
other values are not tested.
Further more in order to create a container with USER we will need to 
create the actual user-network on the docker engine - this will require us to 
add the functionality for  "network create" into docker.cpp. This functionality 
is not needed at all for the mesos and will be used only  for this test.
The same is true for port mapping - maybe we had to better clarify it on 
the comments - we didn't change any functionality at all and didn't touch any 
code related to port mapping or network functionality. We are simply passing a 
new value for the  --net parameter to the docker engine, nothing else has 
changed.
What do you think?


- Ezra


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


On Feb. 10, 2016, 8:34 a.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 8:34 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Review Request 43480: [WIP]Use in_memory as default registry when testing.

[haosdent huang]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Use in_memory as default registry when testing.


Diffs
-

  src/tests/master_tests.cpp 393a6f5fe3744d6ba743f362b7e309d1ee75a303 
  src/tests/mesos.hpp c0997dbacf8753149caba0a8b2406afc35a4845f 
  src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 43471: Add the zookeeper patch for the allow add_auth calls.

[haosdent huang]

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

(Updated Feb. 11, 2016, 11:10 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rebase


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


Repository: mesos


Description
---

Add the zookeeper patch for the allow add_auth calls.


Diffs (updated)
-

  3rdparty/zookeeper-06d3f3f.patch 8f8f72cb712c4bca328bf865ab082926106fbd94 
  3rdparty/zookeeper-3.4.5.patch 3ca180d0c81f5de521ada7fb6c1c248a871ab2da 

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


Testing
---

Before

```
[   OK ] ZooKeeperMasterContenderDetectorTest.NonRetryableFrrors (10053 ms)
[   OK ] MasterZooKeeperTest.MasterInfoAddress (11282 ms)
[   OK ] ZooKeeperTest.Auth (6688 ms)
[   OK ] ZooKeeperTest.Create (6690 ms)
```

After

```
[   OK ] ZooKeeperMasterContenderDetectorTest.NonRetryableFrrors (321 ms)
[   OK ] MasterZooKeeperTest.MasterInfoAddress (447 ms)
[   OK ] ZooKeeperTest.Auth (233 ms)
[   OK ] ZooKeeperTest.Create (275 ms)
```


Thanks,

haosdent huang

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

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

(Updated Feb. 11, 2016, 1:51 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Loaded the correct diff


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera

Re: Review Request 42516: Add support for user-defined networks.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42516]

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

- Mesos ReviewBot


On Feb. 11, 2016, 1:51 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 11, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 43321: Speeded up SchedulerTest.Decline by advancing the clock.

[Guangya Liu]

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




src/tests/scheduler_tests.cpp (line 836)


I think here is not `finish processing the decline call` but `Make sure the 
recoverResources event has been queued.` At this time, the `decline` call is 
not finished yet as the `recoverResources` was just queued.


- Guangya Liu


On 二月 8, 2016, 4:20 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43321/
> ---
> 
> (Updated 二月 8, 2016, 4:20 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4175
> https://issues.apache.org/jira/browse/MESOS-4175
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Speeded up SchedulerTest.Decline by advancing the clock.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 4e2db2ac40c59b9b9a97cd214b3cd1e727a4f0ad 
> 
> Diff: https://reviews.apache.org/r/43321/diff/
> 
> 
> Testing
> ---
> 
> sudo make check -j2 GTEST_FILTER='ContentType/SchedulerTest.Decline/*
> 
> ```sh
> [ RUN  ] ContentType/SchedulerTest.Decline/0
> [   OK ] ContentType/SchedulerTest.Decline/0 (114 ms)
> [ RUN  ] ContentType/SchedulerTest.Decline/1
> [   OK ] ContentType/SchedulerTest.Decline/1 (98 ms)
> ```
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43480: [WIP]Use in_memory as default registry when testing.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [43480]

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

Error:
2016-02-11 13:09:41 URL:https://reviews.apache.org/r/43480/diff/raw/ 
[3045/3045] -> "43480.patch" [1]
Total errors found: 0
Checking 3 files
Error: Commit message summary (the first line) must start with a capital letter.

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

- Mesos ReviewBot


On Feb. 11, 2016, 12:56 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43480/
> ---
> 
> (Updated Feb. 11, 2016, 12:56 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4647
> https://issues.apache.org/jira/browse/MESOS-4647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use in_memory as default registry when testing.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 393a6f5fe3744d6ba743f362b7e309d1ee75a303 
>   src/tests/mesos.hpp c0997dbacf8753149caba0a8b2406afc35a4845f 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43480/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Ezra Silvera]

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

(Updated Feb. 11, 2016, 1:14 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

Change error message + add a test for network_name=""


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


Repository: mesos


Description
---

Signed-off-by: Ezra Silvera 


Diffs (updated)
-

  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
  src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 

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


Testing
---

Using Swarm running on Mesos create a network with "docker  network create 
--driver=bridge myNetwork"   and then create a container on that network:  
"docker run --net=myNetwork"


Thanks,

Ezra Silvera

Re: Review Request 43105: Ensured the allocator does not double account resources.

[Guangya Liu]

> On 二月 4, 2016, 3:46 p.m., Guangya Liu wrote:
> > This case may happen when master is recovering, framework recovery start 
> > before some agent. Can you please add a unit test to cover this code 
> > change? It could be register framework first, then addslave and check the 
> > result.
> 
> Alexander Rukletsov wrote:
> Nope, this can't happen, because in Mesos we reconstruct framework state 
> through reconnecting agents.
> 
> Guangya Liu wrote:
> What about in the case when framework failover? Does there are any 
> potential steps/operations which can cause `addFramework()` be called before 
> `addSlave()`? Thanks.
> 
> Alexander Rukletsov wrote:
> It's tricky. `addFramework()` _can_ be called before some `addSlave()` 
> calls, right, but it's not possible in production code (the master) to report 
> about yet unknown agents in `addFramework()`. If you look at the master code, 
> we reconstruct framework's allocation from agents' allocation and ensure all 
> agents that constitute the "already known" part of the framework allocation 
> are known to the allocator _before_ we call `addFramework()`. However, we can 
> discover additional "pieces" of framework's allocation while more agents 
> rejoin the cluster, but at this point the framework is already known to the 
> allocator. Is my explanation clear enough?

Yes, I see, the master can make sure that the `addFramework()` can always count 
on the agents which was already joined the mesos cluster, the problem is that 
why we need this fix for now? The `agents` in allocator is posted by master, 
does there are any unsync between master and allocator for `agents`?


- Guangya


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


On 二月 2, 2016, 11:13 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43105/
> ---
> 
> (Updated 二月 2, 2016, 11:13 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `addFramework()` is called before `addSlave()` double accounting
> of resources is possible. We have not observed bugs because Mesos
> currently always adds a framework in the allocator after all agents
> with the framework's tasks have been added.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp e163669c9c4e4c98572968f18987704b60722a79 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1a07d69016407e5aad2209586da37fecbcddb765 
> 
> Diff: https://reviews.apache.org/r/43105/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 43321: Speeded up SchedulerTest.Decline by advancing the clock.

[Guangya Liu]

> On 二月 9, 2016, 2:29 p.m., Guangya Liu wrote:
> > src/tests/scheduler_tests.cpp, line 839
> > 
> >
> > Do we need Clock::settle() here to make sure the `recoverResources` 
> > messages to be dispatched and processed completely?
> 
> haosdent huang wrote:
> +1 for add settle
> 
> Shuai Lin wrote:
> Hello haosdent and Guangya,
> 
> I don't think `Clock::settle()` is needed here.
> 
> I guess your rationale is we need to be sure the decline call is 
> processed *before* the next around of allocation is executed, which I totally 
> agree. But we already have it without `clock::settle()`, here is my 
> understanding:
> 
> - We advance the clock after the dispatch event of `recoverResources` is 
> enqueued. And by advancing the clock, we can be sure the 
> `HierarchicalAllocatorProcess::batch()` function, which does the allocation 
> work, being added to the event loop.
> 
> - Since the `recoverResources` is dispatched before 
> `HierarchicalAllocatorProcess::batch()`, it would always be processed first 
> by allocator.
> 
> What do you think?
> 
> Guangya Liu wrote:
> I think we cannot make sure that the 
> `HierarchicalAllocatorProcess::batch()` is always handled before 
> `recoverResources` for some race condition cases, you may see that most of 
> the `advance()` always including `Clock::settle()`
> 
> Shuai Lin wrote:
> Hello Guangya, Could you please elaborate more on the "race condition 
> cases"? IMHO libprocess guarantees for a given actor, first enqueued event is 
> also hanlded first, no?
> 
> Guangya Liu wrote:
> My understanding is that the `decline` will involve two libprocess calls 
> `decline` and `recoverResources`, there might be problem if 
> `HierarchicalAllocatorProcess::batch()` is called after `decline` but before 
> `recoverResources`, comments?
> 
> Shuai Lin wrote:
> Let's inspect the behavior of the allocator actor:
> 
> - When `AWAIT_READY(recoverResources)` returns, we can be sure that a 
> dispatch event of `HierarchicalAllocatorProcess::recoverResource` for the 
> allocator actor is already in the run queue. 
> - After that we advance the clock so that a dispatch event of 
> `HierarchicalAllocatorProcess::batch()` is enqueued immediately (instead of 
> waiting for a duration of `flags.allocation_interval`).
> - Since the `HierarchicalAllocatorProcess::recoverResource` is enquened 
> first, we can be sure it's called before 
> `HierarchicalAllocatorProcess::batch()` is called.

In a multi-core environment, is it possible for the 
`HierarchicalAllocatorProcess::recoverResource` and 
`HierarchicalAllocatorProcess::batch()` be proceed almost same time?


- Guangya


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


On 二月 8, 2016, 4:20 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43321/
> ---
> 
> (Updated 二月 8, 2016, 4:20 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4175
> https://issues.apache.org/jira/browse/MESOS-4175
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Speeded up SchedulerTest.Decline by advancing the clock.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 4e2db2ac40c59b9b9a97cd214b3cd1e727a4f0ad 
> 
> Diff: https://reviews.apache.org/r/43321/diff/
> 
> 
> Testing
> ---
> 
> sudo make check -j2 GTEST_FILTER='ContentType/SchedulerTest.Decline/*
> 
> ```sh
> [ RUN  ] ContentType/SchedulerTest.Decline/0
> [   OK ] ContentType/SchedulerTest.Decline/0 (114 ms)
> [ RUN  ] ContentType/SchedulerTest.Decline/1
> [   OK ] ContentType/SchedulerTest.Decline/1 (98 ms)
> ```
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Review Request 43483: Added logs for better debugging of allocation of `net_cls` handles.

[Avinash sridharan]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added logs for better debugging of allocation of `net_cls` handles.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
2fc79973524d3776d39d378f22ac694030347c73 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 43127: Introduced Appc image cache.

[Jojy Varghese]

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

(Updated Feb. 11, 2016, 4:15 p.m.)


Review request for mesos and Jie Yu.


Bugs: MESOS-4439 and MESOS-4575
https://issues.apache.org/jira/browse/MESOS-4439
https://issues.apache.org/jira/browse/MESOS-4575


Repository: mesos


Description
---

Image cache will cache metadata about all Appc images stores in the store's
image directory. This cache could be useful to:
  - Quickly query if an image is present in the store.
  - By sharing the cache with other components like fetcher, an image can be
  checked if its present before fetching it.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/appc/cache.hpp 
4a63d479d3328605bac08fddffe190dbe21ad2b7 
  src/slave/containerizer/mesos/provisioner/appc/cache.cpp 
af69db3cdfea05c72ecc6ed18adc9ce520ecdd7e 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
1f9b573f9388aafff3304358b8822a48075afb44 
  src/tests/containerizer/provisioner_appc_tests.cpp 
012dba4e24b9a94dc8da0d329baf4bec2d33ffca 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 43480: Use in_memory as default registry when testing.

[haosdent huang]

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

(Updated Feb. 11, 2016, 4:02 p.m.)


Review request for mesos and Ben Mahler.


Summary (updated)
-

Use in_memory as default registry when testing.


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


Repository: mesos


Description
---

Use in_memory as default registry when testing.


Diffs
-

  src/tests/master_tests.cpp 393a6f5fe3744d6ba743f362b7e309d1ee75a303 
  src/tests/mesos.hpp c0997dbacf8753149caba0a8b2406afc35a4845f 
  src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 

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


Testing (updated)
---

sudo test in CentOS 7.1


Thanks,

haosdent huang

Re: Review Request 43417: Windows: Marked functions in headers `inline` to avoid linker errors.

[M Lawindi]

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


Ship it!




Ship It!

- M Lawindi


On Feb. 10, 2016, 7:13 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43417/
> ---
> 
> (Updated Feb. 10, 2016, 7:13 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Marked functions in headers `inline` to avoid linker errors.
> 
> 
> Diffs
> -
> 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
> 046388189823c0c41ce6cc135d5d3838e9131087 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> 9a592c4ec9f45fdd8ae8c724c3cab67876de72f5 
> 
> Diff: https://reviews.apache.org/r/43417/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43336: Introduced Appc image fetcher.

[Jie Yu]

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



First pass. Will do a more detailed path tomorrow. Thanks Jojy!


src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp (lines 30 - 36)


Why do you need this? Can you just include  ?



src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp (lines 55 - 56)


Swap the order of these two parameters. We usually put flags first.

Also, please use `const Shared& fetcher`. (You can share 
'Shared' pointers).



src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp (lines 68 - 70)


Please remove the 'const' in the end. We try to avoid doing that unless 
it's necessary. Especially, in this case, 'fetch' does not absolutely indicate 
that it's const function (depending on the internal impl.)



src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp (lines 73 - 74)


Please fix the indentation for parameters.



src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp (line 74)


NO need for mesos:: namespace prefix since you're in mesos namespace 
already.



src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp (lines 76 - 86)


Ditto on removing const in the end.



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (line 29)


Add a new line above.



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (lines 53 - 54)


Why use leading underscore for parameters here?



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (line 83)


No need to create this temp variable. Compiler will do it anyway. User 
appc.name() in all other places sounds pretty clean to me.



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (line 88)


s/discoveryUrl/url/



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (line 95)


I would just inline this helper here. We try to avoid unnecessary helpers 
because it hurts readability (readers have to jump around in the code).



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (lines 148 - 189)


Hum, I find this unncessary and overly complicated. Can you just assume 
each time we call 'fetch', the 'directory' is an empty temp directory.



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (line 177)


This is not necessary, the caller will cleanup the 'directory'.



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (line 508)


I would split fetch and untar into two steps. At the top level, I would 
like to see something like:

```
return fetch(...)
  .then(lambda::bind(, ...))
  .then(lambda::bind(, ...))
  .onAny(lambda::bind(, ...))
```



src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp (lines 523 - 527)


Hum, can you move this to the lambda in onAny?


- Jie Yu


On Feb. 11, 2016, 5:52 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43336/
> ---
> 
> (Updated Feb. 11, 2016, 5:52 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4596
> https://issues.apache.org/jira/browse/MESOS-4596
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation for simple image discovery for Appc images.
> 
> TODO: Add tests.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 4a2954498efa48a4eb43f82827ff1d6f5f65d389 
>   src/Makefile.am 22f51319a1c06da02cac5822d42315e3027cf500 
>   src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp PRE-CREATION 
>   src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
>   src/slave/flags.cpp 24a23325cc255d0d7b7af7ed096b6d3012ad75c7 
> 
> Diff: https://reviews.apache.org/r/43336/diff/
> 
> 
> Testing
> ---
> 
> Tested with local http(s) server.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 43127: Introduced Appc image cache.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/provisioner/appc/cache.cpp (line 97)


Indentation.



src/slave/containerizer/mesos/provisioner/appc/cache.cpp (line 101)


Do you need this temp variable?



src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 51)


Hum, copying the cache sounds weired to me since we get rid of the 
shared_ptr. Can we use Owned here instead?

That means you need to return Try for Cache::create.



src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 148)


This should be Option?



src/slave/containerizer/mesos/provisioner/appc/store.cpp (lines 156 - 179)


I guess this is fine for now. The logics will change soon as we're adding 
fetching support. I think what we should do here in the future is:
```
1. get imageId as you did
2. if imageId is not found, call fetch using 'appc', move it to store and 
goto 4
3. if imageId is found, check if it's in the storeDir (cache.get(imageId))
4. If yes, fetch its dependencies
5. If not, goto 2
```

The validation will be in step 2 after a layer is downloaded.



src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 168)


No need for this temp var.



src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 177)


No need for this temp var.


- Jie Yu


On Feb. 11, 2016, 4:15 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43127/
> ---
> 
> (Updated Feb. 11, 2016, 4:15 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4439 and MESOS-4575
> https://issues.apache.org/jira/browse/MESOS-4439
> https://issues.apache.org/jira/browse/MESOS-4575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Image cache will cache metadata about all Appc images stores in the store's
> image directory. This cache could be useful to:
>   - Quickly query if an image is present in the store.
>   - By sharing the cache with other components like fetcher, an image can be
>   checked if its present before fetching it.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/cache.hpp 
> 4a63d479d3328605bac08fddffe190dbe21ad2b7 
>   src/slave/containerizer/mesos/provisioner/appc/cache.cpp 
> af69db3cdfea05c72ecc6ed18adc9ce520ecdd7e 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 1f9b573f9388aafff3304358b8822a48075afb44 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 012dba4e24b9a94dc8da0d329baf4bec2d33ffca 
> 
> Diff: https://reviews.apache.org/r/43127/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 43415: CMake: Moved Windows build to version of glog that builds with CMake.

[M Lawindi]

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


Ship it!




Ship It!

- M Lawindi


On Feb. 10, 2016, 7:13 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43415/
> ---
> 
> (Updated Feb. 10, 2016, 7:13 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Moved Windows build to version of glog that builds with CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
> 
> Diff: https://reviews.apache.org/r/43415/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 42992: Support sharing of resources through reference counting of resources.

[Anindya Sinha]

> On Feb. 11, 2016, 3:14 p.m., Guangya Liu wrote:
> > src/common/resources.cpp, lines 202-205
> > 
> >
> > Does this require that the num_consumers should be same?

No. This just checks that we are comparing 2 resources that both have share set 
(for shareable resources), or niether have share set (for regular non-shareable 
resources). It does not mandate num_consumers to be equal.


> On Feb. 11, 2016, 3:14 p.m., Guangya Liu wrote:
> > src/common/resources.cpp, lines 335-338
> > 
> >
> > does this require the num_consumers be same when subtract?

No.  This just checks that we are comparing 2 resources that both have share 
set (for shareable resources), or niether have share set (for regular 
non-shareable resources). It does not mandate num_consumers to be equal.


- Anindya


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


On Feb. 5, 2016, 10:57 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42992/
> ---
> 
> (Updated Feb. 5, 2016, 10:57 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new Offer::Operation of SHARE and UNSHARE for resources.
> Added ShareInfo within Resources protobuf to allow for sharing of resources
> and keep track of consumers of such resources.
> Shareable resources keeps track of its consumers and makes decision based on
> that during task launch and task termination.
> Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af 
>   src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
>   src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2 
>   src/master/allocator/mesos/hierarchical.cpp 
> a9d2c23162892e0f97d89a076d2311091d91 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 18797e42a9c2bd20392020237cfae600a5ffe12c 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 66898e914c7b4ab83c4580be67530f355cfb05ca 
>   src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7 
> 
> Diff: https://reviews.apache.org/r/42992/diff/
> 
> 
> Testing
> ---
> 
> make check done.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 43483: Added logs for better debugging of allocation of `net_cls` handles.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43483]

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

- Mesos ReviewBot


On Feb. 11, 2016, 10:39 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43483/
> ---
> 
> (Updated Feb. 11, 2016, 10:39 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4657
> https://issues.apache.org/jira/browse/MESOS-4657
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added logs for better debugging of allocation of `net_cls` handles.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b22b9f6e9ba77d044c91203b76e0a7237315eb34 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> 2fc79973524d3776d39d378f22ac694030347c73 
> 
> Diff: https://reviews.apache.org/r/43483/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42754: Added support for labels to resource reservations.

[Michael Park]

> On Feb. 10, 2016, 12:19 a.m., Michael Park wrote:
> > include/mesos/type_utils.hpp, line 71
> > 
> >
> > We should keep the 2 newlines here. There are other parts that need 2 
> > lines also, for example between `bool operator!=(const TaskStatus& left, 
> > const TaskStatus& right);` and `inline bool operator==(const ContainerID& 
> > left, const ContainerID& right)`.
> 
> Neil Conway wrote:
> What is the rationale for keeping the two lines? It doesn't seem 
> consistent with the spacing in the rest of the file.
> 
> I fixed the second whitespace issue you mentioned.

Synced with Neil on this. I don't quite know the rationale for keeping __2__ 
newlines, but we use 2 newlines when we need "spacing" in the namespace scope, 
whereas we use 1 newline when we need "spacing" in a class scope. We need 
spacing when either of 2 declarations span multiple lines.

e.g.

```cpp
namespace X {

void F();
void G();

}  // namespace X {
```

```cpp
namespace X {

void F(
SomeReallyReallyLongType someReallyReallyLongName,
SomeReallyReallyLongType someReallyReallyLongName);


void G();

}  // namespace X {
```


- Michael


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


On Feb. 10, 2016, 6:59 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42754/
> ---
> 
> (Updated Feb. 10, 2016, 6:59 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4479
> https://issues.apache.org/jira/browse/MESOS-4479
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Labels are free-form key-value pairs that can be used to associate
> metadata with reserved resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/type_utils.hpp 16f9cda83edd5064527b89e94392a7a54411 
>   include/mesos/v1/mesos.hpp b294f022345dc892c0581622c5b389efbde911a9 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
>   src/common/type_utils.cpp 42e3061818c49c6b62d4fee2479db078dfe0fc89 
>   src/tests/mesos.hpp c0997dbacf8753149caba0a8b2406afc35a4845f 
>   src/tests/reservation_endpoints_tests.cpp 
> 0a8c479b6684a4497f53c81c198bfcf8e879aa54 
>   src/tests/resources_tests.cpp a190cc3900be72031e1c13e5843245f0f82dbf99 
>   src/v1/mesos.cpp 21b14cf7050270facab1ba2b844c3fde664665a4 
>   src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7 
> 
> Diff: https://reviews.apache.org/r/42754/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42754: Added support for labels to resource reservations.

[Michael Park]

> On Feb. 10, 2016, 8:18 a.m., Guangya Liu wrote:
> > include/mesos/v1/mesos.hpp, line 64
> > 
> >
> > two lines here

I'll follow up with a trivial formatting patch after this.


- Michael


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


On Feb. 10, 2016, 6:59 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42754/
> ---
> 
> (Updated Feb. 10, 2016, 6:59 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4479
> https://issues.apache.org/jira/browse/MESOS-4479
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Labels are free-form key-value pairs that can be used to associate
> metadata with reserved resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/type_utils.hpp 16f9cda83edd5064527b89e94392a7a54411 
>   include/mesos/v1/mesos.hpp b294f022345dc892c0581622c5b389efbde911a9 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
>   src/common/type_utils.cpp 42e3061818c49c6b62d4fee2479db078dfe0fc89 
>   src/tests/mesos.hpp c0997dbacf8753149caba0a8b2406afc35a4845f 
>   src/tests/reservation_endpoints_tests.cpp 
> 0a8c479b6684a4497f53c81c198bfcf8e879aa54 
>   src/tests/resources_tests.cpp a190cc3900be72031e1c13e5843245f0f82dbf99 
>   src/v1/mesos.cpp 21b14cf7050270facab1ba2b844c3fde664665a4 
>   src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7 
> 
> Diff: https://reviews.apache.org/r/42754/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43493: Corrected links to slides.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Feb. 11, 2016, 7:25 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43493/
> ---
> 
> (Updated Feb. 11, 2016, 7:25 p.m.)
> 
> 
> Review request for mesos, Guangya Liu and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/presentations.md 2a581b2acdbbaa526764bfb685f41d651485876e 
> 
> Diff: https://reviews.apache.org/r/43493/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 43407: CMake: Force GMock and libevent to build and link statically.

[M Lawindi]

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


Fix it, then Ship it!




Ship It!


3rdparty/libprocess/3rdparty/CMakeLists.txt (line 182)


Fix spaces/tabs


- M Lawindi


On Feb. 10, 2016, 11:36 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43407/
> ---
> 
> (Updated Feb. 10, 2016, 11:36 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Force GMock to build and link statically.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
> 
> Diff: https://reviews.apache.org/r/43407/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43418: Windows: Added slave/status_update_manager.cpp and other files.

[M Lawindi]

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


Ship it!




Ship It!

- M Lawindi


On Feb. 10, 2016, 7:13 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43418/
> ---
> 
> (Updated Feb. 10, 2016, 7:13 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added slave/status_update_manager.cpp and other files.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 9ab84c0898b3adce6063cc50b04ee74cf1471609 
> 
> Diff: https://reviews.apache.org/r/43418/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43483: Added logs for better debugging of allocation of `net_cls` handles.

[Avinash sridharan]

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

(Updated Feb. 11, 2016, 10:39 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added logs for better debugging of allocation of `net_cls` handles.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
b22b9f6e9ba77d044c91203b76e0a7237315eb34 
  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
2fc79973524d3776d39d378f22ac694030347c73 

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


Testing
---

make and make check


Thanks,

Avinash sridharan

Re: Review Request 43198: Added common appc spec utilities.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 11, 2016, 9:16 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43198/
> ---
> 
> (Updated Feb. 11, 2016, 9:16 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4596
> https://issues.apache.org/jira/browse/MESOS-4596
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds common utility functions such as :
>   - validating image information against actual data in the image directory.
>   - getting list of dependencies at depth 1 for an image.
>   - getting image path simple image discovery.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.hpp 462159daeb5c5a75dd5c2c340d797d3cfd4d7e11 
>   src/appc/spec.cpp fc36a0b5bc36c236cc4f49210d0058b34d0ffc40 
> 
> Diff: https://reviews.apache.org/r/43198/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 43127: Introduced Appc image cache.

[Jojy Varghese]

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

(Updated Feb. 11, 2016, 9:16 p.m.)


Review request for mesos and Jie Yu.


Changes
---

review addressed.


Bugs: MESOS-4439 and MESOS-4575
https://issues.apache.org/jira/browse/MESOS-4439
https://issues.apache.org/jira/browse/MESOS-4575


Repository: mesos


Description
---

Image cache will cache metadata about all Appc images stores in the store's
image directory. This cache could be useful to:
  - Quickly query if an image is present in the store.
  - By sharing the cache with other components like fetcher, an image can be
  checked if its present before fetching it.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/appc/cache.hpp 
4a63d479d3328605bac08fddffe190dbe21ad2b7 
  src/slave/containerizer/mesos/provisioner/appc/cache.cpp 
af69db3cdfea05c72ecc6ed18adc9ce520ecdd7e 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
1f9b573f9388aafff3304358b8822a48075afb44 
  src/tests/containerizer/provisioner_appc_tests.cpp 
012dba4e24b9a94dc8da0d329baf4bec2d33ffca 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 43409: Windows: Added `src/resource_estimator.cpp` to build.

[M Lawindi]

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


Ship it!




Ship It!

- M Lawindi


On Feb. 10, 2016, 6:44 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43409/
> ---
> 
> (Updated Feb. 10, 2016, 6:44 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `src/resource_estimator.cpp` to build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 9ab84c0898b3adce6063cc50b04ee74cf1471609 
> 
> Diff: https://reviews.apache.org/r/43409/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43414: CMake:[2/2] Canonicalize location of third-party dependencies.

[M Lawindi]

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


Ship it!




Ship It!

- M Lawindi


On Feb. 10, 2016, 7:13 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43414/
> ---
> 
> (Updated Feb. 10, 2016, 7:13 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[2/2] Canonicalize location of third-party dependencies.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
> 
> Diff: https://reviews.apache.org/r/43414/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43499: Improved docs for writing HA frameworks.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Feb. 11, 2016, 8:21 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43499/
> ---
> 
> (Updated Feb. 11, 2016, 8:21 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved docs for writing HA frameworks.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> faae27d2c11d5fa8f3c55d3a6ba5fa751cea057a 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
> 
> Diff: https://reviews.apache.org/r/43499/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43198: Added common appc spec utilities.

[Jojy Varghese]

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

(Updated Feb. 11, 2016, 9:16 p.m.)


Review request for mesos and Jie Yu.


Changes
---

fixed blank line issue.


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


Repository: mesos


Description
---

This change adds common utility functions such as :
  - validating image information against actual data in the image directory.
  - getting list of dependencies at depth 1 for an image.
  - getting image path simple image discovery.


Diffs (updated)
-

  include/mesos/appc/spec.hpp 462159daeb5c5a75dd5c2c340d797d3cfd4d7e11 
  src/appc/spec.cpp fc36a0b5bc36c236cc4f49210d0058b34d0ffc40 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 43499: Improved docs for writing HA frameworks.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43499]

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

- Mesos ReviewBot


On Feb. 11, 2016, 8:21 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43499/
> ---
> 
> (Updated Feb. 11, 2016, 8:21 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved docs for writing HA frameworks.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> faae27d2c11d5fa8f3c55d3a6ba5fa751cea057a 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
> 
> Diff: https://reviews.apache.org/r/43499/diff/
> 
> 
> Testing
> ---
> 
> Previewed in site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Travis Hegner]

On Feb. 9, 2016, 7:13 p.m., Travis Hegner wrote:
> > hanks
> 
> Joerg Schad wrote:
> Oh btw regarding the commit message:
> We usually have commit messages stating what changed, so in your case it 
> could be something along the lines of 'Added support for new docker network 
> setting.'

Understood. I will take care of both of these.


- Travis


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


On Feb. 4, 2016, 9:27 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 4, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Kapil Arya.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Travis Hegner]

> On Feb. 9, 2016, 7:13 p.m., Joerg Schad wrote:
> > src/docker/docker.cpp, line 307
> > 
> >
> > Are these additional checks which should apply in both cases (i.e. 
> > deprecated and new `addressLocation`? I.e. prior to your change it did not 
> > return an error if `HostConfig.NetworkMode` was not set, or am I mistaken?
> > Feel free to drop if this is the intended behavior!
> 
> haosdent huang wrote:
> `HostConfig.NetworkMode` exists since api 
> [1.15](https://docs.docker.com/engine/reference/api/docker_remote_api_v1.15/) 
> I think it's better we use the old way to get ip first. If we get the ip by 
> old way failed, try use `HostConfig.NetworkMode` and the new way to get ip 
> again.
> 
> Guangya Liu wrote:
> +1 to haosdent, we can first use old way to get IP address then if old 
> way failed, use the new way to get IP address.
> 
> Kapil Arya wrote:
> I think it's better to first attempt the new API, since that's forward 
> looking. If that fails, then we can go for the deprecated route.
> 
> Joerg Schad wrote:
> I wasn't so much concerned about the order of old vs new. The additional 
> check for `HostConfig.NetworkMode` just made me curios. Imagine a scenario 
> using the old way via `NetworkSettings.IPAddress` but not setting 
> `HostConfig.NetworkMode`: After this patch this will fail with `Unable to 
> detect network mode.`. Is that a scenario which might break currently working 
> setups?
> 
> haosdent huang wrote:
> >Is that a scenario which might break currently working setups
> 
> Yes, `HostConfig.NetworkMode` need make sure docker version >=1.3.x.

I don't believe that using the old way first is the correct route to go because 
that field still exists in the new docker API, it is just left blank when using 
a custom network driver. I can't base it off of it's existence, and I can't 
base it off of whether or not the field is blank, because blank might be an 
expected scenario if the container was started without networking.

I agree with Kapil that we should check the new API first, but Haosdent has a 
valid point that the check of HostConfig.NetworkMode could cause a failure 
scenario with older docker versions. I will work on handling that error 
condition in a more graceful way.


> On Feb. 9, 2016, 7:13 p.m., Joerg Schad wrote:
> > src/docker/docker.cpp, line 319
> > 
> >
> > Any reason you have a blank line here?

Likely missed it, or I'm not accustomed to the mesos style yet. I will take 
care of this.


- Travis


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


On Feb. 4, 2016, 9:27 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 4, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Kapil Arya.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Travis Hegner]

On Feb. 9, 2016, 7:13 p.m., Travis Hegner wrote:
> > hanks
> 
> Joerg Schad wrote:
> Oh btw regarding the commit message:
> We usually have commit messages stating what changed, so in your case it 
> could be something along the lines of 'Added support for new docker network 
> setting.'
> 
> Travis Hegner wrote:
> Understood. I will take care of both of these.

A couple of quick questions if I may:

1. Would it be possible or advisable to refactor this code based off of a 
detected docker version? I saw some tests with regard to the docker version, 
but I'm not familiar enough with the project to know if that is a route to go. 
Essentially, if docker version > 1.9.1 use new API otherwise use the old one? 
Perhaps there is a detection for the actual API version as well?
2. How would one test the various execution paths? Would I have to supply a 
sample inspect output for the various versions as part of the test, and then 
validate how the code handles each of the different version samples? I don't 
know how else one would test each possible condition. I did see that some tests 
supplied a sample json blob to test against... would that be advisable in this 
case?
3. Should I be basing this patch off of the master branch, or the latest 
release? If it makes a difference, I am hopeful to get this patch into 0.27.1, 
if it's not already too late for that.


- Travis


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


On Feb. 4, 2016, 9:27 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 4, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Kapil Arya.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Review Request 43490: Added testcase for TASK_KILLING state.

[Abhishek Dasgupta]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Added testcase for TASK_KILLING state.


Diffs
-

  src/tests/scheduler_tests.cpp 37f17094b3f11fd02468bf51b51b8e65ccb350a9 

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


Testing
---

make check


Thanks,

Abhishek Dasgupta

Review Request 43489: KillTask introduces TASK_KILLING state.

[Abhishek Dasgupta]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

KillTask introduces TASK_KILLING state.


Diffs
-

  src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
  src/launcher/executor.cpp c27e0792e4b9bcec0829a46be4232c013d965cf9 

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


Testing
---

make check


Thanks,

Abhishek Dasgupta

Re: Review Request 43201: Updated reservation documentation.

[Greg Mann]

> On Feb. 11, 2016, 2:59 a.m., Guangya Liu wrote:
> > docs/reservation.md, lines 61-63
> > 
> >
> > What about putting NOTE to a new line?
> > 
> > > NOTE: As of 0.27.0, these endpoints cannot be used when HTTP 
> > authentication is disabled due to the current implementation. This will 
> > change in version 0.28.0.
> 
> Greg Mann wrote:
> I tried this and viewed it using the website Docker container. It turns 
> out, forcing a newline before the "NOTE:" also inserts a vertical space, so 
> it's less obvious that the "NOTE" is meant to apply to only the second 
> bullet-point. I think it looks a bit better if we leave this passage as it 
> is, let me know what you think.
> 
> Guangya Liu wrote:
> There are different kinds of `NOTE` in the documents, but most of the 
> `NOTE` start from a new line, that's why I propose that we use a new line to 
> highlight `NOTE`.
> 
> 1) 
> https://github.com/apache/mesos/blob/master/docs/upgrades.md#upgrading-from-022x-to-023x
> 2) 
> https://github.com/apache/mesos/blob/master/docs/slave-recovery.md#enabling-slave-checkpointing
> 3) 
> https://github.com/apache/mesos/blob/master/docs/upgrades.md#upgrading-from-0140-to-0150

In link #2, I actually see a number of NOTEs which do not use a newline. They 
are placed in a bullet point list, which is the same as in this patch. The 
problem is that in order to insert a newline in the bullet point list, you also 
have to insert a vertical space, which makes it less obvious what the NOTE is 
referring to.


- Greg


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


On Feb. 11, 2016, 1:39 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43201/
> ---
> 
> (Updated Feb. 11, 2016, 1:39 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Bugs: MESOS-4452
> https://issues.apache.org/jira/browse/MESOS-4452
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated reservation documentation.
> 
> Clarified differences between the RESERVE offer operation and the `/reserve` 
> operator endpoint.
> 
> 
> Diffs
> -
> 
>   docs/reservation.md 25337109ff19240f926667961a59323bbfeb9956 
> 
> Diff: https://reviews.apache.org/r/43201/diff/
> 
> 
> Testing
> ---
> 
> Viewed in the mesos website container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43417: Windows: Marked functions in headers `inline` to avoid linker errors.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Feb. 9, 2016, 11:13 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43417/
> ---
> 
> (Updated Feb. 9, 2016, 11:13 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Marked functions in headers `inline` to avoid linker errors.
> 
> 
> Diffs
> -
> 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
> 046388189823c0c41ce6cc135d5d3838e9131087 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> 9a592c4ec9f45fdd8ae8c724c3cab67876de72f5 
> 
> Diff: https://reviews.apache.org/r/43417/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 43488: Adding framework capability for TASK_KILLING.

[Abhishek Dasgupta]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Adding framework capability for TASK_KILLING.


Diffs
-

  include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
  include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 

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


Testing
---

make check


Thanks,

Abhishek Dasgupta

Re: Review Request 43336: Introduced Appc image fetcher.

[Jojy Varghese]

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

(Updated Feb. 11, 2016, 5:52 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Added implementation for simple image discovery for Appc images.

TODO: Add tests.


Diffs (updated)
-

  src/CMakeLists.txt 4a2954498efa48a4eb43f82827ff1d6f5f65d389 
  src/Makefile.am 22f51319a1c06da02cac5822d42315e3027cf500 
  src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp PRE-CREATION 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 24a23325cc255d0d7b7af7ed096b6d3012ad75c7 

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


Testing
---

Tested with local http(s) server.


Thanks,

Jojy Varghese

Re: Review Request 43411: Windows: Added dynamic library loading tests to build.

[Daniel Pravat]

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


Ship it!




Ship It!

- Daniel Pravat


On Feb. 10, 2016, 6:47 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> ---
> 
> (Updated Feb. 10, 2016, 6:47 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dynamic library loading tests to build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 
> 27626ae28db090f1a002239ff5c674b82e8fc9a8 
> 
> Diff: https://reviews.apache.org/r/43411/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43411: Windows: Added dynamic library loading tests to build.

[Daniel Pravat]

> On Feb. 11, 2016, 6 p.m., Daniel Pravat wrote:
> > Ship It!

RB diff is showing a more complicated picture.


- Daniel


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


On Feb. 10, 2016, 6:47 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> ---
> 
> (Updated Feb. 10, 2016, 6:47 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dynamic library loading tests to build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 
> 27626ae28db090f1a002239ff5c674b82e8fc9a8 
> 
> Diff: https://reviews.apache.org/r/43411/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Klaus Ma]

> On Feb. 11, 2016, 5:34 p.m., Klaus Ma wrote:
> > src/docker/docker.cpp, line 529
> > 
> >
> > Also check whether `network_name()` is empty string `""`.
> 
> Ezra Silvera wrote:
> I think we already covered this issue in previous review comments. Is 
> there anything special you think is missing ?
> 
> Ezra Silvera 3 weeks ago (Jan. 20, 2016, 12:47 p.m.)
> 
> Guangya Liu, I went through the testing functions and I'm not sure we 
> actually need to add test for this.  Currently there are 3 possible values 
> that can be passed BRIDGE, HOST, NONE. There isn't any test that check the 
> NONE and HOST values. In fact there is  not even a functionality test for 
> BRIDGE value but simply when creating containers the BRIDGE value is used.
> We added a new value "USER" which is identical in terms of code flow 
> to HOST and NONE so I'm not sure we should add a special test for USER while 
> all other values are not tested.
> Further more in order to create a container with USER we will need to 
> create the actual user-network on the docker engine - this will require us to 
> add the functionality for  "network create" into docker.cpp. This 
> functionality is not needed at all for the mesos and will be used only  for 
> this test.
> The same is true for port mapping - maybe we had to better clarify it 
> on the comments - we didn't change any functionality at all and didn't touch 
> any code related to port mapping or network functionality. We are simply 
> passing a new value for the  --net parameter to the docker engine, nothing 
> else has changed.
> What do you think?
> 
> Ezra Silvera wrote:
> My comment was for teh testing. We will add a test for "" string (we 
> actually started with that and switch to the current test due to review 
> comments. :-)

For the test, using user-defined network is fine (similar to test using 
BRIDGE). For the empty string, current code is only check whether 
`network_name()` is set; if it's set to `""`, I'm not sure the behaviour but I 
think we it's better to return error before launching docker command line with 
empty string.


- Klaus


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


On Feb. 10, 2016, 4:34 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 4:34 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera 
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 43336: Introduced Appc image fetcher.

[Jojy Varghese]

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

(Updated Feb. 11, 2016, 5:28 p.m.)


Review request for mesos and Jie Yu.


Changes
---

addressed review.


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


Repository: mesos


Description
---

Added implementation for simple image discovery for Appc images.

TODO: Add tests.


Diffs (updated)
-

  src/CMakeLists.txt 4a2954498efa48a4eb43f82827ff1d6f5f65d389 
  src/Makefile.am 22f51319a1c06da02cac5822d42315e3027cf500 
  src/slave/containerizer/mesos/provisioner/appc/fetcher.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp PRE-CREATION 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 24a23325cc255d0d7b7af7ed096b6d3012ad75c7 

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


Testing
---

Tested with local http(s) server.


Thanks,

Jojy Varghese

Re: Review Request 43490: Added testcase for TASK_KILLING state.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43487, 43488, 43489, 43490]

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

- Mesos ReviewBot


On Feb. 11, 2016, 5:12 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43490/
> ---
> 
> (Updated Feb. 11, 2016, 5:12 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Qian Zhang.
> 
> 
> Bugs: MESOS-4547
> https://issues.apache.org/jira/browse/MESOS-4547
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcase for TASK_KILLING state.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 37f17094b3f11fd02468bf51b51b8e65ccb350a9 
> 
> Diff: https://reviews.apache.org/r/43490/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>