Re: Review Request 16147: Containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Dec. 19, 2013, 7:59 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16147: Containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review30692 --- Hi Ian! Great progress - Here are a few breaking issues (will follo

Re: Review Request 16147: Containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Dec. 21, 2013, 6:50 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16147: Containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review31605 --- src/slave/container/containerizer.cpp

Re: Review Request 16147: Containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review31730 --- src/slave/container/containerizer.hpp

Re: Review Request 16147: Containerizer

> On Jan. 13, 2014, 6:52 a.m., Benjamin Hindman wrote: > > src/slave/container/containerizer.cpp, line 19 > > > > > > Not used? Used for ContainerizerProcess::create implementation which will appear in next update.

Re: Review Request 16147: Containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Jan. 17, 2014, 1:32 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16147: Containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Jan. 17, 2014, 1:36 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16147: Containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Dec. 10, 2013, 2:21 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16147: Containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Dec. 11, 2013, 4:05 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16147: Containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review30225 --- Partial review. One of the main things that has come up is the lack

Re: Review Request 16147: Containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review30208 --- src/slave/slave.hpp

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Jan. 21, 2014, 7:36 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Jan. 21, 2014, 7:33 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review32409 --- Ship it! This is looking really good. There are some minor cleanups

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review32209 --- Ship it! Good stuff! A couple of style nits and request for comment

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review32418 --- Please bear with me. I'm still pretty new, so my comments/questions

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review32670 --- Finally got around to reviewing the new containerizer files. Looks g

Re: Review Request 16147: Containerizer (part 1)

> On Jan. 14, 2014, 4:29 p.m., Jason Dusek wrote: > > src/slave/container/containerizer.hpp, line 84 > > > > > > If Containerizer::launch() had the resources, it could simplify working > > with many external containeri

Re: Review Request 16147: Containerizer (part 1)

> On Jan. 21, 2014, 11:39 p.m., Benjamin Hindman wrote: > > src/slave/slave.cpp, line 2041 > > > > > > What are the ramifications of the overcommit with respect to other > > containers? Should we be moving this close

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Jan. 24, 2014, 6:25 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16147: Containerizer (part 1)

> On Jan. 22, 2014, 12:01 a.m., Niklas Nielsen wrote: > > src/slave/container/mesos_containerizer.cpp, line 169 > > > > > > Is this async signal safe? No, turns out it isn't so I'll reimplement this functionality with

Re: Review Request 16147: Containerizer (part 1)

> On Jan. 22, 2014, 3:25 a.m., Adam B wrote: > > src/local/local.cpp, line 83 > > > > > > I wonder if we should we be mapping ContainerID -> Slave* instead? This map is storing Containerizers that have been created on

Re: Review Request 16147: Containerizer (part 1)

> On Jan. 24, 2014, 1:59 a.m., Adam B wrote: > > src/slave/container/containerizer.cpp, line 58 > > > > > > Do you need to use "process::" since you're "using namespace process" > > above? This code disappeared when I

Re: Review Request 16147: Containerizer (part 1)

> On Jan. 14, 2014, 4:29 p.m., Jason Dusek wrote: > > src/slave/container/containerizer.hpp, line 84 > > > > > > If Containerizer::launch() had the resources, it could simplify working > > with many external containeri

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Jan. 26, 2014, 8:18 a.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Jan. 27, 2014, 3 a.m.) Review request for mesos, Benjamin Hindman, Ben

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review32969 --- Looks great! My main concern is that there seems to be a bug in rec

Re: Review Request 16147: Containerizer (part 1)

> On Jan. 28, 2014, 9:16 a.m., Vinod Kone wrote: > > src/launcher/fetcher.cpp, lines 41-49 > > > > > > could we just do 'xf' and let tar figure out the compression? This was copied straight from the old launcher, but y

Re: Review Request 16147: Containerizer (part 1)

> On Jan. 28, 2014, 9:16 a.m., Vinod Kone wrote: > > src/local/local.cpp, line 153 > > > > > > s/isolator/containerizer/ ? > > > > Do you know what is this TODO about? > > Ian Downes wrote: > No, I don't.

Re: Review Request 16147: Containerizer (part 1)

> On Jan. 28, 2014, 9:16 a.m., Vinod Kone wrote: > > src/launcher/fetcher.cpp, line 222 > > > > > > Why the extraction in an else loop? Was this the old behavior? > > mesos.proto doesn't say that it is only extracted

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Jan. 31, 2014, 11:51 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16147: Containerizer (part 1)

> On Jan. 21, 2014, 11:39 p.m., Benjamin Hindman wrote: > > src/slave/container/containerizer.hpp, line 153 > > > > > > Doing a fetch should not block. We should probably have some > > abstractions around doing the op

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Feb. 1, 2014, 4:01 a.m.) Review request for mesos, Benjamin Hindman, B

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Feb. 6, 2014, 5:34 a.m.) Review request for mesos, Benjamin Hindman, B

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review33867 --- Ship it! src/launcher/fetcher.cpp

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review33880 --- src/launcher/fetcher.cpp

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Feb. 7, 2014, 3:35 a.m.) Review request for mesos, Benjamin Hindman, B

Re: Review Request 16147: Containerizer (part 1)

> On Feb. 7, 2014, 12:55 a.m., Niklas Nielsen wrote: > > src/slave/containerizer/mesos_containerizer.cpp, line 238 > > > > > > This is unfortunately not enough to silence gcc version 4.8.1 > > (Ubuntu/Linaro 4.8.1-

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review33866 --- Ship it! src/launcher/fetcher.cpp

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/#review34025 --- src/slave/containerizer/containerizer.hpp

Re: Review Request 16147: Containerizer (part 1)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16147/ --- (Updated Feb. 12, 2014, 12:17 a.m.) Review request for mesos, Benjamin Hindman,