> On 2012-03-23 05:17:01, Vinod Kone wrote: > > hadoop/hadoop-0.20.205.0.patch, line 1354 > > <https://reviews.apache.org/r/4448/diff/2/?file=94645#file94645line1354> > > > > kill ws here and everywhere else
Done (throughout the entire file!). > On 2012-03-23 05:17:01, Vinod Kone wrote: > > include/mesos/mesos.proto, line 91 > > <https://reviews.apache.org/r/4448/diff/2/?file=94646#file94646line91> > > > > why are we removing the default executor? is there a jira i can look at? We're removing the default executor to keep things simple. Now every TaskInfo requires either a CommandInfo or an ExecutorInfo. > On 2012-03-23 05:17:01, Vinod Kone wrote: > > src/common/type_utils.hpp, line 213 > > <https://reviews.apache.org/r/4448/diff/2/?file=94648#file94648line213> > > > > why are you only wrapping left.resources() but not right? because > > Resources class can compare directly with protobuf? probably its easier to > > grok if you just wrap them both? Agreed, I just have to line wrap now! ;) > On 2012-03-23 05:17:01, Vinod Kone wrote: > > src/master/master.cpp, line 1318 > > <https://reviews.apache.org/r/4448/diff/2/?file=94659#file94659line1318> > > > > it would be cleaner to just extract task.executor() into a variable There are lots of places where we don't have this style, so I'm not going to do it here for now. > On 2012-03-23 05:17:01, Vinod Kone wrote: > > src/master/master.cpp, line 1345 > > <https://reviews.apache.org/r/4448/diff/2/?file=94659#file94659line1345> > > > > s/task's/tasks Done, thanks! > On 2012-03-23 05:17:01, Vinod Kone wrote: > > src/master/master.cpp, line 1361 > > <https://reviews.apache.org/r/4448/diff/2/?file=94659#file94659line1361> > > > > when does this happen? a different slave has an executor with the same > > id? This was actually to catch for other tasks being processed simultaneously (i.e., in the same collection from SchedulerDriver::launchTasks) but it looks like we actually call Master::launchTask after each task rather than at the end so it doesn't look like we need this extra logic! > On 2012-03-23 05:17:01, Vinod Kone wrote: > > src/master/master.cpp, line 1706 > > <https://reviews.apache.org/r/4448/diff/2/?file=94659#file94659line1706> > > > > This could be fixed when we merge with the new slave restart code How nice! > On 2012-03-23 05:17:01, Vinod Kone wrote: > > src/slave/slave.hpp, line 297 > > <https://reviews.apache.org/r/4448/diff/2/?file=94667#file94667line297> > > > > can it have both? No, nor can it have neither. Changed to: CHECK(task.has_executor() != task.has_command()); > On 2012-03-23 05:17:01, Vinod Kone wrote: > > src/tests/resource_offers_tests.cpp, line 218 > > <https://reviews.apache.org/r/4448/diff/2/?file=94671#file94671line218> > > > > wait..what happened to task1's updates? Nothing, I just don't care about them, I only care about the updates that error! > On 2012-03-23 05:17:01, Vinod Kone wrote: > > src/tests/resource_offers_tests.cpp, line 221 > > <https://reviews.apache.org/r/4448/diff/2/?file=94671#file94671line221> > > > > this should be a string constant. While I agree that would be nice, none of the error strings are pulled out into constants so I'll wait for a full sweep to do that. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4448/#review6269 ----------------------------------------------------------- On 2012-03-23 04:42:17, Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4448/ > ----------------------------------------------------------- > > (Updated 2012-03-23 04:42:17) > > > Review request for mesos, Matei Zaharia, John Sirois, and Vinod Kone. > > > Summary > ------- > > See summary. > > > Diffs > ----- > > hadoop/hadoop-0.20.205.0.patch 909a41e > include/mesos/mesos.proto 478f561 > include/mesos/scheduler.hpp 34018f5 > src/common/type_utils.hpp efa1a84 > src/examples/java/TestExceptionFramework.java 6859c91 > src/examples/java/TestFramework.java fcfac40 > src/examples/java/TestMultipleExecutorsFramework.java 03a104b > src/examples/long_lived_framework.cpp 9ca49e5 > src/examples/no_executor_framework.cpp f6aa601 > src/examples/python/test_framework.py 388f7c0 > src/examples/test_framework.cpp f97d080 > src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 3857be3 > src/java/src/org/apache/mesos/MesosSchedulerDriver.java 590f9a2 > src/master/http.cpp 628112b > src/master/master.cpp 6ae8aef > src/python/native/mesos_executor_driver_impl.cpp 6e8fb9c > src/python/native/mesos_scheduler_driver_impl.cpp 4b142ce > src/python/native/module.hpp add021c > src/python/native/module.cpp 59d74a4 > src/python/native/proxy_executor.hpp aabb6c4 > src/python/native/proxy_scheduler.hpp a348cbb > src/sched/sched.cpp 43d9717 > src/slave/slave.hpp b7bc45a > src/tests/exception_tests.cpp 11214c2 > src/tests/fault_tolerance_tests.cpp 130218d > src/tests/master_tests.cpp 256945f > src/tests/resource_offers_tests.cpp 303a933 > src/tests/utils.hpp 8c038ce > src/webui/master/framework.tpl 9e384c1 > > Diff: https://reviews.apache.org/r/4448/diff > > > Testing > ------- > > make check && make hadoop > > > Thanks, > > Benjamin > >
