You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2014/03/29 00:15:34 UTC

Review Request 19809: Fixed a bug in master to properly reconcile completed frameworks during slave re-registration.

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

Review request for mesos, Adam Berry, Benjamin Hindman, and Ben Mahler.


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


Repository: mesos-git


Description
-------

See bug for details.


Diffs
-----

  src/master/master.hpp eb8cd73e9e8f26a7438070ca2bb078bd2d1ec480 
  src/master/master.cpp 5d0ddb0a567b1282bf164a5f1abc41a719208269 
  src/tests/fault_tolerance_tests.cpp 68280113ca32c4c4e0f335bed11044ba4184c34e 

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


Testing
-------

make check


Thanks,

Vinod Kone


Re: Review Request 19809: Fixed a bug in master to properly reconcile completed frameworks during slave re-registration.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19809/
-----------------------------------------------------------

(Updated March 28, 2014, 11:58 p.m.)


Review request for mesos, Adam Berry, Benjamin Hindman, and Ben Mahler.


Changes
-------

Addressed comments. NNFR.


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


Repository: mesos-git


Description
-------

See bug for details.


Diffs (updated)
-----

  src/master/master.hpp eb8cd73e9e8f26a7438070ca2bb078bd2d1ec480 
  src/master/master.cpp 5d0ddb0a567b1282bf164a5f1abc41a719208269 
  src/tests/fault_tolerance_tests.cpp 68280113ca32c4c4e0f335bed11044ba4184c34e 

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


Testing
-------

make check


Thanks,

Vinod Kone


Re: Review Request 19809: Fixed a bug in master to properly reconcile completed frameworks during slave re-registration.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19809/#review38967
-----------------------------------------------------------

Ship it!



src/master/master.hpp
<https://reviews.apache.org/r/19809/#comment71347>

    Let's move it back down for now and address the "slow tests" problem for all the tests at once.



src/master/master.cpp
<https://reviews.apache.org/r/19809/#comment71346>

    & ?



src/tests/fault_tolerance_tests.cpp
<https://reviews.apache.org/r/19809/#comment71344>

    Let's fix the "slowness due to retries" problem for all the tests in one swoop rather than for this specific test.



src/tests/fault_tolerance_tests.cpp
<https://reviews.apache.org/r/19809/#comment71343>

    Why the dynamic casts instead of what we do in, say, MasterTest.MasterInfoOnReElection?


- Ben Mahler


On March 28, 2014, 11:15 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19809/
> -----------------------------------------------------------
> 
> (Updated March 28, 2014, 11:15 p.m.)
> 
> 
> Review request for mesos, Adam Berry, Benjamin Hindman, and Ben Mahler.
> 
> 
> Bugs: MESOS-1161
>     https://issues.apache.org/jira/browse/MESOS-1161
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See bug for details.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp eb8cd73e9e8f26a7438070ca2bb078bd2d1ec480 
>   src/master/master.cpp 5d0ddb0a567b1282bf164a5f1abc41a719208269 
>   src/tests/fault_tolerance_tests.cpp 68280113ca32c4c4e0f335bed11044ba4184c34e 
> 
> Diff: https://reviews.apache.org/r/19809/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 19809: Fixed a bug in master to properly reconcile completed frameworks during slave re-registration.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19809/#review38966
-----------------------------------------------------------

Ship it!



src/master/master.hpp
<https://reviews.apache.org/r/19809/#comment71340>

    What does "moved up" mean? Not everybody does public then protected then private. How about:
    
    // This is made 'public' for testing.
    
    Are there a collection of these that this can go next to?



src/master/master.cpp
<https://reviews.apache.org/r/19809/#comment71341>

    I'll suggest adding more debug output for when we don't add completed tasks. At the very least, update the comment so future readers recognize that you're okay with dropping the completed tasks when a framework hasn't yet registered. I'd also add a TODO to clean this up once we have the registrar storing frameworks because then we can make sure we always have a Framework*.



src/tests/fault_tolerance_tests.cpp
<https://reviews.apache.org/r/19809/#comment71342>

    I'd like us to be even more explicit here for posterity, how about the following amendment:
    
    ... from a slave's re-registration when the slave thinks the framework has completed (but the framework has not actually completed yet).


- Benjamin Hindman


On March 28, 2014, 11:15 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19809/
> -----------------------------------------------------------
> 
> (Updated March 28, 2014, 11:15 p.m.)
> 
> 
> Review request for mesos, Adam Berry, Benjamin Hindman, and Ben Mahler.
> 
> 
> Bugs: MESOS-1161
>     https://issues.apache.org/jira/browse/MESOS-1161
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See bug for details.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp eb8cd73e9e8f26a7438070ca2bb078bd2d1ec480 
>   src/master/master.cpp 5d0ddb0a567b1282bf164a5f1abc41a719208269 
>   src/tests/fault_tolerance_tests.cpp 68280113ca32c4c4e0f335bed11044ba4184c34e 
> 
> Diff: https://reviews.apache.org/r/19809/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>