Re: Review Request 33746: Improve logging of clone flags for linux_launcher.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [33746]

All tests passed.

- Mesos ReviewBot


On May 1, 2015, 5:27 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33746/
> ---
> 
> (Updated May 1, 2015, 5:27 a.m.)
> 
> 
> Review request for mesos, Chi Zhang and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve logging of clone flags for linux_launcher.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 87ff82c2efdf26f38b81f8440640bf04def05931 
>   src/slave/containerizer/linux_launcher.cpp 
> b9e22e3c70bed0c29e2ca8632411789d33f779a8 
> 
> Diff: https://reviews.apache.org/r/33746/diff/
> 
> 
> Testing
> ---
> 
> Example output:
> 
> I0501 05:13:25.414758  7838 linux_launcher.cpp:212] Cloning child process 
> with flags = CLONE_NEWNS | CLONE_NEWPID
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 33746: Improve logging of clone flags for linux_launcher.

[Chi Zhang]

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

Ship it!


Ship It!


src/linux/ns.hpp


add some comment for this function?


- Chi Zhang


On May 1, 2015, 5:27 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33746/
> ---
> 
> (Updated May 1, 2015, 5:27 a.m.)
> 
> 
> Review request for mesos, Chi Zhang and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve logging of clone flags for linux_launcher.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 87ff82c2efdf26f38b81f8440640bf04def05931 
>   src/slave/containerizer/linux_launcher.cpp 
> b9e22e3c70bed0c29e2ca8632411789d33f779a8 
> 
> Diff: https://reviews.apache.org/r/33746/diff/
> 
> 
> Testing
> ---
> 
> Example output:
> 
> I0501 05:13:25.414758  7838 linux_launcher.cpp:212] Cloning child process 
> with flags = CLONE_NEWNS | CLONE_NEWPID
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Review Request 33746: Improve logging of clone flags for linux_launcher.

[Ian Downes]

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

Review request for mesos, Chi Zhang and Jie Yu.


Repository: mesos


Description
---

Improve logging of clone flags for linux_launcher.


Diffs
-

  src/linux/ns.hpp 87ff82c2efdf26f38b81f8440640bf04def05931 
  src/slave/containerizer/linux_launcher.cpp 
b9e22e3c70bed0c29e2ca8632411789d33f779a8 

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


Testing
---

Example output:

I0501 05:13:25.414758  7838 linux_launcher.cpp:212] Cloning child process with 
flags = CLONE_NEWNS | CLONE_NEWPID


Thanks,

Ian Downes

Re: Review Request 30774: Fetcher Cache

[Bernd Mathiske]

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

