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


As discussed, we should add consolidation of tasks in the master against the 
tasks the slave re-registers with.


src/slave/slave.cpp
<https://reviews.apache.org/r/10604/#comment40144>

    Can you check against the slaveId in the TaskInfo to ensure it matches?



src/slave/slave.cpp
<https://reviews.apache.org/r/10604/#comment40125>

    And now to also ensure the framework is not removed, right?



src/slave/slave.cpp
<https://reviews.apache.org/r/10604/#comment40126>

    This NOTE is not possible any longer, right?
    
    We can CHECKNOTNULL on the framework, because a runTask would have inserted 
a pending task, thus preventing framework removal in the interim.



src/slave/slave.cpp
<https://reviews.apache.org/r/10604/#comment40143>

    Check that we're not in recovering here. And handle RECOVERING in runTask 
instead.



src/slave/slave.cpp
<https://reviews.apache.org/r/10604/#comment40124>

    Looks like createExecutor is not needed any longer, can you merge it into 
launchExecutor?


- Ben Mahler


On April 18, 2013, 9:11 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10604/
> -----------------------------------------------------------
> 
> (Updated April 18, 2013, 9:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Description
> -------
> 
> Refactored runTask() and some other pieces of slave, to make this hopefully 
> clear.
> 
> Also, sneaked in some bug fixes when executorStarted() is called.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.hpp 54c66863db217077a050dc414caf0976447500be 
>   src/slave/slave.cpp 00b2375505e362959ac34061e3066cf8ace96adf 
>   src/tests/allocator_zookeeper_tests.cpp 
> 42faaa067bdfa0c7f33260eb5cb3b9e5956c3037 
> 
> Diff: https://reviews.apache.org/r/10604/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> NOTE: GarbageCollectorIntegrationTest.Unschedule test now correctly verifies 
> that executors/frameworks are properly unscheduled despite adding tasks to 
> 'pending'.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to