You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Yifan Gu <yi...@mesosphere.io> on 2014/06/13 07:27:25 UTC

Re: Review Request 22526: WIP:Added resizeTask primitive.

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

(Updated June 13, 2014, 5:27 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Summary (updated)
-----------------

WIP:Added resizeTask primitive.


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


Repository: mesos-git


Description
-------

Added resizeTask primitive.

This is just a proof of concept now. I will work on the unit test.
Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
I put the result in the 'data' field of the TaskStatus.

And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.

Any question or suggestion is highly welcome! Thanks!


Diffs
-----

  include/mesos/mesos.proto 102289b 
  src/common/protobuf_utils.hpp 12ff00a 
  src/master/master.hpp 7a12185 
  src/master/master.cpp 4a01b1a 
  src/messages/messages.proto 8aecc8b 
  src/slave/slave.hpp 34687e5 
  src/slave/slave.cpp 643c088 

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


Testing
-------

make check.


Thanks,

Yifan Gu


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/#review45574
-----------------------------------------------------------


Patch looks great!

Reviews applied: [22526]

All tests passed.

- Mesos ReviewBot


On June 13, 2014, 5:27 a.m., Yifan Gu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22526/
> -----------------------------------------------------------
> 
> (Updated June 13, 2014, 5:27 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-1279
>     https://issues.apache.org/jira/browse/MESOS-1279
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added resizeTask primitive.
> 
> This is just a proof of concept now. I will work on the unit test.
> Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
> I put the result in the 'data' field of the TaskStatus.
> 
> And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.
> 
> Any question or suggestion is highly welcome! Thanks!
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 102289b 
>   src/common/protobuf_utils.hpp 12ff00a 
>   src/master/master.hpp 7a12185 
>   src/master/master.cpp 4a01b1a 
>   src/messages/messages.proto 8aecc8b 
>   src/slave/slave.hpp 34687e5 
>   src/slave/slave.cpp 643c088 
> 
> Diff: https://reviews.apache.org/r/22526/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Yifan Gu
> 
>


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/#review45664
-----------------------------------------------------------



include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/22526/#comment80567>

    Made it an optional field so the master won't need to fill this field when sending it to the slave.


- Yifan Gu


On June 13, 2014, 9:58 p.m., Yifan Gu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22526/
> -----------------------------------------------------------
> 
> (Updated June 13, 2014, 9:58 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-1279
>     https://issues.apache.org/jira/browse/MESOS-1279
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added resizeTask primitive.
> 
> This is just a proof of concept now. I will work on the unit test.
> Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
> I put the result in the 'data' field of the TaskStatus.
> 
> And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.
> 
> Any question or suggestion is highly welcome! Thanks!
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 102289b 
>   include/mesos/scheduler.hpp d224945 
>   include/mesos/scheduler/scheduler.proto 6ab5089 
>   src/common/protobuf_utils.hpp 12ff00a 
>   src/master/master.hpp 7a12185 
>   src/master/master.cpp 4a01b1a 
>   src/messages/messages.proto 8aecc8b 
>   src/sched/sched.cpp 6e14f1c 
>   src/scheduler/scheduler.cpp 4ae188e 
>   src/slave/slave.hpp 34687e5 
>   src/slave/slave.cpp 643c088 
>   src/tests/master_tests.cpp 34df121 
> 
> Diff: https://reviews.apache.org/r/22526/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Yifan Gu
> 
>


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/#review45973
-----------------------------------------------------------


Patch looks great!

Reviews applied: [22526]

All tests passed.

- Mesos ReviewBot


On June 17, 2014, 6:23 p.m., Yifan Gu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22526/
> -----------------------------------------------------------
> 
> (Updated June 17, 2014, 6:23 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-1279
>     https://issues.apache.org/jira/browse/MESOS-1279
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added resizeTask primitive.
> 
> This is just a proof of concept now. I will work on the unit test.
> Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
> I put the result in the 'data' field of the TaskStatus.
> 
> And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.
> 
> Any question or suggestion is highly welcome! Thanks!
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 102289b 
>   include/mesos/scheduler.hpp d224945 
>   include/mesos/scheduler/scheduler.proto 6ab5089 
>   src/Makefile.am c91b438 
>   src/common/protobuf_utils.hpp 12ff00a 
>   src/master/master.hpp 7a12185 
>   src/master/master.cpp 4a01b1a 
>   src/messages/messages.proto 8aecc8b 
>   src/sched/sched.cpp 6e14f1c 
>   src/scheduler/scheduler.cpp 4ae188e 
>   src/slave/slave.hpp 34687e5 
>   src/slave/slave.cpp 643c088 
>   src/tests/resize_task_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22526/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Yifan Gu
> 
>


Re: Review Request 22526: WIP:Added resizeTask primitive.

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


I don't think Yifan is going to complete this work. I am going to discard for now if you don't have any objections.

- Niklas Nielsen


On June 17, 2014, 11:23 a.m., Yifan Gu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22526/
> -----------------------------------------------------------
> 
> (Updated June 17, 2014, 11:23 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-1279
>     https://issues.apache.org/jira/browse/MESOS-1279
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added resizeTask primitive.
> 
> This is just a proof of concept now. I will work on the unit test.
> Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
> I put the result in the 'data' field of the TaskStatus.
> 
> And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.
> 
> Any question or suggestion is highly welcome! Thanks!
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 102289b 
>   include/mesos/scheduler.hpp d224945 
>   include/mesos/scheduler/scheduler.proto 6ab5089 
>   src/Makefile.am c91b438 
>   src/common/protobuf_utils.hpp 12ff00a 
>   src/master/master.hpp 7a12185 
>   src/master/master.cpp 4a01b1a 
>   src/messages/messages.proto 8aecc8b 
>   src/sched/sched.cpp 6e14f1c 
>   src/scheduler/scheduler.cpp 4ae188e 
>   src/slave/slave.hpp 34687e5 
>   src/slave/slave.cpp 643c088 
>   src/tests/resize_task_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22526/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Yifan Gu
> 
>


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.

> On June 19, 2014, 7:42 a.m., Adam B wrote:
> > Great start. I didn't get into the tests at all, and only skimmed over the rest of the code, but here are a few comments from my first pass.
> > 
> > Since this is a huge diff (>1500 new/modified lines), how about splitting it up into multiple reviews/phases. Here are a few splits I can think of:
> > - Refactor launchTask to pull out common logic needed by resizeTask
> > - Add scheduler API (returns error in master, does nothing in slave)
> > - Add master logic and slave API (returns error in slave)
> > - Add slave logic
> > - Add master logic to verify/handleResizeTaskReply
> > - Add metrics
> > - Add tests

Great suggestion! Will do! Thanks for reviewing!!


> On June 19, 2014, 7:42 a.m., Adam B wrote:
> > include/mesos/mesos.proto, line 497
> > <https://reviews.apache.org/r/22526/diff/11/?file=611738#file611738line497>
> >
> >     I worry that some code (ours or clients) may assume that a state > TASK_RUNNING is terminal. I know that hasn't been true since the creation of TASK_STAGING, but it's a general concern of mine. Perhaps it would have been better to have started the TERMINAL states at 10 or 100.

hmm, I checked "inline bool isTerminalState(const TaskState& state)" in  src/common/protobuf_utils.cpp, it checks the state individually, so I think if we use that function else where to check the TERMINAL state, then it would be fine.


> On June 19, 2014, 7:42 a.m., Adam B wrote:
> > include/mesos/mesos.proto, line 526
> > <https://reviews.apache.org/r/22526/diff/11/?file=611738#file611738line526>
> >
> >     Why a boolean instead of an optional error message? Then you could do if(resizeTaskReply.has_error()) { LOG(ERROR) << resizeTaskReply.error(); } else { //do something with resizeTaskReply }

Cool! That will be sweet! Thanks!


> On June 19, 2014, 7:42 a.m., Adam B wrote:
> > include/mesos/scheduler/scheduler.proto, lines 183-186
> > <https://reviews.apache.org/r/22526/diff/11/?file=611740#file611740line183>
> >
> >     Where did '2' go?

somewhere..


> On June 19, 2014, 7:42 a.m., Adam B wrote:
> > src/master/master.cpp, lines 3368-3370
> > <https://reviews.apache.org/r/22526/diff/11/?file=611744#file611744line3368>
> >
> >     Do we still want to report the StatusUpdate to the framework if the master is already up to date?

Yes we do. if the function return Nothing(), then the message will be forwarded.


> On June 19, 2014, 7:42 a.m., Adam B wrote:
> > src/slave/slave.cpp, line 1384
> > <https://reviews.apache.org/r/22526/diff/11/?file=611749#file611749line1384>
> >
> >     Should you maybe check the future first, so you can send back an error ResizeTaskReply message rather than silently aborting the resize?
> >     Maybe some of these errors/returns should actually send an error ResizeTaskReply too?

True. I think if the slave is terminating, it should send back the reply.


> On June 19, 2014, 7:42 a.m., Adam B wrote:
> > src/scheduler/scheduler.cpp, line 371
> > <https://reviews.apache.org/r/22526/diff/11/?file=611747#file611747line371>
> >
> >     send(master.get(), message); ?

lol..


- Yifan


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


On June 17, 2014, 6:23 p.m., Yifan Gu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22526/
> -----------------------------------------------------------
> 
> (Updated June 17, 2014, 6:23 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-1279
>     https://issues.apache.org/jira/browse/MESOS-1279
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added resizeTask primitive.
> 
> This is just a proof of concept now. I will work on the unit test.
> Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
> I put the result in the 'data' field of the TaskStatus.
> 
> And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.
> 
> Any question or suggestion is highly welcome! Thanks!
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 102289b 
>   include/mesos/scheduler.hpp d224945 
>   include/mesos/scheduler/scheduler.proto 6ab5089 
>   src/Makefile.am c91b438 
>   src/common/protobuf_utils.hpp 12ff00a 
>   src/master/master.hpp 7a12185 
>   src/master/master.cpp 4a01b1a 
>   src/messages/messages.proto 8aecc8b 
>   src/sched/sched.cpp 6e14f1c 
>   src/scheduler/scheduler.cpp 4ae188e 
>   src/slave/slave.hpp 34687e5 
>   src/slave/slave.cpp 643c088 
>   src/tests/resize_task_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22526/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Yifan Gu
> 
>


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.

> On June 19, 2014, 7:42 a.m., Adam B wrote:
> > include/mesos/mesos.proto, lines 527-528
> > <https://reviews.apache.org/r/22526/diff/11/?file=611738#file611738line527>
> >
> >     Why do you need both old & new resources? In case multiple resize tasks come through at a time, or to handle resource changes during slave failovers or other tasks starting/completing?

Yes, when a master fails over, or multiple resize tasks come through. the master may not be able to remember if a task is to be scaled up or down.
Any better solutions?


> On June 19, 2014, 7:42 a.m., Adam B wrote:
> > include/mesos/scheduler/scheduler.proto, lines 183-186
> > <https://reviews.apache.org/r/22526/diff/11/?file=611740#file611740line183>
> >
> >     Where did '2' go?
> 
> Yifan Gu wrote:
>     somewhere..

This was a typo


> On June 19, 2014, 7:42 a.m., Adam B wrote:
> > src/scheduler/scheduler.cpp, line 371
> > <https://reviews.apache.org/r/22526/diff/11/?file=611747#file611747line371>
> >
> >     send(master.get(), message); ?
> 
> Yifan Gu wrote:
>     lol..

need to add the missing piece: send message back to master.


- Yifan


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


On June 17, 2014, 6:23 p.m., Yifan Gu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22526/
> -----------------------------------------------------------
> 
> (Updated June 17, 2014, 6:23 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-1279
>     https://issues.apache.org/jira/browse/MESOS-1279
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added resizeTask primitive.
> 
> This is just a proof of concept now. I will work on the unit test.
> Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
> I put the result in the 'data' field of the TaskStatus.
> 
> And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.
> 
> Any question or suggestion is highly welcome! Thanks!
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 102289b 
>   include/mesos/scheduler.hpp d224945 
>   include/mesos/scheduler/scheduler.proto 6ab5089 
>   src/Makefile.am c91b438 
>   src/common/protobuf_utils.hpp 12ff00a 
>   src/master/master.hpp 7a12185 
>   src/master/master.cpp 4a01b1a 
>   src/messages/messages.proto 8aecc8b 
>   src/sched/sched.cpp 6e14f1c 
>   src/scheduler/scheduler.cpp 4ae188e 
>   src/slave/slave.hpp 34687e5 
>   src/slave/slave.cpp 643c088 
>   src/tests/resize_task_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22526/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Yifan Gu
> 
>


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/#review46192
-----------------------------------------------------------


Great start. I didn't get into the tests at all, and only skimmed over the rest of the code, but here are a few comments from my first pass.

Since this is a huge diff (>1500 new/modified lines), how about splitting it up into multiple reviews/phases. Here are a few splits I can think of:
- Refactor launchTask to pull out common logic needed by resizeTask
- Add scheduler API (returns error in master, does nothing in slave)
- Add master logic and slave API (returns error in slave)
- Add slave logic
- Add master logic to verify/handleResizeTaskReply
- Add metrics
- Add tests


include/mesos/mesos.proto
<https://reviews.apache.org/r/22526/#comment81475>

    I worry that some code (ours or clients) may assume that a state > TASK_RUNNING is terminal. I know that hasn't been true since the creation of TASK_STAGING, but it's a general concern of mine. Perhaps it would have been better to have started the TERMINAL states at 10 or 100.



include/mesos/mesos.proto
<https://reviews.apache.org/r/22526/#comment81476>

    Why a boolean instead of an optional error message? Then you could do if(resizeTaskReply.has_error()) { LOG(ERROR) << resizeTaskReply.error(); } else { //do something with resizeTaskReply }



include/mesos/mesos.proto
<https://reviews.apache.org/r/22526/#comment81477>

    Why do you need both old & new resources? In case multiple resize tasks come through at a time, or to handle resource changes during slave failovers or other tasks starting/completing?



include/mesos/scheduler.hpp
<https://reviews.apache.org/r/22526/#comment81478>

    embedded



include/mesos/scheduler.hpp
<https://reviews.apache.org/r/22526/#comment81479>

    s/is succeeded or not/succeeded/



include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/22526/#comment81480>

    Where did '2' go?



src/master/master.hpp
<https://reviews.apache.org/r/22526/#comment81481>

    s/res/resources/



src/master/master.hpp
<https://reviews.apache.org/r/22526/#comment81483>

    s/message invalid/message is invalid/



src/master/master.cpp
<https://reviews.apache.org/r/22526/#comment81485>

    I see what you mean about all the duplicate checker code between here and launchTask. Perhaps refactoring launchTask would be a good precursor review.



src/master/master.cpp
<https://reviews.apache.org/r/22526/#comment81486>

    We generally prefer CopyFrom unless MergeFrom is explicitly needed.



src/master/master.cpp
<https://reviews.apache.org/r/22526/#comment81484>

    Do we still want to report the StatusUpdate to the framework if the master is already up to date?



src/scheduler/scheduler.cpp
<https://reviews.apache.org/r/22526/#comment81487>

    send(master.get(), message); ?



src/slave/slave.hpp
<https://reviews.apache.org/r/22526/#comment81488>

    Does _resizeTask need to be public too?



src/slave/slave.cpp
<https://reviews.apache.org/r/22526/#comment81489>

    Should you maybe check the future first, so you can send back an error ResizeTaskReply message rather than silently aborting the resize?
    Maybe some of these errors/returns should actually send an error ResizeTaskReply too?


- Adam B


On June 17, 2014, 11:23 a.m., Yifan Gu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22526/
> -----------------------------------------------------------
> 
> (Updated June 17, 2014, 11:23 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-1279
>     https://issues.apache.org/jira/browse/MESOS-1279
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added resizeTask primitive.
> 
> This is just a proof of concept now. I will work on the unit test.
> Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
> I put the result in the 'data' field of the TaskStatus.
> 
> And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.
> 
> Any question or suggestion is highly welcome! Thanks!
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 102289b 
>   include/mesos/scheduler.hpp d224945 
>   include/mesos/scheduler/scheduler.proto 6ab5089 
>   src/Makefile.am c91b438 
>   src/common/protobuf_utils.hpp 12ff00a 
>   src/master/master.hpp 7a12185 
>   src/master/master.cpp 4a01b1a 
>   src/messages/messages.proto 8aecc8b 
>   src/sched/sched.cpp 6e14f1c 
>   src/scheduler/scheduler.cpp 4ae188e 
>   src/slave/slave.hpp 34687e5 
>   src/slave/slave.cpp 643c088 
>   src/tests/resize_task_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22526/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Yifan Gu
> 
>


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/
-----------------------------------------------------------

(Updated June 17, 2014, 6:23 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Changes
-------

Fixed a bug when master failover occurs.
TODO: Related tests on this.


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


Repository: mesos-git


Description
-------

Added resizeTask primitive.

This is just a proof of concept now. I will work on the unit test.
Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
I put the result in the 'data' field of the TaskStatus.

And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.

Any question or suggestion is highly welcome! Thanks!


Diffs (updated)
-----

  include/mesos/mesos.proto 102289b 
  include/mesos/scheduler.hpp d224945 
  include/mesos/scheduler/scheduler.proto 6ab5089 
  src/Makefile.am c91b438 
  src/common/protobuf_utils.hpp 12ff00a 
  src/master/master.hpp 7a12185 
  src/master/master.cpp 4a01b1a 
  src/messages/messages.proto 8aecc8b 
  src/sched/sched.cpp 6e14f1c 
  src/scheduler/scheduler.cpp 4ae188e 
  src/slave/slave.hpp 34687e5 
  src/slave/slave.cpp 643c088 
  src/tests/resize_task_tests.cpp PRE-CREATION 

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


Testing
-------

make check.


Thanks,

Yifan Gu


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/
-----------------------------------------------------------

(Updated June 17, 2014, 12:39 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Changes
-------

Added a test to test that when the executor is REGISTERING, the containerizer won't be updated.


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


Repository: mesos-git


Description
-------

Added resizeTask primitive.

This is just a proof of concept now. I will work on the unit test.
Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
I put the result in the 'data' field of the TaskStatus.

And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.

Any question or suggestion is highly welcome! Thanks!


Diffs (updated)
-----

  include/mesos/mesos.proto 102289b 
  include/mesos/scheduler.hpp d224945 
  include/mesos/scheduler/scheduler.proto 6ab5089 
  src/Makefile.am c91b438 
  src/common/protobuf_utils.hpp 12ff00a 
  src/master/master.hpp 7a12185 
  src/master/master.cpp 4a01b1a 
  src/messages/messages.proto 8aecc8b 
  src/sched/sched.cpp 6e14f1c 
  src/scheduler/scheduler.cpp 4ae188e 
  src/slave/slave.hpp 34687e5 
  src/slave/slave.cpp 643c088 
  src/tests/resize_task_tests.cpp PRE-CREATION 

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


Testing
-------

make check.


Thanks,

Yifan Gu


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/
-----------------------------------------------------------

(Updated June 17, 2014, 12:01 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.


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


Repository: mesos-git


Description
-------

Added resizeTask primitive.

This is just a proof of concept now. I will work on the unit test.
Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
I put the result in the 'data' field of the TaskStatus.

And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.

Any question or suggestion is highly welcome! Thanks!


Diffs (updated)
-----

  include/mesos/mesos.proto 102289b 
  include/mesos/scheduler.hpp d224945 
  include/mesos/scheduler/scheduler.proto 6ab5089 
  src/Makefile.am c91b438 
  src/common/protobuf_utils.hpp 12ff00a 
  src/master/master.hpp 7a12185 
  src/master/master.cpp 4a01b1a 
  src/messages/messages.proto 8aecc8b 
  src/sched/sched.cpp 6e14f1c 
  src/scheduler/scheduler.cpp 4ae188e 
  src/slave/slave.hpp 34687e5 
  src/slave/slave.cpp 643c088 
  src/tests/resize_task_tests.cpp PRE-CREATION 

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


Testing
-------

make check.


Thanks,

Yifan Gu


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/
-----------------------------------------------------------

(Updated June 16, 2014, 11:58 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Changes
-------

Put tests in resize_task_tests.

Added more tests.


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


Repository: mesos-git


Description
-------

Added resizeTask primitive.

This is just a proof of concept now. I will work on the unit test.
Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
I put the result in the 'data' field of the TaskStatus.

And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.

Any question or suggestion is highly welcome! Thanks!


Diffs (updated)
-----

  include/mesos/mesos.proto 102289b 
  include/mesos/scheduler.hpp d224945 
  include/mesos/scheduler/scheduler.proto 6ab5089 
  src/Makefile.am c91b438 
  src/common/protobuf_utils.hpp 12ff00a 
  src/master/master.hpp 7a12185 
  src/master/master.cpp 4a01b1a 
  src/messages/messages.proto 8aecc8b 
  src/sched/sched.cpp 6e14f1c 
  src/scheduler/scheduler.cpp 4ae188e 
  src/slave/slave.hpp 34687e5 
  src/slave/slave.cpp 643c088 
  src/tests/allocator_tests.cpp 7ad4964 
  src/tests/resize_task_tests.cpp PRE-CREATION 

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


Testing
-------

make check.


Thanks,

Yifan Gu


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/#review45700
-----------------------------------------------------------



src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/22526/#comment80654>

    This takes some time.
    But if i advance 1s of the clock, then it fails get the offer in 10s. Need to find a way to improve this.


- Yifan Gu


On June 14, 2014, 8:59 p.m., Yifan Gu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22526/
> -----------------------------------------------------------
> 
> (Updated June 14, 2014, 8:59 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-1279
>     https://issues.apache.org/jira/browse/MESOS-1279
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added resizeTask primitive.
> 
> This is just a proof of concept now. I will work on the unit test.
> Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
> I put the result in the 'data' field of the TaskStatus.
> 
> And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.
> 
> Any question or suggestion is highly welcome! Thanks!
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 102289b 
>   include/mesos/scheduler.hpp d224945 
>   include/mesos/scheduler/scheduler.proto 6ab5089 
>   src/common/protobuf_utils.hpp 12ff00a 
>   src/master/master.hpp 7a12185 
>   src/master/master.cpp 4a01b1a 
>   src/messages/messages.proto 8aecc8b 
>   src/sched/sched.cpp 6e14f1c 
>   src/scheduler/scheduler.cpp 4ae188e 
>   src/slave/slave.hpp 34687e5 
>   src/slave/slave.cpp 643c088 
>   src/tests/allocator_tests.cpp 7ad4964 
>   src/tests/master_tests.cpp 34df121 
> 
> Diff: https://reviews.apache.org/r/22526/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Yifan Gu
> 
>


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/
-----------------------------------------------------------

(Updated June 14, 2014, 8:59 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Changes
-------

Checked the update of the containerizer.


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


Repository: mesos-git


Description
-------

Added resizeTask primitive.

This is just a proof of concept now. I will work on the unit test.
Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
I put the result in the 'data' field of the TaskStatus.

And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.

Any question or suggestion is highly welcome! Thanks!


Diffs (updated)
-----

  include/mesos/mesos.proto 102289b 
  include/mesos/scheduler.hpp d224945 
  include/mesos/scheduler/scheduler.proto 6ab5089 
  src/common/protobuf_utils.hpp 12ff00a 
  src/master/master.hpp 7a12185 
  src/master/master.cpp 4a01b1a 
  src/messages/messages.proto 8aecc8b 
  src/sched/sched.cpp 6e14f1c 
  src/scheduler/scheduler.cpp 4ae188e 
  src/slave/slave.hpp 34687e5 
  src/slave/slave.cpp 643c088 
  src/tests/allocator_tests.cpp 7ad4964 
  src/tests/master_tests.cpp 34df121 

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


Testing
-------

make check.


Thanks,

Yifan Gu


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/
-----------------------------------------------------------

(Updated June 14, 2014, 10:57 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Changes
-------

Added a test to test the result of successful scale-up/scale-down. 
TODO: Test the container's update message.


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


Repository: mesos-git


Description
-------

Added resizeTask primitive.

This is just a proof of concept now. I will work on the unit test.
Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
I put the result in the 'data' field of the TaskStatus.

And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.

Any question or suggestion is highly welcome! Thanks!


Diffs (updated)
-----

  include/mesos/mesos.proto 102289b 
  include/mesos/scheduler.hpp d224945 
  include/mesos/scheduler/scheduler.proto 6ab5089 
  src/common/protobuf_utils.hpp 12ff00a 
  src/master/master.hpp 7a12185 
  src/master/master.cpp 4a01b1a 
  src/messages/messages.proto 8aecc8b 
  src/sched/sched.cpp 6e14f1c 
  src/scheduler/scheduler.cpp 4ae188e 
  src/slave/slave.hpp 34687e5 
  src/slave/slave.cpp 643c088 
  src/tests/allocator_tests.cpp 7ad4964 
  src/tests/master_tests.cpp 34df121 

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


Testing
-------

make check.


Thanks,

Yifan Gu


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/
-----------------------------------------------------------

(Updated June 13, 2014, 11:45 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Changes
-------

Changed offerIds as optional, since we should be scale down without offerIds.


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


Repository: mesos-git


Description
-------

Added resizeTask primitive.

This is just a proof of concept now. I will work on the unit test.
Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
I put the result in the 'data' field of the TaskStatus.

And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.

Any question or suggestion is highly welcome! Thanks!


Diffs (updated)
-----

  include/mesos/mesos.proto 102289b 
  include/mesos/scheduler.hpp d224945 
  include/mesos/scheduler/scheduler.proto 6ab5089 
  src/common/protobuf_utils.hpp 12ff00a 
  src/master/master.hpp 7a12185 
  src/master/master.cpp 4a01b1a 
  src/messages/messages.proto 8aecc8b 
  src/sched/sched.cpp 6e14f1c 
  src/scheduler/scheduler.cpp 4ae188e 
  src/slave/slave.hpp 34687e5 
  src/slave/slave.cpp 643c088 
  src/tests/master_tests.cpp 34df121 

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


Testing
-------

make check.


Thanks,

Yifan Gu


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/
-----------------------------------------------------------

(Updated June 13, 2014, 9:58 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Changes
-------

Added a test for not enough resource.
Put tests into one.


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


Repository: mesos-git


Description
-------

Added resizeTask primitive.

This is just a proof of concept now. I will work on the unit test.
Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
I put the result in the 'data' field of the TaskStatus.

And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.

Any question or suggestion is highly welcome! Thanks!


Diffs (updated)
-----

  include/mesos/mesos.proto 102289b 
  include/mesos/scheduler.hpp d224945 
  include/mesos/scheduler/scheduler.proto 6ab5089 
  src/common/protobuf_utils.hpp 12ff00a 
  src/master/master.hpp 7a12185 
  src/master/master.cpp 4a01b1a 
  src/messages/messages.proto 8aecc8b 
  src/sched/sched.cpp 6e14f1c 
  src/scheduler/scheduler.cpp 4ae188e 
  src/slave/slave.hpp 34687e5 
  src/slave/slave.cpp 643c088 
  src/tests/master_tests.cpp 34df121 

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


Testing
-------

make check.


Thanks,

Yifan Gu


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/
-----------------------------------------------------------

(Updated June 13, 2014, 9:02 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Changes
-------

Added invalid offer tests.


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


Repository: mesos-git


Description
-------

Added resizeTask primitive.

This is just a proof of concept now. I will work on the unit test.
Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
I put the result in the 'data' field of the TaskStatus.

And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.

Any question or suggestion is highly welcome! Thanks!


Diffs (updated)
-----

  include/mesos/mesos.proto 102289b 
  include/mesos/scheduler.hpp d224945 
  include/mesos/scheduler/scheduler.proto 6ab5089 
  src/common/protobuf_utils.hpp 12ff00a 
  src/master/master.hpp 7a12185 
  src/master/master.cpp 4a01b1a 
  src/messages/messages.proto 8aecc8b 
  src/sched/sched.cpp 6e14f1c 
  src/scheduler/scheduler.cpp 4ae188e 
  src/slave/slave.hpp 34687e5 
  src/slave/slave.cpp 643c088 
  src/tests/master_tests.cpp 34df121 

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


Testing
-------

make check.


Thanks,

Yifan Gu


Re: Review Request 22526: WIP:Added resizeTask primitive.

Posted by Yifan Gu <yi...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22526/
-----------------------------------------------------------

(Updated June 13, 2014, 8:06 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and Vinod Kone.


Changes
-------

Added API in scheduler and sched.
Added one unitest.
To be continued.


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


Repository: mesos-git


Description
-------

Added resizeTask primitive.

This is just a proof of concept now. I will work on the unit test.
Currently I added one state called "TASK_RESIZE" in state update, so that the master/framework can get the resize result from the slave.
I put the result in the 'data' field of the TaskStatus.

And I feel that I copy-pasted a lot of checkers, which is kind mess, I think they should be put into a separate module later.

Any question or suggestion is highly welcome! Thanks!


Diffs (updated)
-----

  include/mesos/mesos.proto 102289b 
  include/mesos/scheduler.hpp d224945 
  include/mesos/scheduler/scheduler.proto 6ab5089 
  src/common/protobuf_utils.hpp 12ff00a 
  src/master/master.hpp 7a12185 
  src/master/master.cpp 4a01b1a 
  src/messages/messages.proto 8aecc8b 
  src/sched/sched.cpp 6e14f1c 
  src/scheduler/scheduler.cpp 4ae188e 
  src/slave/slave.hpp 34687e5 
  src/slave/slave.cpp 643c088 
  src/tests/master_tests.cpp 34df121 

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


Testing
-------

make check.


Thanks,

Yifan Gu