(Updated April 30, 2015, 7:40 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy 
Chen.


Changes
---

Fixed most issues, but not all. Let's talk about theremaining ones before 
proceeding!


Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and MESOS-2074
https://issues.apache.org/jira/browse/MESOS-2057
https://issues.apache.org/jira/browse/MESOS-2069
https://issues.apache.org/jira/browse/MESOS-2070
https://issues.apache.org/jira/browse/MESOS-2072
https://issues.apache.org/jira/browse/MESOS-2073
https://issues.apache.org/jira/browse/MESOS-2074


Repository: mesos


Description
---

Almost all of the functionality in epic MESOS-336. Downloaded files from 
CommandInfo::URIs can now be cached in a cache directory designated by a slave 
flag. This only happens when asked for by an extra flag in the URI and is thus 
backwards-compatible. The cache has a size limit also given by a new slave 
flag. Cache-resident files are evicted as necessary to make space for newly 
fetched ones. Concurrent attempts to cache the same URI leads to only one 
download. The fetcher program remains external for safety reasons, but is now 
augmented with more elaborate parameters packed into a JSON object to implement 
specific fetch actions for all of the above. Additional testing includes 
fetching from (mock) HDFS and coverage of the new features.


Diffs (updated)
-

  docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b 
  docs/fetcher-cache-internals.md PRE-CREATION 
  docs/fetcher.md PRE-CREATION 
  include/mesos/fetcher/fetcher.proto 311af9aebc6a85dadba9dbeffcf7036b70896bcc 
  include/mesos/mesos.proto 967b1e3bbfb3f6b71d5a15d02cba7ed5ec21816f 
  include/mesos/type_utils.hpp 044637481e5405d4d6f61653a9f9386edd191deb 
  src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
  src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b 
  src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
  src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec 
  src/slave/containerizer/docker.hpp b25ec55bf3cd30d6e8a804d09d90c632a7d12e3f 
  src/slave/containerizer/docker.cpp f9fc89ad7e3c853c3f9f6dcf9aa68e54dc1888c6 
  src/slave/containerizer/fetcher.hpp 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
  src/slave/containerizer/fetcher.cpp 9e9e9d0eb6b0801d53dec3baea32a4cd4acdd5e2 
  src/slave/containerizer/mesos/containerizer.hpp 
5e5f13ed8a71ff9510b40b6032d00fd16d312622 
  src/slave/containerizer/mesos/containerizer.cpp 
f2587280dc0e1d566d2b856a80358c7b3896c603 
  src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
  src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
  src/slave/slave.cpp c78ee3c9e7fc38ad364e83f4abe267e86bfbbc13 
  src/tests/docker_containerizer_tests.cpp 
c9d66b3fbc7d081f36c26781573dca50de823c44 
  src/tests/fetcher_cache_tests.cpp PRE-CREATION 
  src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
  src/tests/mesos.hpp 19db71217f0a3f1ab17a6fd4408f8251410d731d 
  src/tests/mesos.cpp bc082e8d91deb2c5dd64bbc3f0a8a50fa7d19264 

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


Testing
---

make check

--- longer Description: ---

-Replaces all other reviews for the fetcher cache except those related to 
stout: 30006, 30033, 30034, 30036, 30037, 30039, 30124, 30173, 30614, 30616, 
30618, 30621, 30626. See descriptions of those. In dependency order:

30033: Removes the fetcher env tests since these won't be needed any more when 
the fetcher uses JSON in a single env var as a parameter. They never tested 
anything that won't be covered by other tests anyway.

30034: Makes the code structure of all fetcher tests the same. Instead of 
calling the run method of the fetcher directly, calling through fetch(). Also 
removes all uses of I/O redirection, which is not really needed for debugging, 
and thus the next patch can refactor fetch() and run(). (The latter comes in 
two varieties, which complicates matters without much benefit.)

30036: Extends the CommandInfo::URI protobuf with a boolean "caching" field 
that will later cause fetcher cache actions. Also introduces the notion of a 
cache directory to the fetcher info protobuf. And then propagates these 
additions throughout the rest of the code base where applicable. This includes 
passing the slave ID all the way down to the place where the cache dir name is 
constructed.

30037: Extends the fetcher info protobuf with "actions" (fetch directly 
bypassing the cache, fetch through the cache, retrieve from the cache). 
Switches the basis for dealing with uris to "items", which contain the uri, the 
action, and potentially a cache file name. Refactors fetch() and run(), so 
there is only one of each. Introduces ab

Re: Review Request 30774: Fetcher Cache

[Bernd Mathiske]

> On April 29, 2015, 3:41 p.m., Benjamin Hindman wrote:
> > src/slave/containerizer/fetcher.cpp, line 386
> > 
> >
> > Kill the const &.

Killed the &. Any reason this should not be const?


> On April 29, 2015, 3:41 p.m., Benjamin Hindman wrote:
> > src/slave/containerizer/fetcher.cpp, line 597
> > 
> >
> > Let's add some helper functions on Fetcher::Cache so that we can just 
> > get this information directly in the tests rather than this "helper" 
> > function.
> > 
> > // Return the cache.
> > Try> FetcherProcess::cacheFiles();
> > 
> > // Returns the number of entries in the cache.
> > size_t FetcherProcess::cacheSize();

Can we please postpone this until after refactoring fetcher injection into 
slave/containerizer? It will be much easier to make these member functions 
then. I'll put a TODO for now.


- Bernd


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


On April 29, 2015, 1:42 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30774/
> ---
> 
> (Updated April 29, 2015, 1:42 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and 
> MESOS-2074
> https://issues.apache.org/jira/browse/MESOS-2057
> https://issues.apache.org/jira/browse/MESOS-2069
> https://issues.apache.org/jira/browse/MESOS-2070
> https://issues.apache.org/jira/browse/MESOS-2072
> https://issues.apache.org/jira/browse/MESOS-2073
> https://issues.apache.org/jira/browse/MESOS-2074
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Almost all of the functionality in epic MESOS-336. Downloaded files from 
> CommandInfo::URIs can now be cached in a cache directory designated by a 
> slave flag. This only happens when asked for by an extra flag in the URI and 
> is thus backwards-compatible. The cache has a size limit also given by a new 
> slave flag. Cache-resident files are evicted as necessary to make space for 
> newly fetched ones. Concurrent attempts to cache the same URI leads to only 
> one download. The fetcher program remains external for safety reasons, but is 
> now augmented with more elaborate parameters packed into a JSON object to 
> implement specific fetch actions for all of the above. Additional testing 
> includes fetching from (mock) HDFS and coverage of the new features.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b 
>   docs/fetcher-cache-internals.md PRE-CREATION 
>   docs/fetcher.md PRE-CREATION 
>   include/mesos/fetcher/fetcher.proto 
> 311af9aebc6a85dadba9dbeffcf7036b70896bcc 
>   include/mesos/mesos.proto 967b1e3bbfb3f6b71d5a15d02cba7ed5ec21816f 
>   include/mesos/type_utils.hpp 044637481e5405d4d6f61653a9f9386edd191deb 
>   src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
>   src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b 
>   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
>   src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec 
>   src/slave/containerizer/docker.hpp b25ec55bf3cd30d6e8a804d09d90c632a7d12e3f 
>   src/slave/containerizer/docker.cpp f9fc89ad7e3c853c3f9f6dcf9aa68e54dc1888c6 
>   src/slave/containerizer/fetcher.hpp 
> 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 
> 9e9e9d0eb6b0801d53dec3baea32a4cd4acdd5e2 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 5e5f13ed8a71ff9510b40b6032d00fd16d312622 
>   src/slave/containerizer/mesos/containerizer.cpp 
> f2587280dc0e1d566d2b856a80358c7b3896c603 
>   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
>   src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
>   src/slave/slave.cpp c78ee3c9e7fc38ad364e83f4abe267e86bfbbc13 
>   src/tests/docker_containerizer_tests.cpp 
> c9d66b3fbc7d081f36c26781573dca50de823c44 
>   src/tests/fetcher_cache_tests.cpp PRE-CREATION 
>   src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
>   src/tests/mesos.hpp 19db71217f0a3f1ab17a6fd4408f8251410d731d 
>   src/tests/mesos.cpp bc082e8d91deb2c5dd64bbc3f0a8a50fa7d19264 
> 
> Diff: https://reviews.apache.org/r/30774/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> --- longer Description: ---
> 
> -Replaces all other reviews for the fetcher cache except those related to 
> stout: 30006, 30033, 30034, 30036, 30037, 30039, 30124, 30173, 30614, 30616, 
> 30618, 30621, 30626. See descriptions of tho

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

[Jay Buffington]

> On April 21, 2015, 4:25 p.m., Jie Yu wrote:
> > src/slave/slave.cpp, lines 3065-3078
> > 
> >
> > Instead of doing that in your way, can we just try to make sure 
> > `containerizer->wait` here will return a failure (or a Termination with 
> > some reason) when `containerizer->launch` fails. In that way, the 
> > `executorTerminated` will properly send status updates to the slave 
> > (TASK_LOST/TASK_FAILED).
> > 
> > Or am I missing something?
> 
> Jie Yu wrote:
> OK, I think I got confused by the ticket. There are actually two problems 
> here. The problem I am refering to is the fact that we don't send status 
> update to the scheduler if containerizer launch fails until executor 
> reregistration timeout happens. Since for docker containerizer, someone might 
> use a very large timeout value, ideally, the slave should send a status 
> update to the scheduler right after containerizer launch fails.
> 
> After chat with Jay, the problem you guys are refering to is the fact 
> that the scheduler cannot disinguish between the case where the task has 
> failed vs. the case where the configuration of a task is not correct, because 
> in both cases, the scheduler will receive a TASK_FAILED/TASK_LOST.
> 
> Jie Yu wrote:
> To address the first problem, I think the simplest way is to add a 
> containerizer->destroy(..) in executorLaunched when containerizer->launch 
> fails. In that way, it's going to trigger containerizer->wait and thus send 
> status update to the scheduler.
> 
> Jie Yu wrote:
> Regarding the second problem, IMO, we should include a reason field in 
> Termination (https://issues.apache.org/jira/browse/MESOS-2035) and let 
> sendExecutorTerminatedStatusUpdate to propagate the termination reason to the 
> scheduler.
> 
> Timothy Chen wrote:
> Reason field sounds good, I think what you proposed makes sense, in 
> docker containerizer at least we also need to make sure termination message 
> is set correctly as currently it doesn't contain all the error information 
> that we pass back to the launch future.
> 
> Jay Buffington wrote:
> Sorry for the confusion.   There are three problems that are all related. 
>  Yes, we need to send statusUpdates as soon as containerizer launch fails and 
> yes, we need to set the reason so the scheduler can distinguish a slave 
> failure from a bad request.  However, my intention with this patch is not to 
> address either of those two problems.
> 
> My goal with this patch is to simply send the containerizer launch 
> failure message back to the scheduler.  I am using Aurora with the docker 
> containerizer.  There are a myriad of reasons that dockerd could fail to 
> "docker run" a container.  Currently, when that fails the user sees a useless 
> and incorrect "Abnormal Executor Termination" message in the Aurora web ui.  
> With this patch they see the stderr output of "docker run."  This way they 
> can report meaningful error messages to the operations team.
> 
> I can update this patch to address the other two issues, but the key is 
> that when the containerizer launch fails we have to send a statusUpdate with 
> a message that contains future.failure().  Before this patch we were only 
> logging it.  The scheduler needs to get that error message.
> 
> Jie Yu wrote:
> Thanks for clarifying it, Jay! In fact, my proposal above should be able 
> to solve the third problem cleanly. Check out 
> `Slave::sendExecutorTerminatedStatusUpdate`. The containerizer should 
> properly set the message and reason fields in the Termination protobuf 
> (basically why the container gets terminated and what's the error message). 
> The slave will forward the reason and message to the scheduler through status 
> update.
> 
> I chatted with BenM about this yesterday, and there are a couple of notes 
> I want to write down here.
> 
> 1. We probably need multiple levels for TaskStatus::Reason. In other 
> words, we probably need a "repeated Reason reasons" field in status update 
> message. For instance, for a containerizer launch failure, we probably need 
> two reasons: 1) the top level reason REASON_EXECUTOR_TERMINATED; 2) the 
> second level reason REASON_DOCKER_PULL_FAILURE;
> 
> 2. We probably want to allow extension to TaskStatus::Reason enum. For 
> example, some framework/executor may want to add customized reasons. We could 
> leverage the protobuf extension support to achieve that 
> (https://developers.google.com/protocol-buffers/docs/proto#extensions).
> 
> 3. The semantics around Termination is broken currently and we need to 
> clean it up. For instance, the following code is broken,
> ```
> void Slave::sendExecutorTerminatedStatusUpdate(...)
> {
>   ...
>   if (termination.isReady() && termination.get().killed()) {
> taskState = TASK_FAILED;
> // TODO(dhamon): 

Re: Review Request 32837: Re-order structs in Slave State.hpp to prevent forward declaration dependency.

[Joris Van Remoortere]

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

(Updated April 30, 2015, 11:43 p.m.)


Review request for mesos, Ben Mahler, Cody Maloney, and Michael Park.


Changes
---

Update comment.


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


Repository: mesos


Description
---

Prep for unrestricted union Option.


Diffs (updated)
-

  src/slave/state.hpp 31dfdd5a4b644f466756a712deded1b025a73c02 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 33733: Fixed style error with template definitions withing common/parse.hpp

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [33733]

All tests passed.

- Mesos ReviewBot


On April 30, 2015, 10:45 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33733/
> ---
> 
> (Updated April 30, 2015, 10:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joerg Schad, Michael Park, and 
> Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaces every instance of `template<>` to `template <>` in the file 
> `src/common/parse.hpp`
> 
> 
> Diffs
> -
> 
>   src/common/parse.hpp 547b32041f39f0ff0c38179b66a32b2239134abc 
> 
> Diff: https://reviews.apache.org/r/33733/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 33241: docs: Add documentation on observability metrics.

[Joe Smith]

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



docs/metrics.md


master/slave_shutdowns_completed

These are slaves which were not heard from despite the slave-removal rate 
limit, and have been removed from the Master's slave registry. This is 
essentially the slave failing its health check.



docs/metrics.md


These are slaves that are able to cleanly re-join the cluster and connect 
back to the master after the master is disconnected.



docs/metrics.md


These are slaves which are being removed for various reasons, including 
Maintenance. This metric will be broken out for 
https://issues.apache.org/jira/browse/MESOS-2485



docs/metrics.md


Slaves which have failed their health check and are scheduled to be 
removed. They will not be immediately removed due to the Slave Removal 
Rate-Limit, but master/slave_shutdowns_completed will start increasing as they 
do get removed.



docs/metrics.md


This happens when the slave removal rate limit allows for a slave to 
reconnect and send a PONG to the master before being removed.


- Joe Smith


On April 27, 2015, 9:41 a.m., Ricardo Cervera-Navarro wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33241/
> ---
> 
> (Updated April 27, 2015, 9:41 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-2621
> https://issues.apache.org/jira/browse/MESOS-2621
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> docs: Add documentation on observability metrics.
> 
> 
> Diffs
> -
> 
>   docs/home.md 6ab61f85aa7d62e0f19267b886dffb4e0aa826ea 
>   docs/metrics.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33241/diff/
> 
> 
> Testing
> ---
> 
> - Previewed in markdown editor.
> - Replaced page content using the Element Inspector in Chrome to ensure that 
> the tables look good.
> 
> 
> File Attachments
> 
> 
> mesosmetrics2.webarchive
>   
> https://reviews.apache.org/media/uploaded/files/2015/04/27/b89946c8-f709-4bca-b610-affd584c62f3__mesosmetrics2.webarchive
> 
> 
> Thanks,
> 
> Ricardo Cervera-Navarro
> 
>

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

[Alexander Rojas]

> On April 23, 2015, 7:27 p.m., Benjamin Hindman wrote:
> > src/common/parse.hpp, line 46
> > 
> >
> > s/template<>/template <>/
> > 
> > (Here and everywhere else please!)
> 
> Alexander Rojas wrote:
> Now in review [33730](https://reviews.apache.org/r/33730/) (It needed to 
> be fixed in the whole file).

Discarded the fix because this patch no longer modifies anything in that file, 
now it stands as a standalone patch at 
[33733](https://reviews.apache.org/r/33733/).


- Alexander


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


On May 1, 2015, 12:39 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33296/
> ---
> 
> (Updated May 1, 2015, 12:39 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2620
> https://issues.apache.org/jira/browse/MESOS-2620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b 
>   src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
>   src/master/flags.hpp 996cf38c88f9718e55e88d6e906b5e3d1989478a 
>   src/master/flags.cpp 5798989d3f135978ec3d5f714b1cd8d84180fc90 
>   src/master/main.cpp 18f8c3178459da0cbf23a1817ec49cd9d3998bfd 
>   src/messages/flags.hpp PRE-CREATION 
>   src/messages/flags.proto PRE-CREATION 
>   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
>   src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
>   src/slave/main.cpp c62d3ab9825bf952071e8e312d383a0cb46547d2 
> 
> Diff: https://reviews.apache.org/r/33296/diff/
> 
> 
> Testing
> ---
> 
> make check and manual tests.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

[Alexander Rojas]

> On April 23, 2015, 7:20 p.m., Michael Park wrote:
> > src/common/parse.hpp, line 46
> > 
> >
> > nit: `s/template<>/template <>/`
> 
> Alexander Rojas wrote:
> I was checking and all the entries in this file have the from 
> `template<>` should I change them all?
> 
> Michael Park wrote:
> Yes please!
> 
> Alexander Rojas wrote:
> Fix in review [33730](https://reviews.apache.org/r/33730/)

Discarded the fix because this patch no longer modifies anything in that file, 
now it stands as a standalone patch at 
[33733](https://reviews.apache.org/r/33733/).


- Alexander


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


On May 1, 2015, 12:39 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33296/
> ---
> 
> (Updated May 1, 2015, 12:39 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2620
> https://issues.apache.org/jira/browse/MESOS-2620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b 
>   src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
>   src/master/flags.hpp 996cf38c88f9718e55e88d6e906b5e3d1989478a 
>   src/master/flags.cpp 5798989d3f135978ec3d5f714b1cd8d84180fc90 
>   src/master/main.cpp 18f8c3178459da0cbf23a1817ec49cd9d3998bfd 
>   src/messages/flags.hpp PRE-CREATION 
>   src/messages/flags.proto PRE-CREATION 
>   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
>   src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
>   src/slave/main.cpp c62d3ab9825bf952071e8e312d383a0cb46547d2 
> 
> Diff: https://reviews.apache.org/r/33296/diff/
> 
> 
> Testing
> ---
> 
> make check and manual tests.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Review Request 33733: Fixed style error with template definitions withing common/parse.hpp

[Alexander Rojas]

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

Review request for mesos, Benjamin Hindman, Joerg Schad, Michael Park, and Till 
Toenshoff.


Repository: mesos


Description
---

Replaces every instance of `template<>` to `template <>` in the file 
`src/common/parse.hpp`


Diffs
-

  src/common/parse.hpp 547b32041f39f0ff0c38179b66a32b2239134abc 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

[Alexander Rojas]

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

(Updated May 1, 2015, 12:39 a.m.)


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


Changes
---

Moves protobuf defining the firewall rules format to a new `flags.proto` file. 
Adds documentation.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b 
  src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
  src/master/flags.hpp 996cf38c88f9718e55e88d6e906b5e3d1989478a 
  src/master/flags.cpp 5798989d3f135978ec3d5f714b1cd8d84180fc90 
  src/master/main.cpp 18f8c3178459da0cbf23a1817ec49cd9d3998bfd 
  src/messages/flags.hpp PRE-CREATION 
  src/messages/flags.proto PRE-CREATION 
  src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
  src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
  src/slave/main.cpp c62d3ab9825bf952071e8e312d383a0cb46547d2 

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


Testing
---

make check and manual tests.


Thanks,

Alexander Rojas

Re: Review Request 33490: MESOS-2633 Avoid adding duplicate tasks

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [33376, 33490]

All tests passed.

- Mesos ReviewBot


On April 30, 2015, 9:46 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33490/
> ---
> 
> (Updated April 30, 2015, 9:46 p.m.)
> 
> 
> Review request for mesos, Adam B and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Framework::addCompletedTask(const Task& task) we did not check
> for duplicated tasks, so they could be added more than once.
> 
> A simple check has now been added (there still is the issue
> of whether the `operator ==` on `Task` is too strict, so as
> never to cause a match).
> 
> 
> Diffs
> -
> 
>   src/master/framework.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33490/diff/
> 
> 
> Testing
> ---
> 
> buildbot make check pass
> http://build.mesosphere.com:/builders/dev_test/builds/13
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 33490: MESOS-2633 Avoid adding duplicate tasks

[Marco Massenzio]

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

(Updated April 30, 2015, 9:46 p.m.)


Review request for mesos, Adam B and Joris Van Remoortere.


Changes
---

Replaced original UnaryComparator with a lambda


Repository: mesos


Description
---

In Framework::addCompletedTask(const Task& task) we did not check
for duplicated tasks, so they could be added more than once.

A simple check has now been added (there still is the issue
of whether the `operator ==` on `Task` is too strict, so as
never to cause a match).


Diffs (updated)
-

  src/master/framework.cpp PRE-CREATION 

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


Testing
---

buildbot make check pass
http://build.mesosphere.com:/builders/dev_test/builds/13


Thanks,

Marco Massenzio

Re: Review Request 32837: Re-order structs in Slave State.hpp to prevent forward declaration dependency.

[Ben Mahler]

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



src/slave/state.hpp


It seems a bit odd to put the top-level state in the middle, before one can 
see the tree structure through the order they're listed in the header. Why not 
just forward declare above?


- Ben Mahler


On April 30, 2015, 4:22 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32837/
> ---
> 
> (Updated April 30, 2015, 4:22 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, and Michael Park.
> 
> 
> Bugs: MESOS-1991
> https://issues.apache.org/jira/browse/MESOS-1991
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prep for unrestricted union Option.
> 
> 
> Diffs
> -
> 
>   src/slave/state.hpp 31dfdd5a4b644f466756a712deded1b025a73c02 
> 
> Diff: https://reviews.apache.org/r/32837/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 33376: MESOS-2633 Moved struct Framework methods to their own implementation class.

[Marco Massenzio]

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

(Updated April 30, 2015, 9:44 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Addressed comments


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


Repository: mesos


Description
---

Created new file framework.cpp containing all the methods' implementations for 
the `Framework` class (declared in master/master.hpp)

Declared `operator ==` for Task in type_utils.hpp 
(it was implemented before, but not declared in the header file);

Refactored all the LOG(WARNING) to a single utility method.


Diffs (updated)
-

  include/mesos/type_utils.hpp 044637481e5405d4d6f61653a9f9386edd191deb 
  src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
  src/master/framework.cpp PRE-CREATION 
  src/master/master.hpp 49ee050ca4d2b2c5f75ce864fcf6ae703dfdeadd 

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


Testing
---

All tests (make check) pass.


Thanks,

Marco Massenzio

Re: Review Request 32838: Use unrestricted union to remove dynamic allocation from Option.

[Ben Mahler]

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

Ship it!


Ship It!

- Ben Mahler


On April 30, 2015, 4:22 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32838/
> ---
> 
> (Updated April 30, 2015, 4:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Cody Maloney, and 
> Michael Park.
> 
> 
> Bugs: MESOS-1991
> https://issues.apache.org/jira/browse/MESOS-1991
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactor `Option` to use unrestricted union to remove dynamic allocation.
> This depends on the upgrade to GCC 4.8+
> I used `std::remove_const::type` to remove the const from `T` when storing 
> `T` in the union, so that we can properly in-place new from outside the 
> initializer list for this case:
> `Option opt;`
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
> 47fe92c380de3e2abc625dc936afbd034280b76a 
> 
> Diff: https://reviews.apache.org/r/32838/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 33730: Fixes template style issue in common/parse.hpp

[Alexander Rojas]

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

(Updated April 30, 2015, 10:51 p.m.)


Review request for mesos, Benjamin Hindman, Michael Park, and Till Toenshoff.


Repository: mesos


Description
---

Fixes all versions of `template<>` to `template <>` in order to accommodate to 
the guidelines.


Diffs
-


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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 30609: Added a function that reports file size, not following links.

[Bernd Mathiske]

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

(Updated April 30, 2015, 1:38 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and Timothy 
Chen.


Changes
---

Added test comment.


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


Repository: mesos


Description
---

This returns a file's size (on UNIXes as reported by lstat(), not stat()). It 
is desired that in case of a link, the size of the link, not the size of the 
referenced file, is returned.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/stat.hpp 
270c4c848fc0460dcdb9a90823281d735f4550c2 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
343f95be7f316170b37c9358627f3c2090f0e29e 

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


Testing
---

Wrote a simple test that creates a file and tests its size, and also checks if 
a non-existing file yields an error.


Thanks,

Bernd Mathiske

Re: Review Request 30609: Added a function that reports file size, not following links.

[Bernd Mathiske]

> On April 29, 2015, 2:51 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp, line 209
> > 
> >
> > Add a short comment on what this test does, please. We are trying to 
> > get this done for all new tests and for all those we touch.

Good point. Will do.


- Bernd


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


On March 11, 2015, 10:06 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30609/
> ---
> 
> (Updated March 11, 2015, 10:06 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-2072
> https://issues.apache.org/jira/browse/MESOS-2072
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This returns a file's size (on UNIXes as reported by lstat(), not stat()). It 
> is desired that in case of a link, the size of the link, not the size of the 
> referenced file, is returned.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/stat.hpp 
> af940a48b161c194f2efb529b3d589c543b12f61 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> c396c1d2d833b2f1721092fa35b23b5c3c3d99b3 
> 
> Diff: https://reviews.apache.org/r/30609/diff/
> 
> 
> Testing
> ---
> 
> Wrote a simple test that creates a file and tests its size, and also checks 
> if a non-existing file yields an error.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 32108: Added manual make for readability training source code.

[Bernd Mathiske]

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

(Updated April 30, 2015, 1:23 p.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
---

The pre-commit hook did not work, because build boit uses commit before 
configure, etc. happens. Inserted "readability" as normal build target now. 
Suggestions?

More examples for naming issues.

Source code license headers.


Repository: mesos


Description
---

Readability source code now has a Makefile that is generated by bootstrap and 
configure. "cd build/readability; make" compiles the sources in "readability/". 
No linking occurs. This is just to ensure that we have syntactially correct and 
type-checked example files.

Slightly rearranged the content of naming_*.cpp and broke out an extra file for 
whitespace issues.

Replaces:
https://reviews.apache.org/r/31990/
https://reviews.apache.org/r/31992/


Diffs (updated)
-

  Makefile.am dcd0cb474944ae9c882e6cbdb64a33b4be5b9083 
  configure.ac 589ae97d0432370b462576cd1985544564893999 
  readability/Makefile.am PRE-CREATION 
  readability/README.md PRE-CREATION 
  readability/naming_comments.cpp PRE-CREATION 
  readability/naming_review.cpp PRE-CREATION 
  readability/whitespace_comments.cpp PRE-CREATION 
  readability/whitespace_review.cpp PRE-CREATION 

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


Testing
---

cd build/readability; make

Also observed that the pre-commit hook does this as well.


Thanks,

Bernd Mathiske

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

[Alexander Rojas]

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

(Updated April 30, 2015, 10:14 p.m.)


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


Changes
---

Just updates the patch to accomodate for changes on its parent one.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/mesos.proto 967b1e3bbfb3f6b71d5a15d02cba7ed5ec21816f 
  include/mesos/type_utils.hpp 044637481e5405d4d6f61653a9f9386edd191deb 
  src/common/parse.hpp 547b32041f39f0ff0c38179b66a32b2239134abc 
  src/master/flags.hpp 996cf38c88f9718e55e88d6e906b5e3d1989478a 
  src/master/flags.cpp 5798989d3f135978ec3d5f714b1cd8d84180fc90 
  src/master/main.cpp 18f8c3178459da0cbf23a1817ec49cd9d3998bfd 
  src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
  src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
  src/slave/main.cpp c62d3ab9825bf952071e8e312d383a0cb46547d2 

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


Testing
---

make check and manual tests.


Thanks,

Alexander Rojas

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

[Alexander Rojas]

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

(Updated April 30, 2015, 10:13 p.m.)


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


Changes
---

Fixes non comment related issues (documentation is comming).


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


Repository: mesos


Description
---

Introduces the interface `FirewallRule` which will be matched against incoming 
connections in order to allow them to be served or being blocked. For details, 
check the [design 
doc](https://docs.google.com/document/d/1JSJTJMJ6ZXLkCSmvOIabTLrjtqqr0E-u99Rx2BHR1hs/edit).


Diffs (updated)
-

  3rdparty/libprocess/include/Makefile.am 
8aab0593f296c7aae71289f9bd6cf3eb3578a721 
  3rdparty/libprocess/include/process/firewall.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/process.hpp 
392c74df3e8a122aecd3633dffdeec4bcbd1f097 
  3rdparty/libprocess/src/process.cpp 8ab0dbcd3e9b858244f1e67061897143204eada0 
  3rdparty/libprocess/src/tests/process_tests.cpp 
b6c6c511cabc525516b55d38fc4f5f596a3d6517 

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


Testing
---

make check


Thanks,

Alexander Rojas

Review Request 33730: Fixes template style issue in common/parse.hpp

[Alexander Rojas]

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

Review request for mesos, Benjamin Hindman, Michael Park, and Till Toenshoff.


Summary (updated)
-

Fixes template style issue in common/parse.hpp


Repository: mesos


Description (updated)
---

Fixes all versions of `template<>` to `template <>` in order to accommodate to 
the guidelines.


Diffs
-


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


Testing (updated)
---

make check


Thanks,

Alexander Rojas

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

[Alexander Rojas]

> On April 23, 2015, 7:20 p.m., Michael Park wrote:
> > src/common/parse.hpp, line 46
> > 
> >
> > nit: `s/template<>/template <>/`
> 
> Alexander Rojas wrote:
> I was checking and all the entries in this file have the from 
> `template<>` should I change them all?
> 
> Michael Park wrote:
> Yes please!

Fix in review [33730](https://reviews.apache.org/r/33730/)


> On April 23, 2015, 7:20 p.m., Michael Park wrote:
> > src/common/parse.hpp, line 60
> > 
> >
> > nit: `s/template<>/template <>/`

Fix in review [33730](https://reviews.apache.org/r/33730/)


- Alexander


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


On April 22, 2015, 4:36 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33296/
> ---
> 
> (Updated April 22, 2015, 4:36 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2620
> https://issues.apache.org/jira/browse/MESOS-2620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 
>   include/mesos/type_utils.hpp cdf5864389a72002b538c263d70bcade2bdffa45 
>   src/common/parse.hpp 547b32041f39f0ff0c38179b66a32b2239134abc 
>   src/master/flags.hpp 996cf38c88f9718e55e88d6e906b5e3d1989478a 
>   src/master/flags.cpp 5798989d3f135978ec3d5f714b1cd8d84180fc90 
>   src/master/main.cpp 88bb39d57ea7d654bea11a3db0cf0903b4b22d99 
>   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
>   src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
>   src/slave/main.cpp c62d3ab9825bf952071e8e312d383a0cb46547d2 
> 
> Diff: https://reviews.apache.org/r/33296/diff/
> 
> 
> Testing
> ---
> 
> make check and manual tests.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

[Alexander Rojas]

> On April 23, 2015, 7:27 p.m., Benjamin Hindman wrote:
> > src/common/parse.hpp, line 46
> > 
> >
> > s/template<>/template <>/
> > 
> > (Here and everywhere else please!)

Now in review [33730](https://reviews.apache.org/r/33730/) (It needed to be 
fixed in the whole file).


- Alexander


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


On April 22, 2015, 4:36 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33296/
> ---
> 
> (Updated April 22, 2015, 4:36 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2620
> https://issues.apache.org/jira/browse/MESOS-2620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3a8e8bf303e0576c212951f6028af77e54d93537 
>   include/mesos/type_utils.hpp cdf5864389a72002b538c263d70bcade2bdffa45 
>   src/common/parse.hpp 547b32041f39f0ff0c38179b66a32b2239134abc 
>   src/master/flags.hpp 996cf38c88f9718e55e88d6e906b5e3d1989478a 
>   src/master/flags.cpp 5798989d3f135978ec3d5f714b1cd8d84180fc90 
>   src/master/main.cpp 88bb39d57ea7d654bea11a3db0cf0903b4b22d99 
>   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
>   src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
>   src/slave/main.cpp c62d3ab9825bf952071e8e312d383a0cb46547d2 
> 
> Diff: https://reviews.apache.org/r/33296/diff/
> 
> 
> Testing
> ---
> 
> make check and manual tests.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 30609: Added a function that reports file size, not following links.

[Bernd Mathiske]

> On April 29, 2015, 4:48 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/stat.hpp, lines 61-79
> > 
> >
> > If we add `followSymlinks` here, it seems that one could add 
> > `followSymlinks` on all of the `stat` related methods, but that seems like 
> > it would be a bit of a mess?
> > 
> > I'm curious whether we should just expose two simple functions to force 
> > the callers to think about following links whenever they want file 
> > statistics:
> > 
> > ```
> > Try stat(const std::string& path);
> > Try lstat(const std::string& path);
> > ```
> > 
> > Otherwise, do we want 1 stout function per `stat` struct member? For 
> > each function, are we going to have a `followSymlinks` boolean? Just 
> > thinking of how to provide a complete and consistent interface that is 
> > simple to reason about.

Yes, adding followSymlinks to existing functions could be a bit of a mess, 
because we would end up with different defaults, since stat and lstat are not 
being used consistently at the moment.

But adding the two wrapper functions stat and lstat without removing all the 
others and rewriting all the call sites would be a bit of a mess, too. 

Suggestions?

Bernd


- Bernd


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


On March 11, 2015, 10:06 a.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30609/
> ---
> 
> (Updated March 11, 2015, 10:06 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-2072
> https://issues.apache.org/jira/browse/MESOS-2072
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This returns a file's size (on UNIXes as reported by lstat(), not stat()). It 
> is desired that in case of a link, the size of the link, not the size of the 
> referenced file, is returned.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/stat.hpp 
> af940a48b161c194f2efb529b3d589c543b12f61 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> c396c1d2d833b2f1721092fa35b23b5c3c3d99b3 
> 
> Diff: https://reviews.apache.org/r/30609/diff/
> 
> 
> Testing
> ---
> 
> Wrote a simple test that creates a file and tests its size, and also checks 
> if a non-existing file yields an error.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 33646: [Proposal] Follow Google Style Guide for include order completely.

[Alexander Rojas]

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

Ship it!


Ship It!

- Alexander Rojas


On April 30, 2015, 7:17 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33646/
> ---
> 
> (Updated April 30, 2015, 7:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2673
> https://issues.apache.org/jira/browse/MESOS-2673
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Proposal for following Google Style Guide for include order completely.
> 
> 
> Diffs
> -
> 
>   docs/proposals/style/include_order.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33646/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 33646: [Proposal] Follow Google Style Guide for include order completely.

[Joerg Schad]

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

(Updated April 30, 2015, 5:17 p.m.)


Review request for mesos, Benjamin Hindman and Joris Van Remoortere.


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


Repository: mesos


Description
---

Proposal for following Google Style Guide for include order completely.


Diffs
-

  docs/proposals/style/include_order.md PRE-CREATION 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 32838: Use unrestricted union to remove dynamic allocation from Option.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [32837, 33558, 33572, 33193, 32838]

All tests passed.

- Mesos ReviewBot


On April 30, 2015, 4:22 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32838/
> ---
> 
> (Updated April 30, 2015, 4:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Cody Maloney, and 
> Michael Park.
> 
> 
> Bugs: MESOS-1991
> https://issues.apache.org/jira/browse/MESOS-1991
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactor `Option` to use unrestricted union to remove dynamic allocation.
> This depends on the upgrade to GCC 4.8+
> I used `std::remove_const::type` to remove the const from `T` when storing 
> `T` in the union, so that we can properly in-place new from outside the 
> initializer list for this case:
> `Option opt;`
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
> 47fe92c380de3e2abc625dc936afbd034280b76a 
> 
> Diff: https://reviews.apache.org/r/32838/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 33376: MESOS-2633 Moved struct Framework methods to their own implementation class.

[Joris Van Remoortere]

> On April 29, 2015, 8:24 p.m., Joris Van Remoortere wrote:
> > src/master/framework.cpp, lines 101-102
> > 
> >
> > The style is either:
> > Follow the alignment if you stay on the 1st line:
> > ```
> > bool Framework::hasExecutor(const SlaveID& slaveId,
> > const ExecutorID& executorId)
> > ```
> > or
> > Indent by 4 if you start the parameter list on a new line:
> > ```
> > bool Framework::hasExecutor(
> > const SlaveID& slaveId,
> > const ExecutorID& executorId)
> > ```
> > 
> > Here and the others below please!
> 
> Marco Massenzio wrote:
> ummm...
> // 2: Don't use.
> allocator->resourcesRecovered(frameworkId, slaveId,
>   resources, filters);
> 
> but I agree I had misread (4):
> 
> // 4: OK.
> allocator->resourcesRecovered(
> frameworkId,
> slaveId,
> resources,
> filters);
> 
> will change

I think rule 2 might not be clear. It's suggesting that if you align like that, 
then you should only have 1 argument per line, not 2.


- Joris


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


On April 22, 2015, 8:35 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33376/
> ---
> 
> (Updated April 22, 2015, 8:35 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2633
> https://issues.apache.org/jira/browse/MESOS-2633
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created new file framework.cpp containing all the methods' implementations 
> for the `Framework` class (declared in master/master.hpp)
> 
> Declared `operator ==` for Task in type_utils.hpp 
> (it was implemented before, but not declared in the header file);
> 
> Refactored all the LOG(WARNING) to a single utility method.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp cdf5864389a72002b538c263d70bcade2bdffa45 
>   src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
>   src/master/framework.cpp PRE-CREATION 
>   src/master/master.hpp 550d2c50cd01aa5830a7e7e03ab4a0240c221b15 
> 
> Diff: https://reviews.apache.org/r/33376/diff/
> 
> 
> Testing
> ---
> 
> All tests (make check) pass.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 32838: Use unrestricted union to remove dynamic allocation from Option.

[Joris Van Remoortere]

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

(Updated April 30, 2015, 4:22 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Cody Maloney, and 
Michael Park.


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


Repository: mesos


Description
---

Refactor `Option` to use unrestricted union to remove dynamic allocation.
This depends on the upgrade to GCC 4.8+
I used `std::remove_const::type` to remove the const from `T` when storing 
`T` in the union, so that we can properly in-place new from outside the 
initializer list for this case:
`Option opt;`


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
47fe92c380de3e2abc625dc936afbd034280b76a 

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


Testing
---

make check


Thanks,

Joris Van Remoortere

Re: Review Request 32838: Use unrestricted union to remove dynamic allocation from Option.

[Joris Van Remoortere]

> On April 30, 2015, 12:13 a.m., Ben Mahler wrote:
> > Do you want to add the 'mesos' group or is this not ready to be published?

Added 'mesos' group. Thanks!


- Joris


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


On April 30, 2015, 4:22 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32838/
> ---
> 
> (Updated April 30, 2015, 4:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Cody Maloney, and 
> Michael Park.
> 
> 
> Bugs: MESOS-1991
> https://issues.apache.org/jira/browse/MESOS-1991
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactor `Option` to use unrestricted union to remove dynamic allocation.
> This depends on the upgrade to GCC 4.8+
> I used `std::remove_const::type` to remove the const from `T` when storing 
> `T` in the union, so that we can properly in-place new from outside the 
> initializer list for this case:
> `Option opt;`
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
> 47fe92c380de3e2abc625dc936afbd034280b76a 
> 
> Diff: https://reviews.apache.org/r/32838/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 32837: Re-order structs in Slave State.hpp to prevent forward declaration dependency.

[Joris Van Remoortere]

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

(Updated April 30, 2015, 4:22 p.m.)


Review request for mesos, Ben Mahler, Cody Maloney, and Michael Park.


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


Repository: mesos


Description
---

Prep for unrestricted union Option.


Diffs
-

  src/slave/state.hpp 31dfdd5a4b644f466756a712deded1b025a73c02 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 33376: MESOS-2633 Moved struct Framework methods to their own implementation class.

[Marco Massenzio]

> On April 29, 2015, 8:24 p.m., Joris Van Remoortere wrote:
> > src/master/framework.cpp, lines 185-186
> > 
> >
> > Can you find other places in the code where the TODOs are aligned like 
> > this?
> > It's definitely neat, but it seems to be different from other places.
> 
> Marco Massenzio wrote:
> I actually did ask Nik - and he suggested this.
> I'll check out other TODO's in the code base

Most multiline TODO's seem to just continue without the lined-up indent
Google style guid is silent on the topic (AFAICT)

The line is anyway gone


- Marco


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


On April 22, 2015, 8:35 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33376/
> ---
> 
> (Updated April 22, 2015, 8:35 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2633
> https://issues.apache.org/jira/browse/MESOS-2633
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created new file framework.cpp containing all the methods' implementations 
> for the `Framework` class (declared in master/master.hpp)
> 
> Declared `operator ==` for Task in type_utils.hpp 
> (it was implemented before, but not declared in the header file);
> 
> Refactored all the LOG(WARNING) to a single utility method.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp cdf5864389a72002b538c263d70bcade2bdffa45 
>   src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
>   src/master/framework.cpp PRE-CREATION 
>   src/master/master.hpp 550d2c50cd01aa5830a7e7e03ab4a0240c221b15 
> 
> Diff: https://reviews.apache.org/r/33376/diff/
> 
> 
> Testing
> ---
> 
> All tests (make check) pass.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

[Alexander Rojas]

> On April 23, 2015, 2:49 p.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/process.cpp, line 1993
> > 
> >
> > `<< "': firewall rule forbids connection";` might be a bit more 
> > mesos'esque.

It follows the patter of that same function.


- Alexander


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


On April 22, 2015, 4:35 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33295/
> ---
> 
> (Updated April 22, 2015, 4:35 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, and Till Toenshoff.
> 
> 
> Bugs: MESOS-2620
> https://issues.apache.org/jira/browse/MESOS-2620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the interface `FirewallRule` which will be matched against 
> incoming connections in order to allow them to be served or being blocked. 
> For details, check the [design 
> doc](https://docs.google.com/document/d/1JSJTJMJ6ZXLkCSmvOIabTLrjtqqr0E-u99Rx2BHR1hs/edit).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> 3da3e6cef1b5cb66c223425744d84741846ea730 
>   3rdparty/libprocess/include/process/firewall.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 
> 392c74df3e8a122aecd3633dffdeec4bcbd1f097 
>   3rdparty/libprocess/src/process.cpp 
> 97ac09fd10b767bc46387abc3657d005a00830c8 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> eb38edce2c483ba7f963a826893a15a075238618 
> 
> Diff: https://reviews.apache.org/r/33295/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 33376: MESOS-2633 Moved struct Framework methods to their own implementation class.

[Marco Massenzio]

> On April 29, 2015, 8:24 p.m., Joris Van Remoortere wrote:
> > src/master/framework.cpp, line 187
> > 
> >
> > 1) Let's sync with BenH if we want to factor out logging like this. 
> > There are arguments on both sides, so let's follow up with a discussion, 
> > and maybe add it to the style guide!
> > 2) *If* we *do* decide to factor out the logging like this, let's 
> > change the name to be more meaninful, or even move this function into 
> > `updateFrameworkInfo` as a lambda. This message is clearly specific to that 
> > function, and is not usable by other functions in `Framework`.
> > 3) Since we're in the implementation file, we can do `using 
> > std::string` and remove the namespacing of `std::string` here.
> 
> Marco Massenzio wrote:
> I actually wasn't quite sure myself (and the more I think about this, the 
> less I like my own choice): this is just a helper function, to avoid 
> violating the DRY Principle.
> I would like to move it out of the Framework class, and just leave it the 
> cpp file as a local (static) helper method.
> Again, to me, it scratches the itch of copy & pasted code, which I 
> positively dislike.
> 
> If I change it to a local helper method, would that address your concerns?
> 
> Ben Mahler wrote:
> >this is just a helper function, to avoid violating the DRY Principle.
> 
> This is a great example of how to apply DRY! :)
> 
> Namely, we have to be careful about introducing helper functions to 
> follow the DRY principle as it often comes at a cost: loss of readability. In 
> this case, the reader encountering `logWarning` has no _local_ clues to know 
> that `logWarning` is not a general way to log warnings, but rather it prints 
> a special message about updating framework information. That doesn't seem 
> very intuitive? Also, now we have a function at the Framework scope, callable 
> from any other place in the framework code, which is going to mean that when 
> a reader encounters it, he/she won't be sure that it's only used within the 
> updating FrameworkInfo code. So in the process of applying DRY we've made our 
> software harder to understand. Does this make sense?
> 
> There are ways to follow DRY without sacrificing readability, for 
> example, can we collect the set of fields that we weren't able to change and 
> just print them together in one log message? If there's some reason to have 
> multiple log messages, could we have a local lambda for it? (Although I don't 
> see a need for having 1 log message per field).

Absolutely, Ben, thanks for clarifying.
That's actually what I meant by 'scratchin the itch' (I realize now I was being 
economical with words... I was typing single-handed... which is != from 
single-handedly typing :)
I've already reverted the code.


- Marco


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


On April 22, 2015, 8:35 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33376/
> ---
> 
> (Updated April 22, 2015, 8:35 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2633
> https://issues.apache.org/jira/browse/MESOS-2633
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created new file framework.cpp containing all the methods' implementations 
> for the `Framework` class (declared in master/master.hpp)
> 
> Declared `operator ==` for Task in type_utils.hpp 
> (it was implemented before, but not declared in the header file);
> 
> Refactored all the LOG(WARNING) to a single utility method.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp cdf5864389a72002b538c263d70bcade2bdffa45 
>   src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
>   src/master/framework.cpp PRE-CREATION 
>   src/master/master.hpp 550d2c50cd01aa5830a7e7e03ab4a0240c221b15 
> 
> Diff: https://reviews.apache.org/r/33376/diff/
> 
> 
> Testing
> ---
> 
> All tests (make check) pass.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 32150: Enabled the master to handle reservation operations.

[Alexander Rukletsov]

> On April 29, 2015, 9:54 a.m., Alexander Rukletsov wrote:
> > src/master/master.cpp, lines 2486-2488
> > 
> >
> > This looks a bit weird. Is it what `clang-format` proposes?
> 
> Michael Park wrote:
> It is indeed. Do you have a preferred formatting? I couldn't really find 
> one that I consider to be the definitive answer so I went with this.

If in doubt, go for consistency:
`stout/flags/flags.hpp:182`
`stout/flags/flags.hpp:247`
`libprocess/src/http.cpp:664`

I think this is simple, readable and kind of consistent with our style guide: 
```
Option principal = framework->info.has_principal()
  ? framework->info.principal()
  : Option::none();
```


- Alexander


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


On April 28, 2015, 10:43 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32150/
> ---
> 
> (Updated April 28, 2015, 10:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Jie Yu.
> 
> 
> Bugs: MESOS-2139
> https://issues.apache.org/jira/browse/MESOS-2139
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Handled reservation operations in `Master::_accept`.
> 
> Added `validate` functions in `src/master/validation.{hpp,cpp}`.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp d42a6f321c88ec5d0418264bdda39d083ff54a7e 
>   src/master/validation.hpp 2d7416c053f82d6316542fa9c35b0e7bc605abec 
>   src/master/validation.cpp dc25995bf57397d42fcde458414f0402d19bf792 
>   src/tests/master_validation_tests.cpp 
> 4f2ad58c3ae0f611fb476c4d91a37dd6a5541395 
> 
> Diff: https://reviews.apache.org/r/32150/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [29327]

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

Error:
 2015-04-30 07:45:09 URL:https://reviews.apache.org/r/29327/diff/raw/ 
[2935/2935] -> "29327.patch" [1]
error: patch failed: src/slave/containerizer/docker.cpp:499
error: src/slave/containerizer/docker.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On April 30, 2015, 7:43 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29889/
> ---
> 
> (Updated April 30, 2015, 7:43 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a one mega patch and contains many reviews that's already on rb.
> 
> This review is not meant to be merged, only provided for easier review.
> 
> 
> Diffs
> -
> 
>   Dockerfile 35abf25 
>   docs/configuration.md 54c4e31 
>   docs/docker-containerizer.md a5438b7 
>   src/Makefile.am 93c7c8a 
>   src/docker/docker.hpp 3ebbc1f 
>   src/docker/docker.cpp 3a485a2 
>   src/docker/executor.cpp PRE-CREATION 
>   src/slave/constants.hpp fd1c1ab 
>   src/slave/constants.cpp 2a99b11 
>   src/slave/containerizer/docker.hpp b25ec55 
>   src/slave/containerizer/docker.cpp f9fc89a 
>   src/slave/flags.hpp d3b1ce1 
>   src/slave/flags.cpp d0932b0 
>   src/tests/docker_containerizer_tests.cpp c9d66b3 
> 
> Diff: https://reviews.apache.org/r/29889/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

[Timothy Chen]

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

(Updated April 30, 2015, 7:43 a.m.)


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


Repository: mesos


Description
---

This is a one mega patch and contains many reviews that's already on rb.

This review is not meant to be merged, only provided for easier review.


Diffs (updated)
-

  Dockerfile 35abf25 
  docs/configuration.md 54c4e31 
  docs/docker-containerizer.md a5438b7 
  src/Makefile.am 93c7c8a 
  src/docker/docker.hpp 3ebbc1f 
  src/docker/docker.cpp 3a485a2 
  src/docker/executor.cpp PRE-CREATION 
  src/slave/constants.hpp fd1c1ab 
  src/slave/constants.cpp 2a99b11 
  src/slave/containerizer/docker.hpp b25ec55 
  src/slave/containerizer/docker.cpp f9fc89a 
  src/slave/flags.hpp d3b1ce1 
  src/slave/flags.cpp d0932b0 
  src/tests/docker_containerizer_tests.cpp c9d66b3 

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


Testing
---

make check


Thanks,

Timothy Chen