Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-27 Thread Adam B
On Jan. 27, 2014, 10:44 p.m., Vinod Kone wrote: src/slave/slave.cpp, lines 725-728 https://reviews.apache.org/r/16724/diff/3/?file=425219#file425219line725 I think you've brought this up before but did you figure out why a completed executor has terminated tasks? Adam B wrote:

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-27 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/ --- (Updated Feb. 27, 2014, 8:28 p.m.) Review request for mesos and Benjamin

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-27 Thread Adam B
On Jan. 29, 2014, 4:58 p.m., Vinod Kone wrote: src/slave/slave.cpp, line 712 https://reviews.apache.org/r/16724/diff/4/?file=454079#file454079line712 why the different log level? s/2/1/? Adam B wrote: I figured that the different log levels could be used for

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-27 Thread Adam B
On Feb. 27, 2014, 12:26 p.m., Benjamin Hindman wrote: src/tests/fault_tolerance_tests.cpp, line 665 https://reviews.apache.org/r/16724/diff/7/?file=497356#file497356line665 Please move '{' to newline and use const for parameters (and did you really want to return a 'const

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-18 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/#review34675 --- Ship it! src/master/master.cpp

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-18 Thread Adam B
On Feb. 18, 2014, 8:12 a.m., Niklas Nielsen wrote: src/master/master.cpp, line 2889 https://reviews.apache.org/r/16724/diff/6/?file=492069#file492069line2889 Where does completedFramework clash (since the '_')? I guess completedFramework doesn't actually clash here, but I thought

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-18 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/#review34767 --- src/tests/fault_tolerance_tests.cpp

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-18 Thread Adam B
On Feb. 18, 2014, 2:07 p.m., Vinod Kone wrote: src/tests/fault_tolerance_tests.cpp, line 712 https://reviews.apache.org/r/16724/diff/6/?file=492072#file492072line712 Why do you need to AWAIT on this? Copy/paste coding, sanity-checking overkill as I was trying to get this stuff

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-18 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/ --- (Updated Feb. 18, 2014, 6:07 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-17 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/ --- (Updated Feb. 17, 2014, 4:23 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-07 Thread Adam B
On Jan. 29, 2014, 4:58 p.m., Vinod Kone wrote: src/slave/slave.cpp, line 712 https://reviews.apache.org/r/16724/diff/4/?file=454079#file454079line712 why the different log level? s/2/1/? I figured that the different log levels could be used for different verbosity

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-02-07 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/ --- (Updated Feb. 7, 2014, 3:56 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-29 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/ --- (Updated Jan. 29, 2014, 4:17 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-29 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/#review33187 --- Ship it! src/master/master.hpp

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-28 Thread Adam B
On Jan. 27, 2014, 10:44 p.m., Vinod Kone wrote: Can we add a test for this please? Certainly. I'll jump on that this week. Thanks for reviewing! On Jan. 27, 2014, 10:44 p.m., Vinod Kone wrote: src/master/master.cpp, line 1367

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-28 Thread Adam B
On Jan. 27, 2014, 8:54 p.m., Benjamin Hindman wrote: src/master/master.cpp, line 2657 https://reviews.apache.org/r/16724/diff/3/?file=425217#file425217line2657 Let's use the same parameter name here and in the declaration please (same for Master::readdCompletedFramework too).

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-28 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/#review33008 --- Did you forget to update the diff? I'm planning to cut 0.17.0-rc1

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-28 Thread Adam B
On Jan. 28, 2014, 9:59 a.m., Vinod Kone wrote: Did you forget to update the diff? I'm planning to cut 0.17.0-rc1 in an hour or so. If you want this patch to be included in that release please update it before then. Just marked things fixed as I made the edits at home. Building/testing

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-27 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/#review32949 --- Ship it! src/master/master.cpp

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-27 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/#review32961 --- Can we add a test for this please? src/master/master.cpp

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-16 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/ --- (Updated Jan. 16, 2014, 4:24 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-16 Thread Adam B
On Jan. 13, 2014, 2:52 p.m., Joe Smith wrote: src/slave/slave.cpp, line 731 https://reviews.apache.org/r/16724/diff/2/?file=420459#file420459line731 super naive question: is there a way we can add tests for this new chunk of code? would be great to lock-in (via tests) that the

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-13 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/#review31628 --- Ship it! Looks great Adam! src/master/master.hpp

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-13 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/#review31665 --- src/slave/slave.cpp

Re: Review Request 16724: Added completed frameworks/tasks to slave re-registration.

2014-01-10 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/ --- (Updated Jan. 10, 2014, 5:51 p.m.) Review request for mesos, Benjamin Hindman