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/05/21 05:00:00 UTC

Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

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

Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.


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


Repository: mesos-git


Description
-------

Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.


Diffs
-----

  src/master/master.cpp 075755cad5c50a57c92d7d82f2466b467796f673 
  src/tests/master_tests.cpp 1ea1da685420aeee4cbed4765d7cfdf31fc7231f 

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


Testing
-------

make check


Thanks,

Vinod Kone


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

Posted by Vinod Kone <vi...@gmail.com>.

> On May 21, 2014, 3:33 a.m., Ben Mahler wrote:
> > Higher level comment, why didn't you update removeOffer to make the necessary call on the allocator?
> 
> Ben Mahler wrote:
>     To be more clear, why not have a 'useOffer' and a 'discardOffer' for the two cases we care about?

added a TODO for now.


> On May 21, 2014, 3:33 a.m., Ben Mahler wrote:
> > src/master/master.cpp, line 1633
> > <https://reviews.apache.org/r/21750/diff/1/?file=585964#file585964line1633>
> >
> >     Ditto about a comment here on why we can CHECK this as it wasn't immediately obvious to me.

see below.


> On May 21, 2014, 3:33 a.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1654-1658
> > <https://reviews.apache.org/r/21750/diff/1/?file=585964#file585964line1654>
> >
> >     Not yours, but we should probably have a comment here saying that we can CHECK these things because at this point the ValidOfferChecker will have already rejected it.
> >     
> >     Likewise, we should also have a comment where we instantiate the vector of offer visitors to describe that the order is essential to get right (because of the CHECKs!).
> >     
> >     Last thing, would be nice if these were just singletons since they have no state (they can't be purely static because we want the virtual aspect of the functions). Allocating them on the heap via 'new' every time a launch task comes in seems fairly wasteful and unnecessarily complicates the code :(. I guess a TODO for this would be nice, same for TaskInfoVisitors :)

i removed the CHECK because it depends on the order of validators which i don't like.


> On May 21, 2014, 3:33 a.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1661-1663
> > <https://reviews.apache.org/r/21750/diff/1/?file=585964#file585964line1661>
> >
> >     Would love to see a comment as to why we can CHECK this (i.e. because we remove offers for disconnected slaves).

this is actually a bug. https://issues.apache.org/jira/browse/MESOS-1418 i will fix this in another (dependent) patch.


> On May 21, 2014, 3:33 a.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1812-1818
> > <https://reviews.apache.org/r/21750/diff/1/?file=585964#file585964line1812>
> >
> >     this isn't indented properly

the outer "if" wasn't indented properly afaict. fixed.


> On May 21, 2014, 3:33 a.m., Ben Mahler wrote:
> > src/master/master.cpp, line 1814
> > <https://reviews.apache.org/r/21750/diff/1/?file=585964#file585964line1814>
> >
> >     Shouldn't "offers" be singular in this sentence? Maybe the "// Remove offers." comment could be amended to reflect the need to recover resources?

fixed.


> On May 21, 2014, 3:33 a.m., Ben Mahler wrote:
> > src/master/master.cpp, line 1824
> > <https://reviews.apache.org/r/21750/diff/1/?file=585964#file585964line1824>
> >
> >     This isn't lined up, do we need the space before the ":"?

done


> On May 21, 2014, 3:33 a.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1810-1811
> > <https://reviews.apache.org/r/21750/diff/1/?file=585964#file585964line1810>
> >
> >     Is this comment correct? Seems to me that we need this check also because it's possible that the offer id is invalid?

hmm. doesn't look like it. killed it.


> On May 21, 2014, 3:33 a.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1809-1818
> > <https://reviews.apache.org/r/21750/diff/1/?file=585964#file585964line1809>
> >
> >     Some cleanup notes for posterity, the getOffer helper in the master code seems messier in general:
> >     
> >     Offer* offer = getOffer(offerId);
> >     if (offer != NULL) {
> >       removeOffer(offer);
> >       // Now 'offer' is pointing to deleted memory!!!
> >     }
> >     
> >     vs.
> >     
> >     if (offers.contains(offerId)) {
> >       removeOffer(offerId);
> >     }
> >     
> >     But oh well for now :)

