Re: Review Request 49616: Add suppression benchmark.

[Jacob Janco]

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

(Updated July 26, 2016, 7:03 a.m.)


Review request for mesos, James Peach, Joris Van Remoortere, and Jiang Yan Xu.


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


Repository: mesos


Description
---

- Useful for high framework clusters which carry
  many suppressed frameworks that are considered
  during allocation.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
bb6947fcecb5b78047e98d10fc1278c612a69548 

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


Testing
---

MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.SuppressOffers*" make check


Thanks,

Jacob Janco

Re: Review Request 49616: Add suppression benchmark.

[Jacob Janco]

> On July 26, 2016, 6 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 3637
> > 
> >
> > Add the elapse time of adding slave here, an example here 
> > https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3714

Collapsed this down to adding slaves rather than tracking for adding frameworks 
as in the scenario the frameworks are added without resources available. 
Instead why not track frameworks/adding slaves in with a portion of resources 
used? What do you think?


- Jacob


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


On July 26, 2016, 7:03 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49616/
> ---
> 
> (Updated July 26, 2016, 7:03 a.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5781
> https://issues.apache.org/jira/browse/MESOS-5781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Useful for high framework clusters which carry
>   many suppressed frameworks that are considered
>   during allocation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> bb6947fcecb5b78047e98d10fc1278c612a69548 
> 
> Diff: https://reviews.apache.org/r/49616/diff/
> 
> 
> Testing
> ---
> 
> MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.SuppressOffers*" make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49616: Add suppression benchmark.

[Guangya Liu]

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




src/tests/hierarchical_allocator_tests.cpp (lines 3792 - 3816)


I prefer that we calcluate the elapse time of `addSlave` and `addFramework` 
separately.

What about update the code as this:

```
  Stopwatch watch;
  watch.start();

  for (size_t i = 0; i < frameworkCount; i++) {
frameworks.push_back(createFrameworkInfo("*"));
allocator->addFramework(frameworks[i].id(), frameworks[i], {});
  }

  // Wait for all the `addFramework` operations to be processed.
  Clock::settle();
  
  watch.stop();
  
  cout << "Added " << frameworkCount << " frameworks"
   << " in " << watch.elapsed() << endl;

  // Each agent has a portion of it's resources allocated to a single
  // framework. We round-robin through the frameworks when allocating.
  vector agents;
  agents.reserve(agentCount);

  const string agentTotal = 
"cpus:24;mem:4096;disk:4096;ports:[31000-32000]";
  Resources allocation = 
Resources::parse("cpus:16;mem:1024;disk:1024").get();
  
  Try<::mesos::Value::Ranges> ranges = fragment(createRange(31000, 32000), 
16);
  ASSERT_SOME(ranges);
  ASSERT_EQ(16, ranges->range_size());

  allocation += createPorts(ranges.get());

  for (size_t i = 0; i < agentCount; i++) {
agents.push_back(createSlaveInfo(agentTotal));

hashmap used;
used[frameworks[i % frameworkCount].id()] = allocation;

allocator->addSlave(
agents[i].id(), agents[i], None(), agents[i].resources(), used);
  }

  // Wait for all the `addSlave` operations to be processed.
  Clock::settle();

  watch.stop();

  cout << "Added " << slaveCount << " agents"
   << " in " << watch.elapsed() << endl;
```



src/tests/hierarchical_allocator_tests.cpp (line 3800)


A comment here:

// Wait for all the `addFramework` operations to be processed.



src/tests/hierarchical_allocator_tests.cpp (line 3809)


It is better adding some check here.

```
Try<::mesos::Value::Ranges> ranges = fragment(createRange(31000, 32000), 
16);
ASSERT_SOME(ranges);
ASSERT_EQ(16, ranges->range_size());

allocation += createPorts(ranges.get());
```


- Guangya Liu


On 七月 26, 2016, 7:03 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49616/
> ---
> 
> (Updated 七月 26, 2016, 7:03 a.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5781
> https://issues.apache.org/jira/browse/MESOS-5781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Useful for high framework clusters which carry
>   many suppressed frameworks that are considered
>   during allocation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> bb6947fcecb5b78047e98d10fc1278c612a69548 
> 
> Diff: https://reviews.apache.org/r/49616/diff/
> 
> 
> Testing
> ---
> 
> MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.SuppressOffers*" make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49616: Add suppression benchmark.

[Guangya Liu]

> On 七月 26, 2016, 6 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 3637
> > 
> >
> > Add the elapse time of adding slave here, an example here 
> > https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3714
> 
> Jacob Janco wrote:
> Collapsed this down to adding slaves rather than tracking for adding 
> frameworks as in the scenario the frameworks are added without resources 
> available. Instead why not track frameworks/adding slaves in with a portion 
> of resources used? What do you think?

Thanks Jacob, I think that tracking those values separatelly maybe better. Even 
though adding framework will have no resources, but this can calculate the 
elapse time of adding frameworks and the time of allocator caused if there are 
no agents.


- Guangya


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


On 七月 26, 2016, 7:03 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49616/
> ---
> 
> (Updated 七月 26, 2016, 7:03 a.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5781
> https://issues.apache.org/jira/browse/MESOS-5781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Useful for high framework clusters which carry
>   many suppressed frameworks that are considered
>   during allocation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> bb6947fcecb5b78047e98d10fc1278c612a69548 
> 
> Diff: https://reviews.apache.org/r/49616/diff/
> 
> 
> Testing
> ---
> 
> MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.SuppressOffers*" make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 49616: Add suppression benchmark.

[Jacob Janco]

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

(Updated July 26, 2016, 7:28 a.m.)


Review request for mesos, James Peach, Joris Van Remoortere, and Jiang Yan Xu.


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


Repository: mesos


Description
---

- Useful for high framework clusters which carry
  many suppressed frameworks that are considered
  during allocation.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
bb6947fcecb5b78047e98d10fc1278c612a69548 

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


Testing
---

MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.SuppressOffers*" make check


Thanks,

Jacob Janco

Re: Review Request 49616: Add suppression benchmark.

[Jacob Janco]

> On July 26, 2016, 6 a.m., Guangya Liu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 3637
> > 
> >
> > Add the elapse time of adding slave here, an example here 
> > https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L3714
> 
> Jacob Janco wrote:
> Collapsed this down to adding slaves rather than tracking for adding 
> frameworks as in the scenario the frameworks are added without resources 
> available. Instead why not track frameworks/adding slaves in with a portion 
> of resources used? What do you think?
> 
> Guangya Liu wrote:
> Thanks Jacob, I think that tracking those values separatelly maybe 
> better. Even though adding framework will have no resources, but this can 
> calculate the elapse time of adding frameworks and the time of allocator 
> caused if there are no agents.

Sounds good, adding.


- Jacob


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


On July 26, 2016, 7:28 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49616/
> ---
> 
> (Updated July 26, 2016, 7:28 a.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5781
> https://issues.apache.org/jira/browse/MESOS-5781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Useful for high framework clusters which carry
>   many suppressed frameworks that are considered
>   during allocation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> bb6947fcecb5b78047e98d10fc1278c612a69548 
> 
> Diff: https://reviews.apache.org/r/49616/diff/
> 
> 
> Testing
> ---
> 
> MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.SuppressOffers*" make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Review Request 50439: Made the log in `cgroups::prepare` more accurate.

[haosdent huang]

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

Review request for mesos, Zhiwei Chen and Jie Yu.


Repository: mesos


Description
---

Made the log in `cgroups::prepare` more accurate.


Diffs
-

  src/linux/cgroups.cpp ada0cd493ef02a0a9f4c7fe170c62cb7697184dc 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 38627: Adds an overload of ModuleManager::create() allowing overriding parameters programatically

[Jay Guo]

> On Oct. 21, 2015, 7:02 a.m., Klaus Ma wrote:
> > src/module/manager.hpp, line 94
> > 
> >
> > Should we merge with `moduleParameters[moduleName]` or replace it? IMO, 
> > prefer to merge them; I'd like to leave it to you and your shepherd.
> 
> Alexander Rojas wrote:
> I don't understand what you mean with this comment? is the same as the 
> following one?
> 
> Klaus Ma wrote:
> for example, if `{a:1, b:2}` in `moduleParameters[moduleName]` and `{b:3, 
> c:4}` in `params`, what's expected result? `{a:1, b:3, c:4}` or `{b:3, c:4}`.
> 
> Alexander Rojas wrote:
> Merge is not a good idea almost ever. In this case, if you have a 
> parameter which can appear once, and the user gave `foo=5` and the API gave 
> `foo=10` which one to choose?

Fairly an old thread but I do prefer `merge` over `replace`, in case some 
parameters are only available during runtime, e.g. PIDs. Wordaround does exist 
but negative consequences are stated in original story description. Thoughts?


- Jay


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


On Nov. 16, 2015, 10:08 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38627/
> ---
> 
> (Updated Nov. 16, 2015, 10:08 a.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, Niklas Nielsen, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3072
> https://issues.apache.org/jira/browse/MESOS-3072
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allows developers to provide their own parameters when loading modules 
> instead of using the ones provided by the user when loading Mesos. This helps 
> to deal with default modules (those used when the user doesn't provide any), 
> and for testing of the modules.
> 
> 
> Diffs
> -
> 
>   src/examples/example_module_impl.cpp 
> db015cea130701a4e0a6fcb890c79fbb0c02c1ce 
>   src/examples/test_module.hpp 0514963b6100a6e9a834f2b9670cebc310918fb8 
>   src/module/manager.hpp 302eb409fb8ef53b9cef8d2ecbe7b7f452b095ef 
>   src/tests/module.hpp 0820978441aede18dae6d1701433bff705b8c3c2 
>   src/tests/module_tests.cpp 60497aac3200ab9a679a81a593b5bf0d02fd4b50 
> 
> Diff: https://reviews.apache.org/r/38627/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Review Request 50441: Added a blank line in common/resources.cpp.

[Guangya Liu]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Added a blank line in common/resources.cpp to keep two lines
between each method.


Diffs
-

  src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 49616: Add suppression benchmark.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49616]

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

- Mesos ReviewBot


On July 26, 2016, 7:28 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49616/
> ---
> 
> (Updated July 26, 2016, 7:28 a.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5781
> https://issues.apache.org/jira/browse/MESOS-5781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Useful for high framework clusters which carry
>   many suppressed frameworks that are considered
>   during allocation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> bb6947fcecb5b78047e98d10fc1278c612a69548 
> 
> Diff: https://reviews.apache.org/r/49616/diff/
> 
> 
> Testing
> ---
> 
> MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.SuppressOffers*" make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 50439: Made the log in `cgroups::prepare` more accurate.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50439]

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

- Mesos ReviewBot


On July 26, 2016, 8:59 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50439/
> ---
> 
> (Updated July 26, 2016, 8:59 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the log in `cgroups::prepare` more accurate.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp ada0cd493ef02a0a9f4c7fe170c62cb7697184dc 
> 
> Diff: https://reviews.apache.org/r/50439/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50439: Made the log in `cgroups::prepare` more accurate.

[Qian Zhang]

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


Fix it, then Ship it!





src/linux/cgroups.cpp (line 432)


Need a space after `attached:`.


- Qian Zhang


On July 26, 2016, 4:59 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50439/
> ---
> 
> (Updated July 26, 2016, 4:59 p.m.)
> 
> 
> Review request for mesos, Zhiwei Chen and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the log in `cgroups::prepare` more accurate.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp ada0cd493ef02a0a9f4c7fe170c62cb7697184dc 
> 
> Diff: https://reviews.apache.org/r/50439/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50439: Made the log in `cgroups::prepare` more accurate.

[haosdent huang]

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

(Updated July 26, 2016, 12:51 p.m.)


Review request for mesos, Zhiwei Chen and Jie Yu.


Changes
---

Address @qianzhang's comment.


Repository: mesos


Description
---

Made the log in `cgroups::prepare` more accurate.


Diffs (updated)
-

  src/linux/cgroups.cpp ada0cd493ef02a0a9f4c7fe170c62cb7697184dc 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49849: Implemented `CpuSubsystem`.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (lines 169 - 171)


I think these 3 lines can be merged into one line which will not exceeds 80 
chars.


- Qian Zhang


On July 25, 2016, 10:48 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49849/
> ---
> 
> (Updated July 25, 2016, 10:48 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5042
> https://issues.apache.org/jira/browse/MESOS-5042
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `CpuSubsystem`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49849/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49850: Implemented `CpuacctSubsystem`.

[Qian Zhang]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (lines 199 - 201)


I think these 3 lines can be merged into a single line which will not 
exceeds 80 chars.


- Qian Zhang


On July 22, 2016, 3:10 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49850/
> ---
> 
> (Updated July 22, 2016, 3:10 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5043
> https://issues.apache.org/jira/browse/MESOS-5043
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `CpuacctSubsystem`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49850/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48291: Use varint comparator in replica log.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On June 6, 2016, 7:54 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48291/
> ---
> 
> (Updated June 6, 2016, 7:54 p.m.)
> 
> 
> Review request for mesos, Bing Li, Benjamin Mahler, Zhiwei Chen, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-970
> https://issues.apache.org/jira/browse/MESOS-970
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since bug discussed at
> https://groups.google.com/forum/#!topic/leveldb/F-rDkWiQm6c
> was fixed. After upgrading leveldb to 1.18 we could replace
> default byte-wise comparator with varint comparator.
> 
> 
> Diffs
> -
> 
>   src/log/leveldb.cpp f389d74b123574665c611b46cb52e3dc7042b331 
> 
> Diff: https://reviews.apache.org/r/48291/diff/
> 
> 
> Testing
> ---
> 
> `make check dist` on Ubuntu x64
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 50441: Added a blank line in common/resources.cpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50441]

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

- Mesos ReviewBot


On July 26, 2016, 10:21 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50441/
> ---
> 
> (Updated July 26, 2016, 10:21 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a blank line in common/resources.cpp to keep two lines
> between each method.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
> 
> Diff: https://reviews.apache.org/r/50441/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

[Qian Zhang]

> On July 26, 2016, 9:40 a.m., haosdent huang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, line 498
> > 
> >
> > ```
> > if (info->updatedLimit || limit > currentLimit.get()) {
> > ```

Can you check `info->pid.isNone()` just like what the existing cgroups memory 
isolator is doing?


- Qian


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


On July 22, 2016, 3:10 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 22, 2016, 3:10 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5045
> https://issues.apache.org/jira/browse/MESOS-5045
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `MemorySubsystem`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50439: Made the log in `cgroups::prepare` more accurate.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50439]

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

- Mesos ReviewBot


On July 26, 2016, 12:51 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50439/
> ---
> 
> (Updated July 26, 2016, 12:51 p.m.)
> 
> 
> Review request for mesos, Zhiwei Chen and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the log in `cgroups::prepare` more accurate.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp ada0cd493ef02a0a9f4c7fe170c62cb7697184dc 
> 
> Diff: https://reviews.apache.org/r/50439/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

[Jiang Yan Xu]

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



Haven't commented on tests. Will look at them together with other tests.

---

Chatted with BenM and Anindya. I think we are able to simplify the code and 
minimize the specially handling of shared resources by separating the 
responsibilities between master, allocator and sorter this way:

- Invariants: All usage of resources (shared or not) should have directly 
correponding allocations in the allocator and sorter.
- Allocator:
- The only thing special about shared resources in `allocate()` is that 
it's *available* once created regardless of past allocations (i.e., `total - 
allocated`), this adheres to the definition of shared resources and is easily 
explanable. When multiple instances of the same shared resources are allocated, 
all instances/copies are maintained in `slave.allocated` and other data 
structures.
- We just need to tweak `updateAllocation` for the master's special 
allocation.
- Sorter: Shared resources only needs a little special handling when creating 
`createStrippedScalarQuantity`, multiple instances can appear in a framework's 
allocation but they don't affect sorting.
- Master: The only special handlings are that 
- Task validation allows tasks that consume shared resources not in 
`_offeredResources` as long as they are in the original `offeredResources` or 
they are created in the same ACCEPT call. 
- Tasks may use more shared resources than being allocated, this leads to a 
special allocation for additional instances of these shared resources.

These semantics combined should result in simpler and smaller changes than 
maintaining other additional variables which are adjusted in special ways due 
to shared resources.


src/common/resources.cpp (line 1224)


Add comment.

// Because we currently only allow persistent volumes to be shared, the 
originally resource must be non-shared.



src/common/resources.cpp (line 1266)


Add a comment:

// Because we currently only allow persistent volumes to be shared, we 
return the resource to the non-shared state after destroy of the volume.



src/master/allocator/mesos/hierarchical.hpp (line 327)


s/each copy/the number of copies/



src/master/allocator/mesos/hierarchical.hpp (line 328)


s/allocated/allocated to/
s/recovered/recovered from/



src/master/allocator/mesos/hierarchical.cpp (lines 604 - 607)


Now we have to modify this method to process the LAUNCH call and adjust the 
API docs accordingly.



src/master/allocator/mesos/hierarchical.cpp (line 1429)


s/=/-/



src/master/allocator/sorter/drf/sorter.cpp (line 164)


s/no more/no existing/



src/master/allocator/sorter/drf/sorter.cpp (lines 165 - 166)


For this you want to exclude shared resources already allocated to the 
client but `resources` can have more instances of the same shared resources 
than `allocations[name].resources[slaveId].shared()`.



src/master/master.hpp (line 324)


s/shared resources/each shared resource/ (because later in this sentence 
you are referencing "this shared resource"



src/master/master.hpp (line 325)


Say "each copy corresponds to a running task using it" instead?



src/master/master.hpp (line 331)


s/assigned/requested/ (assigned is a bit ambiguous, at this point resources 
are still not given to the task yet).



src/master/master.hpp (line 340)


s/accept/ACCEPT/



src/master/master.hpp (line 341)


s/removing it/removing them/



src/master/master.hpp (line 344)


With `hashmap pendingResources` we are preventing 
DESTROY from affecting pending tasks of the same framework but letting DESTROY 
to go through under pending tasks of other frameworks (of the same role).

If we use `pendingResources` it should be across frameworks right?

But because we already need to add

```
multihashmap pendingTasks;
```

for /r/47082/,

we can just do that instead in this review, the number of tasks pending for 
each slave should be small enough to loop through quickly.

Re: Review Request 49819: Implemented `CgroupsIsolatorProcess::prepare`.

[haosdent huang]

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

(Updated July 26, 2016, 5:10 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::prepare`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49817: Implemented `CgroupsIsolatorProcess::recover`.

[haosdent huang]

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

(Updated July 26, 2016, 5:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::recover`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49821: Implemented `CgroupsIsolatorProcess::watch`.

[haosdent huang]

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

(Updated July 26, 2016, 5:10 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::watch`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49820: Implemented `CgroupsIsolatorProcess::isolate`.

[haosdent huang]

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

(Updated July 26, 2016, 5:10 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::isolate`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49824: Implemented `CgroupsIsolatorProcess::usage`.

[haosdent huang]

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

(Updated July 26, 2016, 5:11 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::usage`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49823: Implemented `CgroupsIsolatorProcess::update`.

[haosdent huang]

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

(Updated July 26, 2016, 5:11 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::update`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49827: Implemented `CgroupsIsolatorProcess::cleanup`.

[haosdent huang]

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

(Updated July 26, 2016, 5:11 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::cleanup`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49825: Implemented `CgroupsIsolatorProcess::status`.

[haosdent huang]

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

(Updated July 26, 2016, 5:11 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::status`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49828: Added default methods implementations for `Subsystem` base class.

[haosdent huang]

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

(Updated July 26, 2016, 5:12 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added default methods implementations for `Subsystem` base class.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49849: Implemented `CpuSubsystem`.

[haosdent huang]

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

(Updated July 26, 2016, 5:12 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address comments.


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


Repository: mesos


Description
---

Implemented `CpuSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49850: Implemented `CpuacctSubsystem`.

[haosdent huang]

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

(Updated July 26, 2016, 5:13 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @comments.


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


Repository: mesos


Description
---

Implemented `CpuacctSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

[haosdent huang]

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

(Updated July 26, 2016, 5:13 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address comments.


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


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

[haosdent huang]

> On July 26, 2016, 1:40 a.m., haosdent huang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, line 498
> > 
> >
> > ```
> > if (info->updatedLimit || limit > currentLimit.get()) {
> > ```
> 
> Qian Zhang wrote:
> Can you check `info->pid.isNone()` just like what the existing cgroups 
> memory isolator is doing?

Because we didn't store `pid` in `info` here. Another reason is I think use 
`updatedLimit` would more clear.


- haosdent


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


On July 26, 2016, 5:13 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 26, 2016, 5:13 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5045
> https://issues.apache.org/jira/browse/MESOS-5045
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `MemorySubsystem`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 348f105f9c3109a02f1dde0649f1b829cb9ddd04 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49852: Implemented `NetClsSubsystem`.

[haosdent huang]

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

(Updated July 26, 2016, 5:15 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address comments.


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


Repository: mesos


Description
---

Implemented `NetClsSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49854: Implemented `DevicesSubsystem`.

[haosdent huang]

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

(Updated July 26, 2016, 5:16 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `DevicesSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49853: Implemented `PerfEventSubsystem`.

[haosdent huang]

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

(Updated July 26, 2016, 5:15 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `PerfEventSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45573: Add `PerfEventHandleManager`.

[haosdent huang]

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

(Updated July 26, 2016, 5:15 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Add `PerfEventHandleManager`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
1de38491a33e5782b20e5ee491412779d09ee391 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
31f35385691681ef5da14be747edfb5f57c5d05a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49855: Enabled cgroups unified isolator in isolation.

[haosdent huang]

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

(Updated July 26, 2016, 5:16 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Enabled cgroups unified isolator in isolation.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49855: Enabled cgroups unified isolator in isolation.

[haosdent huang]

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

(Updated July 26, 2016, 5:18 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Enabled cgroups unified isolator in isolation.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50439: Made the log in `cgroups::prepare` more accurate.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 26, 2016, 12:51 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50439/
> ---
> 
> (Updated July 26, 2016, 12:51 p.m.)
> 
> 
> Review request for mesos, Zhiwei Chen and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the log in `cgroups::prepare` more accurate.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp ada0cd493ef02a0a9f4c7fe170c62cb7697184dc 
> 
> Diff: https://reviews.apache.org/r/50439/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49855: Enabled cgroups unified isolator in isolation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50038, 49814, 49817, 49819, 49820, 49821, 49823, 49824, 
49825, 49827, 49828, 49849, 49850, 49851, 49852, 45573, 49853, 49854, 49855]

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

- Mesos ReviewBot


On July 26, 2016, 5:18 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49855/
> ---
> 
> (Updated July 26, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5652
> https://issues.apache.org/jira/browse/MESOS-5652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled cgroups unified isolator in isolation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 86a8d8a85b6a33c87798108cb65af85bb9bbbc77 
> 
> Diff: https://reviews.apache.org/r/49855/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50214: Supported non-shell command in MesosLaunch to avoid arbitrary commands.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/launch.cpp (line 220)


I'd do the following:
```
Try s = Error("Not launched");

if (parse->shell()) {
  s = subprocess(...);
} else {
  s = subprocess(...);
}

if (s.isError()) {
  ...
}

Option status = s->status().await();
...
```



src/slave/containerizer/mesos/launch.cpp (line 250)


Not yours, but can you follow up with a patch to use return EXIT_FAILURE 
instead?


- Jie Yu


On July 26, 2016, 3:19 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50214/
> ---
> 
> (Updated July 26, 2016, 3:19 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5388
> https://issues.apache.org/jira/browse/MESOS-5388
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently all pre_exec_commands are executed as shell commands
> in Mesos Launch. It is not safe because arbitrary shell command
> may be included in some user facing api (e.g., container_path).
> We should execute those command as a subprocess to prevent
> arbitrary shell command injection.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 51f0c110ff0c414837fd69db81047979a0093388 
> 
> Diff: https://reviews.apache.org/r/50214/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 50414: Added variables for building Java Protobuf.

[Srinivas Brahmaroutu]

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

(Updated July 26, 2016, 7:34 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description (updated)
---

Added variables for building Java Protobuf.


Diffs (updated)
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
d34ea4062984a9f29910c830fb854df58d18c3d7 

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


Testing
---

cmake .. && make


Thanks,

Srinivas Brahmaroutu

Re: Review Request 50415: Added build step to build Java Protobuf.

[Srinivas Brahmaroutu]

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

(Updated July 26, 2016, 7:34 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description (updated)
---

Added build step to build Java Protobuf.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 770a3828a6e0ffaa4f185392fdc1a2152446449d 

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


Testing
---

cmake && make protobuf-2.6.1-java 
check if 
3rdparty/protobuf-2.6.1/src/protobuf-2.6.1-java/java/target/protobuf-java-2.6.1.jar


Thanks,

Srinivas Brahmaroutu

Review Request 50455: Enabled Java builds if Java is found.

[Srinivas Brahmaroutu]

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

Review request for mesos.


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


Repository: mesos


Description
---

If cmake is configured with ENABLE_JAVA=ON/1 then we detect 
Java and JNI and set up appropriate java environment.


Diffs
-

  3rdparty/CMakeLists.txt 770a3828a6e0ffaa4f185392fdc1a2152446449d 
  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 

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


Testing
---


Thanks,

Srinivas Brahmaroutu

Re: Review Request 50455: Enabled Java builds if Java is found.

[Srinivas Brahmaroutu]

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

(Updated July 26, 2016, 7:40 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

If cmake is configured with ENABLE_JAVA=ON/1 then we detect 
Java and JNI and set up appropriate java environment.


Diffs
-

  3rdparty/CMakeLists.txt 770a3828a6e0ffaa4f185392fdc1a2152446449d 
  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 

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


Testing (updated)
---

cmake ENABLE_JAVA=ON 
check build variables set for Java.
cmake should output
-- 
-- Java Found and will be enabled
-- 


Thanks,

Srinivas Brahmaroutu

Review Request 50454: Fixed ExamplesTest.DiskFullFramework on non-root Linux builds.

[Joseph Wu]

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

Review request for mesos, Artem Harutyunyan and Vinod Kone.


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


Repository: mesos


Description
---

This test does not need systemd support and is not run as root.


Diffs
-

  src/tests/disk_full_framework_test.sh 
3fc180b3f1eaea91ff3c7d4fa8bf16e7278c55c4 

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


Testing
---

Verified that the test passes on OSX and CentOS 7.


Thanks,

Joseph Wu

Review Request 50456: Added custom command to generate Java protobufs.

[Srinivas Brahmaroutu]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

The code loops through all proto definitions relevant to Java
and generated Protos.Java into proper output directories.


Diffs
-

  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
  src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 
  src/java/CMakeLists.txt PRE-CREATION 

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


Testing
---

cmake .. && make mesos-java-protos.
check to see if java/generated folder has all the new java files created.
/src/java/generated/org/apache/mesos/containerizer/Protos.java
/src/java/generated/org/apache/mesos/fetcher/Protos.java
/src/java/generated/org/apache/mesos/Protos.java
/src/java/generated/org/apache/mesos/scheduler/Protos.java
/src/java/generated/org/apache/mesos/v1/executor/Protos.java
/src/java/generated/org/apache/mesos/v1/Protos.java
/src/java/generated/org/apache/mesos/v1/scheduler/Protos.java


Thanks,

Srinivas Brahmaroutu

Review Request 50457: Added code to generate MesosNativeLibrary.java.

[Srinivas Brahmaroutu]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

Using configure_file (./config.status like) to generate
source file fmor the template.  This is required to build
the final mesos-java.jar.


Diffs
-

  src/java/CMakeLists.txt PRE-CREATION 

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


Testing
---

cmake .. 
Check to see if 
/src/java/generated/org/apache/mesos/MesosNativeLibrary.java is 
generated.


Thanks,

Srinivas Brahmaroutu

Review Request 50458: Added code to build mesos-java.jar.

[Srinivas Brahmaroutu]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

Build mesos-java.jar from the generated protos and the java source files.


Diffs
-

  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
  src/java/CMakeLists.txt PRE-CREATION 

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


Testing
---

cmake && make mesos-java 
Check to see if /src/java/mesos-java.jar


Thanks,

Srinivas Brahmaroutu

Review Request 50459: Added code to generate JNI Header files.

[Srinivas Brahmaroutu]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

Generate JNI headers from the java classes built in 
mesos-java.jar. Used for C++ to Java bindings used
when communicating with java based frameworks.


Diffs
-

  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
  src/java/CMakeLists.txt PRE-CREATION 

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


Testing
---

cmake && make mesos-jni-header
check to see if all the headers are generated.
/include/java/jni/org_apache_mesos_Log.h
/include/java/jni/org_apache_mesos_Log_Entry.h
/include/java/jni/org_apache_mesos_Log_OperationFailedException.h
/include/java/jni/org_apache_mesos_Log_Position.h
/include/java/jni/org_apache_mesos_Log_Reader.h
/include/java/jni/org_apache_mesos_Log_Writer.h
/include/java/jni/org_apache_mesos_Log_WriterFailedException.h
/include/java/jni/org_apache_mesos_MesosExecutorDriver.h
/include/java/jni/org_apache_mesos_MesosSchedulerDriver.h
/include/java/jni/org_apache_mesos_state_AbstractState.h
/include/java/jni/org_apache_mesos_state_AbstractState_ExpungeFuture.h
/include/java/jni/org_apache_mesos_state_AbstractState_FetchFuture.h
/include/java/jni/org_apache_mesos_state_AbstractState_NamesFuture.h
/include/java/jni/org_apache_mesos_state_AbstractState_StoreFuture.h
/include/java/jni/org_apache_mesos_state_LevelDBState.h
/include/java/jni/org_apache_mesos_state_LogState.h
/include/java/jni/org_apache_mesos_state_Variable.h
/include/java/jni/org_apache_mesos_state_ZooKeeperState.h


Thanks,

Srinivas Brahmaroutu

Re: Review Request 43569: Updated log message if container not found.

[Jie Yu]

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




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


We put `+` in the end


- Jie Yu


On March 23, 2016, 1:50 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43569/
> ---
> 
> (Updated March 23, 2016, 1:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated log message if container not found.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ee7a265975323ca891114a286357c8e42901560c 
> 
> Diff: https://reviews.apache.org/r/43569/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45011: Fix numify() to handle negative numbers consistently (MESOS-4070).

[Greg Mann]

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



Hi Yong,
Do you still have interest in getting this committed? If so, it looks like it 
needs to be rebased.

- Greg Mann


On March 18, 2016, 10:49 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45011/
> ---
> 
> (Updated March 18, 2016, 10:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Neil Conway, and Cong Wang.
> 
> 
> Bugs: MESOS-4070
> https://issues.apache.org/jira/browse/MESOS-4070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix updated the implementation of numify() so that negative numbers
> could be handled conssitently for hex and non-hex numbers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> 26a637bec1193dd51437bd689c34fbe6d1935d89 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp 
> 444377df00922df12d4b3ed25b4cfe9071cff5c3 
> 
> Diff: https://reviews.apache.org/r/45011/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 50454: Fixed ExamplesTest.DiskFullFramework on non-root Linux builds.

[Joseph Wu]

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

(Updated July 26, 2016, 1:31 p.m.)


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


Changes
---

Fix typo.


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


Repository: mesos


Description
---

This test does not need systemd support and is not run as root.


Diffs (updated)
-

  src/tests/disk_full_framework_test.sh 
3fc180b3f1eaea91ff3c7d4fa8bf16e7278c55c4 

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


Testing
---

Verified that the test passes on OSX and CentOS 7.


Thanks,

Joseph Wu

Re: Review Request 50454: Fixed ExamplesTest.DiskFullFramework on non-root Linux builds.

[Anand Mazumdar]

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



AFAICT, there are a couple of issues at play here:
- The test always failing on some distros.
- The test running forever due to a race condition.

It's not immediatly clear from the review description as to why disabling 
systemd helps with the latter i.e. avoiding the race. Can you update the 
description to reflect that?

- Anand Mazumdar


On July 26, 2016, 8:31 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50454/
> ---
> 
> (Updated July 26, 2016, 8:31 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Vinod Kone.
> 
> 
> Bugs: MESOS-5907
> https://issues.apache.org/jira/browse/MESOS-5907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test does not need systemd support and is not run as root.
> 
> 
> Diffs
> -
> 
>   src/tests/disk_full_framework_test.sh 
> 3fc180b3f1eaea91ff3c7d4fa8bf16e7278c55c4 
> 
> Diff: https://reviews.apache.org/r/50454/diff/
> 
> 
> Testing
> ---
> 
> Verified that the test passes on OSX and CentOS 7.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 50454: Fixed ExamplesTest.DiskFullFramework on non-root Linux builds.

[Joseph Wu]

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

(Updated July 26, 2016, 2 p.m.)


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


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


Repository: mesos


Description (updated)
---

The example tests will check if an agent exits after two seconds, under
the assumption that the agent fails fast if configured incorrectly.  
On systems with systemd, the two second timeout may be insufficient
as there is additional logic/validation for initializing systemd.

The test suite does not need systemd support in the first place, 
so the agent launched in the DiskFullFramework example test should
explicitly disable systemd logic.


Diffs
-

  src/tests/disk_full_framework_test.sh 
3fc180b3f1eaea91ff3c7d4fa8bf16e7278c55c4 

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


Testing
---

Verified that the test passes on OSX and CentOS 7.


Thanks,

Joseph Wu

Re: Review Request 50454: Fixed ExamplesTest.DiskFullFramework on non-root Linux builds.

[Anand Mazumdar]

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


Ship it!




LGTM

- Anand Mazumdar


On July 26, 2016, 9 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50454/
> ---
> 
> (Updated July 26, 2016, 9 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Vinod Kone.
> 
> 
> Bugs: MESOS-5907
> https://issues.apache.org/jira/browse/MESOS-5907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The example tests will check if an agent exits after two seconds, under
> the assumption that the agent fails fast if configured incorrectly.  
> On systems with systemd, the two second timeout may be insufficient
> as there is additional logic/validation for initializing systemd.
> 
> The test suite does not need systemd support in the first place, 
> so the agent launched in the DiskFullFramework example test should
> explicitly disable systemd logic.
> 
> 
> Diffs
> -
> 
>   src/tests/disk_full_framework_test.sh 
> 3fc180b3f1eaea91ff3c7d4fa8bf16e7278c55c4 
> 
> Diff: https://reviews.apache.org/r/50454/diff/
> 
> 
> Testing
> ---
> 
> Verified that the test passes on OSX and CentOS 7.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 50215: Updated pre exec commands as non-shell in docker volume isolator.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 26, 2016, 9:08 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50215/
> ---
> 
> (Updated July 26, 2016, 9:08 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5388
> https://issues.apache.org/jira/browse/MESOS-5388
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By adding apostrophes to mount 'source' and 'target', arbitraty
> commands defined by users postfixed to 'container_path' will
> take no effect. 'mount' command will return an error for invalid
> mount 'target'.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 96806a75e6f7abc3a229c01b375fdba30d267ab4 
> 
> Diff: https://reviews.apache.org/r/50215/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 40410: Libprocess Reinit: Move MetricsProcess instantiation into process.cpp.

[Joseph Wu]

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

(Updated July 26, 2016, 2:13 p.m.)


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


Changes
---

Rebase this patch as it does not apply.


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


Repository: mesos


Description
---

The metrics singleton must be unified with `process::initialize` so 
that it also falls under the scope of reinitialization.  The singleton 
must also not be guarded by `Once`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
54487ab2614c7f8e8df10d1e0c39880a6cf5bde3 
  3rdparty/libprocess/src/metrics/metrics.cpp 
ac1544e70f5884f946ce3c31c5430c6a2c1f9dd1 
  3rdparty/libprocess/src/process.cpp 7f331b812de2f0437838f48e0959441c8e04c358 
  3rdparty/libprocess/src/tests/system_tests.cpp 
0f4d0424689522337806ba2227ec4330c700e17e 

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


Testing
---

Tests done in a subsequent review.


Thanks,

Joseph Wu

Re: Review Request 50215: Updated pre exec commands as non-shell in docker volume isolator.

[Gilbert Song]

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

(Updated July 26, 2016, 2:08 p.m.)


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


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


Repository: mesos


Description (updated)
---

By adding apostrophes to mount 'source' and 'target', arbitraty
commands defined by users postfixed to 'container_path' will
take no effect. 'mount' command will return an error for invalid
mount 'target'.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
96806a75e6f7abc3a229c01b375fdba30d267ab4 

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


Testing
---

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song

Re: Review Request 40268: Libprocess Reinit: Change Socket::DEFAULT_KIND to a non-static value.

[Joseph Wu]

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

(Updated July 26, 2016, 2:15 p.m.)


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


Changes
---

Rebase this patch as it does not apply.


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


Repository: mesos


Description
---

This is needed for tests that utilize the test-only 
`process::reinitialize` function.


Diffs (updated)
-

  3rdparty/libprocess/include/process/socket.hpp 
881b44b987e5894cac838dae046ab7dbad20b000 
  3rdparty/libprocess/src/socket.cpp 668f26d85cf3097ff9b90a10be14c5833fafd558 

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


Testing
---

Some test cleanup issues were uncovered by this change; fixed here: 
https://reviews.apache.org/r/40453/


Thanks,

Joseph Wu

Re: Review Request 40512: Libprocess Reinit: Add a test-only method to reinitialize libprocess.

[Joseph Wu]

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

(Updated July 26, 2016, 2:17 p.m.)


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


Changes
---

Rebased and fixed typo.


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


Repository: mesos


Description
---

This builds upon earlier changes to complete `process::finalize`.  
Tests which need to reconfigure libprocess, such as some SSL-related 
tests, can use `process::reinitialize` to do so.  

This method may also be useful for providing additional isolation 
between tests, such as cleaning up all processes after each test case.


Diffs (updated)
-

  3rdparty/libprocess/src/openssl.cpp a73313b3221d6c80b35f21c00f35d9f9c795f1ec 
  3rdparty/libprocess/src/process.cpp 7f331b812de2f0437838f48e0959441c8e04c358 

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


Testing
---

TODO: Define `process::reinitialize` in several tests, such as `HTTPTest`, 
`MetricsTest`, and `ReapTest` and call `process::reinitialize` before and after 
tests.


Thanks,

Joseph Wu

Re: Review Request 50454: Fixed ExamplesTest.DiskFullFramework on non-root Linux builds.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50454]

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

- Mesos ReviewBot


On July 26, 2016, 9 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50454/
> ---
> 
> (Updated July 26, 2016, 9 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Artem Harutyunyan, and Vinod Kone.
> 
> 
> Bugs: MESOS-5907
> https://issues.apache.org/jira/browse/MESOS-5907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The example tests will check if an agent exits after two seconds, under
> the assumption that the agent fails fast if configured incorrectly.  
> On systems with systemd, the two second timeout may be insufficient
> as there is additional logic/validation for initializing systemd.
> 
> The test suite does not need systemd support in the first place, 
> so the agent launched in the DiskFullFramework example test should
> explicitly disable systemd logic.
> 
> 
> Diffs
> -
> 
>   src/tests/disk_full_framework_test.sh 
> 3fc180b3f1eaea91ff3c7d4fa8bf16e7278c55c4 
> 
> Diff: https://reviews.apache.org/r/50454/diff/
> 
> 
> Testing
> ---
> 
> Verified that the test passes on OSX and CentOS 7.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 50460: Added code to build libstate shared library.

[Srinivas Brahmaroutu]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

Builds shared library with zookeeper which libmesosjava.jar
is dependant.


Diffs
-

  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
  src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 
  src/state/CMakeLists.txt PRE-CREATION 

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


Testing
---

cmake .. && make state 

build libstate dynamic library.


Thanks,

Srinivas Brahmaroutu

Review Request 50463: Added code to build libmesosjava shared library.

[Srinivas Brahmaroutu]

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

Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description
---

Build libmesosjava shared library. This library allows Java
frameworks to communicate with mesos.


Diffs
-

  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
  src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 

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


Testing
---

cmake && make mesosjava

Build /src/libmesosjava.* shared libraries.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49819: Implemented `CgroupsIsolatorProcess::prepare`.

[Jie Yu]

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




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


no need for the quote here because containerId is generated by the agent.



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 258 - 261)


I'd move this down right above when we actually need 'user'. I.e.,

```
if (containerConfig.has_user()) {
  Try chown = os::chown(
  containerConfig.user(),
  path::join(hierarchy, cgroup),
  false);
  ...
}
```



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 263 - 264)


I'd rephrase here:
```
We save 'Info' into 'infos' first so that even if 'prepare' fails, we can 
properly cleanup the *side effects* created below.
```



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 271 - 272)


Ditto on no quote for containerid.



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


I'd just inline `createCgroup` here as it's only used here.



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 287 - 288)


Do you need the temp variable?
```
prepares.push_back(subsystem->prepare(containerId));
```



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


I would add a TODO here sayting that here we assume command executor's 
resources include the task's resources. Revisit here if this semantics changes.



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


Move this to the end of the above line.



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


```
return Error(
"Failed to check the existence of cgroup at 'xxx': ");```



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 313 - 314)


Let's be consistent on the indentation for multi-line arguments. I'd prefer 
consistently putting it into the next line:
```
return Error(
"Cgroup at 'xxx' already exists");
```



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


```
"Failed to create the cgroup at 'xxx': "
```



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 322 - 324)


I'd add a TODO here. To me, this is not the best way to achieve the goal. 
For instance, two tasks under the same user can change cgroups setting for each 
other. The right solution should be using cgroups namespaces + user namespaces.



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


```
"Failed to chown the cgroup at `xxx`: "
```


- Jie Yu


On July 26, 2016, 5:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49819/
> ---
> 
> (Updated July 26, 2016, 5:10 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `CgroupsIsolatorProcess::prepare`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 348f105f9c3109a02f1dde0649f1b829cb9ddd04 
> 
> Diff: https://reviews.apache.org/r/49819/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49817: Implemented `CgroupsIsolatorProcess::recover`.

[Jie Yu]

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



'recover' sounds like the most complex one. Can you rebase this patch to the 
end (i.e., after 'cleanup')? It's easier to review in this way.

- Jie Yu


On July 26, 2016, 5:09 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49817/
> ---
> 
> (Updated July 26, 2016, 5:09 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `CgroupsIsolatorProcess::recover`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 348f105f9c3109a02f1dde0649f1b829cb9ddd04 
> 
> Diff: https://reviews.apache.org/r/49817/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50247: Added a abstract base class for scheduler library.

[Vinod Kone]

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




include/mesos/v1/scheduler.hpp (line 44)


Add comments here on what this class represents or useful for?


- Vinod Kone


On July 20, 2016, 7:01 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50247/
> ---
> 
> (Updated July 20, 2016, 7:01 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds an abstract base class `MesosBase` that
> would be used by implementations for interacting with
> Mesos via the Scheduler API. This is needed later for
> implementing the V0->V1 Scheduler Adapter.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp 18e7a95f8342ea155f9e339945b05810b6bd82b5 
> 
> Diff: https://reviews.apache.org/r/50247/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 50248: Added helper functions for v1 JNI `construct()`/`convert()`.

[Vinod Kone]

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




src/java/jni/construct.cpp (line 460)


s/NULL/nullptr/ to be consistent with above?


- Vinod Kone


On July 20, 2016, 7:01 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50248/
> ---
> 
> (Updated July 20, 2016, 7:01 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-5788
> https://issues.apache.org/jira/browse/MESOS-5788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These would be used later in the chain for the making JNI
> calls to v1 Mesos implementation in native code.
> 
> 
> Diffs
> -
> 
>   src/java/jni/construct.cpp e5c070c819698729780b487ebd214ee43f15ae87 
>   src/java/jni/convert.cpp 45ff488b921c0254761ec0a7b5c06608aa86ca3f 
> 
> Diff: https://reviews.apache.org/r/50248/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 50250: Added v1 Scheduler/Mesos interface in Java.

[Vinod Kone]

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




src/Makefile.am (lines 1518 - 1519)


white space.


- Vinod Kone


On July 20, 2016, 7:01 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50250/
> ---
> 
> (Updated July 20, 2016, 7:01 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-5788
> https://issues.apache.org/jira/browse/MESOS-5788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds an interface for the v1 Scheduler + Mesos
> interface that the schedulers can use to connect to Mesos.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
>   src/java/src/org/apache/mesos/v1/scheduler/Mesos.java PRE-CREATION 
>   src/java/src/org/apache/mesos/v1/scheduler/Scheduler.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50250/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 50250: Added v1 Scheduler/Mesos interface in Java.

[Vinod Kone]

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




src/java/src/org/apache/mesos/v1/scheduler/Mesos.java (line 33)


add `reconnect()` as well?


- Vinod Kone


On July 20, 2016, 7:01 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50250/
> ---
> 
> (Updated July 20, 2016, 7:01 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-5788
> https://issues.apache.org/jira/browse/MESOS-5788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds an interface for the v1 Scheduler + Mesos
> interface that the schedulers can use to connect to Mesos.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
>   src/java/src/org/apache/mesos/v1/scheduler/Mesos.java PRE-CREATION 
>   src/java/src/org/apache/mesos/v1/scheduler/Scheduler.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50250/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 50251: Added java implementations for the V0/V1 implementation for Mesos.

[Vinod Kone]

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




src/Makefile.am (lines 1518 - 1521)


not yours, but can you fix this white space?


- Vinod Kone


On July 20, 2016, 7:02 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50251/
> ---
> 
> (Updated July 20, 2016, 7:02 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-5788
> https://issues.apache.org/jira/browse/MESOS-5788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added java implementations for the V0/V1 implementation for Mesos.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
>   src/java/src/org/apache/mesos/v1/scheduler/JNIMesos.java PRE-CREATION 
>   src/java/src/org/apache/mesos/v1/scheduler/V0Mesos.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50251/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 50472: Added MESOS-5388 to 1.0 CHANGELOG.

[Jie Yu]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Added MESOS-5388 to 1.0 CHANGELOG.

This is a trivial fix (one liner) but fixed an important security hole 
(allowing user to rm -rf /).


Diffs
-

  CHANGELOG 54b5f316e5ccc175cc3f471681b02e677499317d 

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


Testing
---


Thanks,

Jie Yu

Re: Review Request 50472: Added MESOS-5388 to 1.0 CHANGELOG.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 26, 2016, 3:37 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50472/
> ---
> 
> (Updated July 26, 2016, 3:37 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-5388 to 1.0 CHANGELOG.
> 
> This is a trivial fix (one liner) but fixed an important security hole 
> (allowing user to rm -rf /).
> 
> 
> Diffs
> -
> 
>   CHANGELOG 54b5f316e5ccc175cc3f471681b02e677499317d 
> 
> Diff: https://reviews.apache.org/r/50472/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 50473: Fix numify() to handle negative numbers consistently (MESOS-4070).

[Yong Tang]

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

Review request for mesos, Greg Mann, Jie Yu, Neil Conway, and Cong Wang.


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


Repository: mesos


Description
---

This fix updated the implementation of numify() so that negative numbers
could be handled conssitently for hex and non-hex numbers.


Diffs
-

  3rdparty/stout/include/stout/numify.hpp 
26a637bec1193dd51437bd689c34fbe6d1935d89 
  3rdparty/stout/tests/numify_tests.cpp 
444377df00922df12d4b3ed25b4cfe9071cff5c3 

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


Testing
---

make check (Ubuntu 14.04)

NOTE: This RR is from #45011.


Thanks,

Yong Tang

Re: Review Request 50252: Added native implementation for v1 Mesos interface.

[Vinod Kone]

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




src/java/jni/org_apache_mesos_v1_scheduler_JNIMesos.cpp (line 279)


add a null check and reference a JIRA.


- Vinod Kone


On July 20, 2016, 7:02 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50252/
> ---
> 
> (Updated July 20, 2016, 7:02 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-5788
> https://issues.apache.org/jira/browse/MESOS-5788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the native C++ implementation for the v1
> Java class `JNIMesos` used for interacting with Mesos.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
>   src/java/jni/org_apache_mesos_v1_scheduler_JNIMesos.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50252/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 43569: Updated log message if container not found.

[Guangya Liu]

> On 七月 26, 2016, 8:29 p.m., Jie Yu wrote:
> > src/slave/containerizer/docker.cpp, line 379
> > 
> >
> > We put `+` in the end

Thanks Yu Jie, I saw that the `containerId` was already printted here 
https://github.com/apache/mesos/blob/master/src/slave/slave.cpp#L4029-L4033 , 
so seems I do not add `containerId` for such case. Will update this patch to 
only include `containerId` for non `return Failure()` cases here.


- Guangya


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


On 三月 23, 2016, 1:50 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43569/
> ---
> 
> (Updated 三月 23, 2016, 1:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated log message if container not found.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ee7a265975323ca891114a286357c8e42901560c 
> 
> Diff: https://reviews.apache.org/r/43569/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45011: Fix numify() to handle negative numbers consistently (MESOS-4070).

[Yong Tang]

> On July 26, 2016, 8:30 p.m., Greg Mann wrote:
> > Hi Yong,
> > Do you still have interest in getting this committed? If so, it looks like 
> > it needs to be rebased.

Thanks Greg. I tried to rebase but my local copy has been lost. Don't know how 
to recover so I just created a new review request:
https://reviews.apache.org/r/50473/

Please take a look and let me know if there are any issues.


- Yong


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


On March 18, 2016, 10:49 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45011/
> ---
> 
> (Updated March 18, 2016, 10:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Neil Conway, and Cong Wang.
> 
> 
> Bugs: MESOS-4070
> https://issues.apache.org/jira/browse/MESOS-4070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix updated the implementation of numify() so that negative numbers
> could be handled conssitently for hex and non-hex numbers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> 26a637bec1193dd51437bd689c34fbe6d1935d89 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp 
> 444377df00922df12d4b3ed25b4cfe9071cff5c3 
> 
> Diff: https://reviews.apache.org/r/45011/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45011: Fix numify() to handle negative numbers consistently (MESOS-4070).

[Greg Mann]

> On July 26, 2016, 8:30 p.m., Greg Mann wrote:
> > Hi Yong,
> > Do you still have interest in getting this committed? If so, it looks like 
> > it needs to be rebased.
> 
> Yong Tang wrote:
> Thanks Greg. I tried to rebase but my local copy has been lost. Don't 
> know how to recover so I just created a new review request:
> https://reviews.apache.org/r/50473/
> 
> Please take a look and let me know if there are any issues.

Awesome, thanks Yong!!


- Greg


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


On March 18, 2016, 10:49 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45011/
> ---
> 
> (Updated March 18, 2016, 10:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Neil Conway, and Cong Wang.
> 
> 
> Bugs: MESOS-4070
> https://issues.apache.org/jira/browse/MESOS-4070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix updated the implementation of numify() so that negative numbers
> could be handled conssitently for hex and non-hex numbers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 
> 26a637bec1193dd51437bd689c34fbe6d1935d89 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp 
> 444377df00922df12d4b3ed25b4cfe9071cff5c3 
> 
> Diff: https://reviews.apache.org/r/45011/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 43558: Speed up ExamplesTest.PersistentVolumeFramework.

[Greg Mann]

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


Fix it, then Ship it!





src/tests/persistent_volume_framework_test.sh (lines 28 - 29)


It looks like this has already been removed from this file - could you 
update the diff?


- Greg Mann


On April 10, 2016, 10:57 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43558/
> ---
> 
> (Updated April 10, 2016, 10:57 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Jie Yu.
> 
> 
> Bugs: MESOS-4663
> https://issues.apache.org/jira/browse/MESOS-4663
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Speed up ExamplesTest.PersistentVolumeFramework.
> 
> 
> Diffs
> -
> 
>   src/examples/persistent_volume_framework.cpp 
> b4faa0ee25dc3a72c17ef2b0640a3695423ef79a 
>   src/tests/persistent_volume_framework_test.sh 
> 074cdcbc4a738170e84887c1773cd7c118112d58 
> 
> Diff: https://reviews.apache.org/r/43558/diff/
> 
> 
> Testing
> ---
> 
> Repeat test in CentOS 7.1
> ```
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="ExamplesTest.PersistentVolumeFramework" --gtest_repeat=200 
> --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 41787: Speed up SlaveRecoveryTest.*.

[Greg Mann]

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




src/tests/slave_recovery_tests.cpp (lines 419 - 428)


I would prefer to place `Clock::pause()` at the very beginning of the test, 
with `Clock::resume()` placed at the very end of the test. I think this is a 
little easier for future authors to reason about - we can keep the clock paused 
for the entire test, rather than pausing it for only a portion.

And the same for the other occurrences below. What do you think?


- Greg Mann


On April 11, 2016, 1:33 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41787/
> ---
> 
> (Updated April 11, 2016, 1:33 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Timothy Chen.
> 
> 
> Bugs: MESOS-4158
> https://issues.apache.org/jira/browse/MESOS-4158
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests are sped up by using clock,
> while some relies on the fix of MESOS-4111 and a comment is put.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 79132344be3bcd2bda54357cd5e7e0c59a766fd8 
> 
> Diff: https://reviews.apache.org/r/41787/diff/
> 
> 
> Testing
> ---
> 
> SlaveRecoveryTest/0.RecoverStatusUpdateManager (335 ms total)
> SlaveRecoveryTest/0.ReconnectExecutor (333 ms total)
> SlaveRecoveryTest/0.ReconcileKillTask (443 ms total)
> SlaveRecoveryTest/0.ReconcileShutdownFramework (1379 ms total)
> SlaveRecoveryTest/0.CleanupExecutor (474 ms total)
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 50323: Added build script for mesos-local executable.

[Srinivas Brahmaroutu]

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

(Updated July 26, 2016, 11:42 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description (updated)
---

The executable will help to run a local mesos cluster.


Diffs (updated)
-

  src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 
  src/local/CMakeLists.txt PRE-CREATION 

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


Testing
---

cmake .. && make mesos-local


Thanks,

Srinivas Brahmaroutu

Re: Review Request 50324: Added script to build mesos-execute.

[Srinivas Brahmaroutu]

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

(Updated July 26, 2016, 11:43 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description (updated)
---

The mesos-execute executable helps in running a task 
in mesos cluster.


Diffs (updated)
-

  src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 
  src/cli/CMakeLists.txt PRE-CREATION 

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


Testing
---

cmake .. && make mesos-execute


Thanks,

Srinivas Brahmaroutu

Re: Review Request 50253: Added native implementation for the V0 Mesos Adapter.

[Vinod Kone]

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




src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 64)


make it explicit that this is v0::Scheduler and v1::MesosBase.



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 76)


// v0 scheduler interface methods.



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 119)


// v1 MesosBase methods.



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 172)


move these out of the class below public method definitions.



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 175)


s/executor/scheduler/



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 430)


add a comment to see send() for more details.



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 463)


set `subscribeCall` to false?

more importantly we need to drain the queue, with a big comment on why we 
do it and why it is safe.



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 600)


instead of CHECK here and everywhere else in this functio, do a validation 
and drop instead?



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 613)


i think you can always call this?



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 623)


log a warning/error?



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 632)


again, is this necessary?



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 643)


log warning/error?



src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp (line 660)


log a warning/error.


- Vinod Kone


On July 20, 2016, 7:02 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50253/
> ---
> 
> (Updated July 20, 2016, 7:02 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-5788
> https://issues.apache.org/jira/browse/MESOS-5788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the C++ implementation for the JAVA V0 to V1 Mesos
> implementation adapter (driver + scheduler).
> 
> 
> Diffs
> -
> 
>   src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
>   src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50253/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 50325: Added script to build mesos-log tool.

[Srinivas Brahmaroutu]

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

(Updated July 26, 2016, 11:45 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description (updated)
---

The mesos-log executable allows to operate logs 
and helps setup log replica.


Diffs (updated)
-

  src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 
  src/log/CMakeLists.txt PRE-CREATION 

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


Testing
---

cmake .. && make mesos-log


Thanks,

Srinivas Brahmaroutu

Re: Review Request 50326: Added script to build mesos-resolve.

[Srinivas Brahmaroutu]

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

(Updated July 26, 2016, 11:47 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description (updated)
---

The executabl mesos-resolve help to resolve mesos 
master in the cluster.


Diffs (updated)
-

  src/cli/CMakeLists.txt PRE-CREATION 

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


Testing
---

cmake .. && make mesos-resolve


Thanks,

Srinivas Brahmaroutu

Re: Review Request 50327: Added scripts to build sample framework executables.

[Srinivas Brahmaroutu]

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

(Updated July 26, 2016, 11:48 p.m.)


Review request for mesos, Alex Clemmer and Joseph Wu.


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


Repository: mesos


Description (updated)
---

These test Framworks and Executors help in running various
scenarios on how jobs are handles in mesos clusters.


Diffs (updated)
-

  CMakeLists.txt ee8f565586b958929bc4e95b09827244d6d4155f 
  cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
  src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 
  src/examples/CMakeLists.txt PRE-CREATION 
  src/examples/cmake/ExamplesConfigure.cmake PRE-CREATION 

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


Testing
---

cmake .. && make dynamic-reservation-framework test-http-framework 
test-framework test-executor test-http-executor long-lived-framework 
long-lived-executor no-executor-framework docker-no-executor-framework 
balloon-framework balloon-executor load-generator-framework 
persistent-volume-framework


Thanks,

Srinivas Brahmaroutu

Re: Review Request 50254: Added an example test for the V0/V1 Mesos java implementation.

[Vinod Kone]

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




src/Makefile.am (line 1535)


whitespace.



src/Makefile.am (line 2235)


whitespace.



src/examples/java/V1TestFramework.java (line 134)


can you add a hearbeat timer and do a `reconnect()` if it fires?



src/examples/java/V1TestFramework.java (line 334)


would be nice to include the version in the `name` as well.



src/examples/java/V1TestFramework.java (line 349)


pass args[2] as well if it's supplied to configure number of tasks.


- Vinod Kone


On July 20, 2016, 7:03 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50254/
> ---
> 
> (Updated July 20, 2016, 7:03 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-5788
> https://issues.apache.org/jira/browse/MESOS-5788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an example test for the V0/V1 Mesos java implementation.
> 
> 
> Diffs
> -
> 
>   configure.ac d2136909b7305498ae901a5ea00133142b77f9e6 
>   src/Makefile.am cc83525a4455bbb0e654d346921d66ed2436411d 
>   src/examples/java/V1TestFramework.java PRE-CREATION 
>   src/examples/java/v1-test-framework.in PRE-CREATION 
>   src/tests/examples_tests.cpp ac513ce9aa3c8f366fe81ba937e3dc0d51a26940 
>   src/tests/java_v0_framework_test.sh PRE-CREATION 
>   src/tests/java_v1_framework_test.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50254/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 50463: Added code to build libmesosjava shared library.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [50463, 50460, 50459, 50458, 50457, 50456, 50455, 50415, 50414]

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

Error:
2016-07-26 23:52:09 URL:https://reviews.apache.org/r/50456/diff/raw/ 
[3839/3839] -> "50456.patch" [1]
error: patch failed: cmake/MesosConfigure.cmake:127
error: cmake/MesosConfigure.cmake: patch does not apply

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

- Mesos ReviewBot


On July 26, 2016, 9:26 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50463/
> ---
> 
> (Updated July 26, 2016, 9:26 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5902
> https://issues.apache.org/jira/browse/MESOS-5902
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Build libmesosjava shared library. This library allows Java
> frameworks to communicate with mesos.
> 
> 
> Diffs
> -
> 
>   cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
>   src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 
> 
> Diff: https://reviews.apache.org/r/50463/diff/
> 
> 
> Testing
> ---
> 
> cmake && make mesosjava
> 
> Build /src/libmesosjava.* shared libraries.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 50472: Added MESOS-5388 to 1.0 CHANGELOG.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On July 26, 2016, 10:37 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50472/
> ---
> 
> (Updated July 26, 2016, 10:37 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MESOS-5388 to 1.0 CHANGELOG.
> 
> This is a trivial fix (one liner) but fixed an important security hole 
> (allowing user to rm -rf /).
> 
> 
> Diffs
> -
> 
>   CHANGELOG 54b5f316e5ccc175cc3f471681b02e677499317d 
> 
> Diff: https://reviews.apache.org/r/50472/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 50477: Fixed use-after-close bug when using libevent and SSL.

[Benjamin Mahler]

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

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


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


Repository: mesos


Description
---

The deferred call to SSL_shutdown within ~LibeventSSLSocketImpl
can occur after the socket fd has been closed by Socket::~Impl.

This can lead to a TLS Alert message being sent on any fd if
it the fd is re-used between the close and the SSL_shutdown!

Thanks to Jan-Philip Gehrcke for reporting the issue.


Diffs
-

  3rdparty/libprocess/include/process/socket.hpp 
881b44b987e5894cac838dae046ab7dbad20b000 
  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
4d376d8b7c1b29105de69bed2e4077f8c94fed0b 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
f4c0b0b97960400b0282837979bf0ed17f56a068 

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


Testing
---

make check

Ran my repro steps (issue HTTP requests while hammering the master with HTTPS 
requests). Issue disappears after this fix.


Thanks,

Benjamin Mahler

Re: Review Request 49851: Implemented `MemorySubsystem`.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp (lines 71 - 75)


I think this should not be a virtual method, and it may be better to put it 
after the `cleanup()` method.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 81)


This comment needs to be updated since now the parameter is 
`_notifyCallback` rather than `_isolator`.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 83)


Does it have to be a virtual method?



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 174)


s/that uses/that is used/



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 391)


I think here you should use `limitSwap` rather than 
`flags.cgroups_limit_swap`.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 494)


How do we recover this field `updatedLimit`? I mean during agent recovery, 
`updatedLimit` of each Info will be reset to `false`, right? But I think that 
is not correct.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 683)


I think you also need to do `infos.erase(containerId);` here.


- Qian Zhang


On July 27, 2016, 1:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 27, 2016, 1:13 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5045
> https://issues.apache.org/jira/browse/MESOS-5045
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `MemorySubsystem`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 348f105f9c3109a02f1dde0649f1b829cb9ddd04 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50123: Added GPU scheduler for docker containerizer.

[Guangya Liu]

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



Some early comments.


src/slave/containerizer/docker.hpp (line 500)


We generally don't rely on transitive includes, because it's easier to 
maintain (quickly scan a file and add includes for all symbols present). It is 
better adding `#include` in this header file.

BTW: I saw that this header file is also missing some other include files 
after a scan:
```
#include
#include 
#include 
```

It would be great if you can do a clean up here as well.



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


1) move this right before line 66
2) include set in the header.

```
#include 
```



src/slave/containerizer/docker.cpp (lines 170 - 185)


The `else` is not needed here.

```
if (nvidia.isSome()) {
  return new DockerContainerizer(
  flags,
  fetcher,
  Owned(logger.get()),
  docker,
  nvidia.get().allocator);
}

return new DockerContainerizer(
flags,
fetcher,
Owned(logger.get()),
docker,
None());
```

Also the right format of `if-else` should be:
```
if() {
  ;
} else {
  ;
}
```

No need to add new line for `else {`.



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


I prefer that moving this before the use of this variable, i.e. putting it 
right before 1326



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


s/.get()/->



src/slave/containerizer/docker.cpp (lines 1313 - 1318)


What about the following format plus some comments here:

```
const Resources& resources = taskInfo->resources();

Option gpus = resources.gpus();

// Make sure that the `gpus` resource is not fractional.
// We rely on scalar resources only having 3 digits of precision.
if (static_cast(gpus.getOrElse(0.0) * 1000.0) % 1000 != 0) {
  return Failure("The 'gpus' resource must be an unsigned integer");
}

size_t requested = static_cast(resources.gpus().getOrElse(0.0));

```



src/slave/containerizer/docker.cpp (lines 1325 - 1358)


How about simplify the logic as this:

```
Option nvidiaGpus = None();

if (requested > 0) {

}
```



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


1) s/allocator.get().allocate(requested)/allocator->allocate(requested)

2) I think it is not right to use `Option` here as the 
`allocator->allocate(requested)` is returning a `Future`.

One proposal is as following:
```
allocator->allocate(requested)
  .then(defer(...));
```

You can also refer to 
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/gpu/isolator.cpp#L356
 to get some detail.



src/slave/containerizer/docker.cpp (lines 1335 - 1336)


```
return Failure("Not enough GPUs available to reserve " +
   stringify(requested) + " GPUs");

```



src/slave/containerizer/docker.cpp (lines 1337 - 1338)


new line here



src/slave/containerizer/docker.cpp (lines 1341 - 1344)


s/string/const string



src/slave/containerizer/docker.cpp (lines 1344 - 1345)


new line here and move this right before `argv.push_back`



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


s/string/const string



src/slave/containerizer/docker.cpp (lines 1353 - 1354)


new line here



src/slave/containerizer/docker.cpp (lines 1356 - 1357)


new line here



src/slave/containerizer/docker.cpp (lines 1550 - 1561)


What about update the format as following:

```
// Release GPUs after the task exit.
set deallocated;
if (!containers_[containerId]->gpuAllocated.empty()) {
  foreach (const Gpu& &pu, containers_[containerId]->gpuAllocated) {
deallocated.insert(gpu);
  }
  
  containers_[containerId]->gpuAllocated.clear();
  
  if 

Re: Review Request 50327: Added scripts to build sample framework executables.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50064, 50179, 50323, 50324, 50325, 50326, 50327]

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

- Mesos ReviewBot


On July 26, 2016, 11:48 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50327/
> ---
> 
> (Updated July 26, 2016, 11:48 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These test Framworks and Executors help in running various
> scenarios on how jobs are handles in mesos clusters.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt ee8f565586b958929bc4e95b09827244d6d4155f 
>   cmake/MesosConfigure.cmake b2318ed8eb4e11de43abfc15f51d12b2c0ff8fa1 
>   src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 
>   src/examples/CMakeLists.txt PRE-CREATION 
>   src/examples/cmake/ExamplesConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50327/diff/
> 
> 
> Testing
> ---
> 
> cmake .. && make dynamic-reservation-framework test-http-framework 
> test-framework test-executor test-http-executor long-lived-framework 
> long-lived-executor no-executor-framework docker-no-executor-framework 
> balloon-framework balloon-executor load-generator-framework 
> persistent-volume-framework
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Review Request 50482: Fixed the CORS error when redirect in webui.

[haosdent huang]

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

Review request for mesos, Adam B, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

Fixed the CORS error when redirect in webui.


Diffs
-

  src/master/http.cpp 1cbec976ccedae81def388c62889624aaf9a794e 
  src/webui/master/static/js/controllers.js 
ceaf1402ebdae4efd1a8a6fc9c7b795de69e2fc0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50482: Fixed the CORS error when redirect in webui.

[haosdent huang]

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

(Updated July 27, 2016, 3:41 a.m.)


Review request for mesos, Adam B, Jie Yu, Vinod Kone, and Jiang Yan Xu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Fixed the CORS error when redirect in webui.


Diffs (updated)
-

  src/master/http.cpp 1cbec976ccedae81def388c62889624aaf9a794e 
  src/webui/master/static/js/controllers.js 
ceaf1402ebdae4efd1a8a6fc9c7b795de69e2fc0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50215: Updated pre exec commands as non-shell in docker volume isolator.

[James DeFelice]

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




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


what stops a user from injecting a '; in the source and then following it 
up with an arbitrary command?


- James DeFelice


On July 26, 2016, 9:08 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50215/
> ---
> 
> (Updated July 26, 2016, 9:08 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5388
> https://issues.apache.org/jira/browse/MESOS-5388
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By adding apostrophes to mount 'source' and 'target', arbitraty
> commands defined by users postfixed to 'container_path' will
> take no effect. 'mount' command will return an error for invalid
> mount 'target'.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 96806a75e6f7abc3a229c01b375fdba30d267ab4 
> 
> Diff: https://reviews.apache.org/r/50215/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 50477: Fixed use-after-close bug when using libevent and SSL.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50477]

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

- Mesos ReviewBot


On July 27, 2016, 2:03 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50477/
> ---
> 
> (Updated July 27, 2016, 2:03 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-5913
> https://issues.apache.org/jira/browse/MESOS-5913
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The deferred call to SSL_shutdown within ~LibeventSSLSocketImpl
> can occur after the socket fd has been closed by Socket::~Impl.
> 
> This can lead to a TLS Alert message being sent on any fd if
> it the fd is re-used between the close and the SSL_shutdown!
> 
> Thanks to Jan-Philip Gehrcke for reporting the issue.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> 881b44b987e5894cac838dae046ab7dbad20b000 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> 4d376d8b7c1b29105de69bed2e4077f8c94fed0b 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> f4c0b0b97960400b0282837979bf0ed17f56a068 
> 
> Diff: https://reviews.apache.org/r/50477/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Ran my repro steps (issue HTTP requests while hammering the master with HTTPS 
> requests). Issue disappears after this fix.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 43569: Updated log message if container not found.

[Guangya Liu]

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

(Updated 七月 27, 2016, 5:45 a.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

Updated log message if container not found.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 12bad2db03bcf755317c654f028b628c5c407a62 
  src/slave/containerizer/mesos/containerizer.cpp 
86a8d8a85b6a33c87798108cb65af85bb9bbbc77 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 50482: Fixed the CORS error when redirect in webui.

[Jiang Yan Xu]

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



LGTM and verified that the web UI issue was solved.


src/master/http.cpp (line 2038)


Is `request.url.path == stringify(request.url)` guaranteed? Even if it is, 
would `request.url.path` be more explicit?



src/webui/master/static/js/controllers.js (lines 375 - 376)


Fix indentation.


- Jiang Yan Xu


On July 26, 2016, 8:41 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50482/
> ---
> 
> (Updated July 26, 2016, 8:41 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5911
> https://issues.apache.org/jira/browse/MESOS-5911
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the CORS error when redirect in webui.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 1cbec976ccedae81def388c62889624aaf9a794e 
>   src/webui/master/static/js/controllers.js 
> ceaf1402ebdae4efd1a8a6fc9c7b795de69e2fc0 
> 
> Diff: https://reviews.apache.org/r/50482/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>