there is an existing TODO to kill getOffer, so I'll punt on it for now.


- Vinod


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


On May 21, 2014, 3 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21750/
> -----------------------------------------------------------
> 
> (Updated May 21, 2014, 3 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1400
>     https://issues.apache.org/jira/browse/MESOS-1400
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 075755cad5c50a57c92d7d82f2466b467796f673 
>   src/tests/master_tests.cpp 1ea1da685420aeee4cbed4765d7cfdf31fc7231f 
> 
> Diff: https://reviews.apache.org/r/21750/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

Posted by Ben Mahler <be...@gmail.com>.

> On May 21, 2014, 3:33 a.m., Ben Mahler wrote:
> > Higher level comment, why didn't you update removeOffer to make the necessary call on the allocator?

To be more clear, why not have a 'useOffer' and a 'discardOffer' for the two cases we care about?


- Ben


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


On May 21, 2014, 3 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21750/
> -----------------------------------------------------------
> 
> (Updated May 21, 2014, 3 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1400
>     https://issues.apache.org/jira/browse/MESOS-1400
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 075755cad5c50a57c92d7d82f2466b467796f673 
>   src/tests/master_tests.cpp 1ea1da685420aeee4cbed4765d7cfdf31fc7231f 
> 
> Diff: https://reviews.apache.org/r/21750/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

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


Higher level comment, why didn't you update removeOffer to make the necessary call on the allocator?


src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment77835>

    Ditto about a comment here on why we can CHECK this as it wasn't immediately obvious to me.



src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment77834>

    Not yours, but we should probably have a comment here saying that we can CHECK these things because at this point the ValidOfferChecker will have already rejected it.
    
    Likewise, we should also have a comment where we instantiate the vector of offer visitors to describe that the order is essential to get right (because of the CHECKs!).
    
    Last thing, would be nice if these were just singletons since they have no state (they can't be purely static because we want the virtual aspect of the functions). Allocating them on the heap via 'new' every time a launch task comes in seems fairly wasteful and unnecessarily complicates the code :(. I guess a TODO for this would be nice, same for TaskInfoVisitors :)



src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment77832>

    Would love to see a comment as to why we can CHECK this (i.e. because we remove offers for disconnected slaves).



src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment77839>

    Some cleanup notes for posterity, the getOffer helper in the master code seems messier in general:
    
    Offer* offer = getOffer(offerId);
    if (offer != NULL) {
      removeOffer(offer);
      // Now 'offer' is pointing to deleted memory!!!
    }
    
    vs.
    
    if (offers.contains(offerId)) {
      removeOffer(offerId);
    }
    
    But oh well for now :)



src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment77841>

    Is this comment correct? Seems to me that we need this check also because it's possible that the offer id is invalid?



src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment77838>

    this isn't indented properly



src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment77837>

    Shouldn't "offers" be singular in this sentence? Maybe the "// Remove offers." comment could be amended to reflect the need to recover resources?



src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment77840>

    This isn't lined up, do we need the space before the ":"?


- Ben Mahler


On May 21, 2014, 3 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21750/
> -----------------------------------------------------------
> 
> (Updated May 21, 2014, 3 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1400
>     https://issues.apache.org/jira/browse/MESOS-1400
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 075755cad5c50a57c92d7d82f2466b467796f673 
>   src/tests/master_tests.cpp 1ea1da685420aeee4cbed4765d7cfdf31fc7231f 
> 
> Diff: https://reviews.apache.org/r/21750/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21750/#review43701
-----------------------------------------------------------


Thanks for fixing / cleaning this Vinod (and keeping me in the loop)! The fix looks good to me, but I'll let BenM shoot you the ship-it :)

- Niklas Nielsen


On May 20, 2014, 8 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21750/
> -----------------------------------------------------------
> 
> (Updated May 20, 2014, 8 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1400
>     https://issues.apache.org/jira/browse/MESOS-1400
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 075755cad5c50a57c92d7d82f2466b467796f673 
>   src/tests/master_tests.cpp 1ea1da685420aeee4cbed4765d7cfdf31fc7231f 
> 
> Diff: https://reviews.apache.org/r/21750/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21750/#review43632
-----------------------------------------------------------



src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment77903>

    This could maybe roll into my change to pull the visitors out of master.cpp: https://reviews.apache.org/r/20423/


- Dominic Hamon


On May 20, 2014, 8 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21750/
> -----------------------------------------------------------
> 
> (Updated May 20, 2014, 8 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1400
>     https://issues.apache.org/jira/browse/MESOS-1400
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 075755cad5c50a57c92d7d82f2466b467796f673 
>   src/tests/master_tests.cpp 1ea1da685420aeee4cbed4765d7cfdf31fc7231f 
> 
> Diff: https://reviews.apache.org/r/21750/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

Posted by Vinod Kone <vi...@gmail.com>.

> On May 28, 2014, 10:02 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 3700-3702
> > <https://reviews.apache.org/r/21750/diff/2/?file=595659#file595659line3700>
> >
> >     More specifically, I think it might be more readable to have multiple methods here, rather than one method with boolean flags:
> >     
> >     useOffer(offerId);
> >     rescindOffer(offerId);
> >     discardOffer(offerId);
> >     
> >     Because adding more booleans here is problematic, unless you used an enum to denote how to remove it: USE, DISCARD, RESCIND. Maybe more guidance in the TODO?

agreed. amended the TODO.


- Vinod


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


On May 29, 2014, 12:17 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21750/
> -----------------------------------------------------------
> 
> (Updated May 29, 2014, 12:17 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1400
>     https://issues.apache.org/jira/browse/MESOS-1400
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp efb4de1216df3fccf3706a5c615960ec00434440 
>   src/tests/master_tests.cpp b0ff6277120636fe9045b12443e5e2dd29a3df1b 
> 
> Diff: https://reviews.apache.org/r/21750/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

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

Ship it!


Looks good thanks!


src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment78508>

    Don't think you need any of these 'stringify' calls.



src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment78509>

    More specifically, I think it might be more readable to have multiple methods here, rather than one method with boolean flags:
    
    useOffer(offerId);
    rescindOffer(offerId);
    discardOffer(offerId);
    
    Because adding more booleans here is problematic, unless you used an enum to denote how to remove it: USE, DISCARD, RESCIND. Maybe more guidance in the TODO?


- Ben Mahler


On May 28, 2014, 2:03 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21750/
> -----------------------------------------------------------
> 
> (Updated May 28, 2014, 2:03 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1400
>     https://issues.apache.org/jira/browse/MESOS-1400
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 21472e94658be6e31f13eacc5412144d62b12096 
>   src/tests/master_tests.cpp f607d89b17b9347cd4fb3a715d268e59559de1f6 
> 
> Diff: https://reviews.apache.org/r/21750/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21750/#review48919
-----------------------------------------------------------



src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment85702>

    Ah got it, sorry I think I didn't check careful enough.
    
    Thanks!


- Timothy Chen


On May 29, 2014, 12:17 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21750/
> -----------------------------------------------------------
> 
> (Updated May 29, 2014, 12:17 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1400
>     https://issues.apache.org/jira/browse/MESOS-1400
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp efb4de1216df3fccf3706a5c615960ec00434440 
>   src/tests/master_tests.cpp b0ff6277120636fe9045b12443e5e2dd29a3df1b 
> 
> Diff: https://reviews.apache.org/r/21750/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

Posted by Ben Mahler <be...@gmail.com>.

> On July 28, 2014, 9:23 p.m., Timothy Chen wrote:
> > src/master/master.cpp, line 1679
> > <https://reviews.apache.org/r/21750/diff/3/?file=598185#file598185line1679>
> >
> >     When offers are removed (which I think is the only path of how Offer can no longer exist), the offer object is also being deleted as well. Therefore, I think if we keep using OfferID& that is referencing the offerId from the offer object it is a undefined behavior.

Is there a bug here?

The offerIds that are getting passed into the offer visitors are not references inside the 'Offer' heap object, they are parameters to launchTasks() coming from the parsed 'LaunchTasksMessage' protobuf.


- Ben


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


On May 29, 2014, 12:17 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21750/
> -----------------------------------------------------------
> 
> (Updated May 29, 2014, 12:17 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1400
>     https://issues.apache.org/jira/browse/MESOS-1400
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp efb4de1216df3fccf3706a5c615960ec00434440 
>   src/tests/master_tests.cpp b0ff6277120636fe9045b12443e5e2dd29a3df1b 
> 
> Diff: https://reviews.apache.org/r/21750/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21750/#review48908
-----------------------------------------------------------



src/master/master.cpp
<https://reviews.apache.org/r/21750/#comment85686>

    When offers are removed (which I think is the only path of how Offer can no longer exist), the offer object is also being deleted as well. Therefore, I think if we keep using OfferID& that is referencing the offerId from the offer object it is a undefined behavior.


- Timothy Chen


On May 29, 2014, 12:17 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21750/
> -----------------------------------------------------------
> 
> (Updated May 29, 2014, 12:17 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1400
>     https://issues.apache.org/jira/browse/MESOS-1400
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp efb4de1216df3fccf3706a5c615960ec00434440 
>   src/tests/master_tests.cpp b0ff6277120636fe9045b12443e5e2dd29a3df1b 
> 
> Diff: https://reviews.apache.org/r/21750/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

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

(Updated May 29, 2014, 12:17 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.


Changes
-------

rebased and comments. NNFR.


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


Repository: mesos-git


Description
-------

Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.


Diffs (updated)
-----

  src/master/master.cpp efb4de1216df3fccf3706a5c615960ec00434440 
  src/tests/master_tests.cpp b0ff6277120636fe9045b12443e5e2dd29a3df1b 

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


Testing
-------

make check


Thanks,

Vinod Kone


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

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

(Updated May 28, 2014, 2:03 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.


Changes
-------

readded depends on.


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


Repository: mesos-git


Description
-------

Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.


Diffs
-----

  src/master/master.cpp 21472e94658be6e31f13eacc5412144d62b12096 
  src/tests/master_tests.cpp f607d89b17b9347cd4fb3a715d268e59559de1f6 

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


Testing
-------

make check


Thanks,

Vinod Kone


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

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

(Updated May 28, 2014, 1:09 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.


Changes
-------

comments.


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


Repository: mesos-git


Description
-------

Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.


Diffs (updated)
-----

  src/master/master.cpp 21472e94658be6e31f13eacc5412144d62b12096 
  src/tests/master_tests.cpp f607d89b17b9347cd4fb3a715d268e59559de1f6 

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


Testing
-------

make check


Thanks,

Vinod Kone


Re: Review Request 21750: Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.

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

(Updated May 21, 2014, 3 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.


Changes
-------

added "depends on".


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


Repository: mesos-git


Description
-------

Fixed Master::launchTasks() to inform allocator of unused resources when any of the offers are invalid.


Diffs
-----

  src/master/master.cpp 075755cad5c50a57c92d7d82f2466b467796f673 
  src/tests/master_tests.cpp 1ea1da685420aeee4cbed4765d7cfdf31fc7231f 

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


Testing
-------

make check


Thanks,

Vinod Kone