Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2016-01-13 Thread Klaus Ma 

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


ping @mcypark :).

- Klaus Ma


On Dec. 13, 2015, 11:28 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Dec. 13, 2015, 11:28 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f85 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 5211f54 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-12-14 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [37168]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 13, 2015, 3:28 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Dec. 13, 2015, 3:28 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9762f85 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 5211f54 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-12-13 Thread Klaus Ma 

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

(Updated Dec. 13, 2015, 11:28 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase the code; @mcypark, please help to review when you have time.


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


Repository: mesos


Description
---

Provide example for dynamic reservation features.


Diffs (updated)
-

  src/Makefile.am 9762f85 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 5211f54 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-11-09 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [37168]

All tests passed.

- Mesos ReviewBot


On Nov. 9, 2015, 8:10 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Nov. 9, 2015, 8:10 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am c479aca 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 3f56b30 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-11-09 Thread Klaus Ma 


> On Oct. 6, 2015, 7:12 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 120-123
> > 
> >
> > `reserveResources` returns an error when there are not enough resources 
> > available. While this is a necessary condition, it's not sufficient to 
> > assume that the reservation succeeded. We need to wait until the reserved 
> > resources are actually offered back to us before we can assume that the 
> > resources have been reserved.

I used to add `RESERVING` state to show the case that the framework did not get 
the reserved resources; but it seems not necessary: the `RESERVING` state will 
transfer to `RESERVED` and dispatch task, the code logic of `RESERVING` & 
`RESERVED` are same. Should we add `RESERVING` state to make it more readable 
or change the term to `RESERVE`?


> On Oct. 6, 2015, 7:12 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 126-131
> > 
> >
> > Based on how this reads, it looks like we always launch tasks on the 
> > offer if the agent's state is `RESERVED`. In reality, we only launch tasks 
> > if the agent is in a reserved state and there has not been a task launched 
> > on this agent yet. I think we should capture that by having something like:
> > 
> > ```
> > case State::RESERVED:
> >   if (/* no task have been launched on this agent yet &&
> >  the offer has suffcient resources to launch a task */) {
> > launchTask(driver, offer);
> >   }
> >   break;

Agree; for the condition `no task have been launched on this agent yet`, it's 
guantee by state of slave; and also check whether reserved resources contains 
task resource.


> On Oct. 6, 2015, 7:12 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 149-151
> > 
> >
> > Generally, having the same state be the initial state as well as the 
> > termination state and trying to differentiate between them is a bad idea. 
> > For example, we're assuming `states.empty()` is the termination state 
> > condition here but it's also the condition of the initial state. How do we 
> > know that we're not here because `offers` happened to be empty? Keeping the 
> > initial state and the termination state distinct will make the code easier 
> > to reason about and less error-prone.

Address by adding `tasksFinished == totalTasks`: when all tasks are done & 
states is empty, stop the driver.


> On Oct. 6, 2015, 7:12 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 243-244
> > 
> >
> > It looks weird to see `resources_.flatten(role, reservationInfo)` but 
> > `TASK_RESOURCES.flatten(role)`, what's the reason for that?
> > 
> > Also, `find` has funky behavior where it'll match resources that aren't 
> > exactly the thing that you specify to `find`. Is this funkiness accounted 
> > for? or do we just want to reserve `task_resources` if 
> > `resources.contains(task_resoures) == true`?

Update to check `resources.contains(task_resoures)`.


> On Oct. 6, 2015, 7:12 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 213-214
> > 
> >
> > This loop launches as many tasks as possible as long as they fit in the 
> > offered resources. I thought we only want to launch 1 task per agent and 
> > therefore 1 task per offer. I guess that behavior could be satisfied by 
> > making sure we only reserve 1 "task resources" worth of resources per 
> > agent, but we should capture that postcondition in here as a precondition.

Update to only dispatch 1 task to the reserved resources; and use 
`reserved.contains(taskResources)` to check whether got reserved resources by 
this framework.


> On Oct. 6, 2015, 7:12 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 338
> > 
> >
> > This logic is typically inlined in all other example frameworks. Any 
> > particular reason as to why this is factored out to a function? and why is 
> > it all caps?

OK :). Moved to `main()`


> On Oct. 6, 2015, 7:12 a.m., Michael Park wrote:
> > src/tests/flags.hpp, lines 155-158
> > 
> >
> > I think we don't need this since we have:
> > 
> > ```
> > os::setenv("MESOS_ROLES", flags.role.get());
> > ```
> > 
> > This seems to be how other example frameworks such as 
> > `persistent_volume_framework.cpp` have gotten away without this explicit 
> > `roles` flag.

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-11-09 Thread Klaus Ma 

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

(Updated Nov. 9, 2015, 4:10 p.m.)


Review request for mesos and Michael Park.


Changes
---

Address comments


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


Repository: mesos


Description
---

Provide example for dynamic reservation features.


Diffs (updated)
-

  src/Makefile.am c479aca 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 3f56b30 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-11-08 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [37168]

All tests passed.

- Mesos ReviewBot


On Nov. 9, 2015, 5:29 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Nov. 9, 2015, 5:29 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am c479aca 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 3f56b30 
>   src/tests/flags.hpp 984cd4a 
>   src/tests/script.cpp d2280c2 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-11-08 Thread Klaus Ma 

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

(Updated Nov. 9, 2015, 1:29 p.m.)


Review request for mesos and Michael Park.


Changes
---

Address comments


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


Repository: mesos


Description
---

Provide example for dynamic reservation features.


Diffs (updated)
-

  src/Makefile.am c479aca 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 3f56b30 
  src/tests/flags.hpp 984cd4a 
  src/tests/script.cpp d2280c2 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-09-14 Thread Klaus Ma 

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



src/Makefile.am (line 1463)


Done



src/examples/dynamic_reservation_framework.cpp (line 40)


Done



src/examples/dynamic_reservation_framework.cpp (line 49)


Done



src/examples/dynamic_reservation_framework.cpp (line 180)


Done



src/examples/dynamic_reservation_framework.cpp (line 183)


Done



src/examples/dynamic_reservation_framework.cpp (line 184)


Done



src/examples/dynamic_reservation_framework.cpp (line 281)


Done



src/examples/dynamic_reservation_framework.cpp (line 283)


Done


- Klaus Ma


On Sept. 6, 2015, 4:11 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Sept. 6, 2015, 4:11 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5fdca0f 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 3f56b30 
>   src/tests/flags.hpp 06da36d 
>   src/tests/script.cpp bcc1fab 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-09-14 Thread Klaus Ma 

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

(Updated Sept. 14, 2015, 1:48 p.m.)


Review request for mesos and Michael Park.


Changes
---

Address review comments


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


Repository: mesos


Description
---

Provide example for dynamic reservation features.


Diffs (updated)
-

  src/Makefile.am 8963cea 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 3f56b30 
  src/tests/flags.hpp 06da36d 
  src/tests/script.cpp bcc1fab 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-09-14 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [37168]

All tests passed.

- Mesos ReviewBot


On Sept. 14, 2015, 1:48 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Sept. 14, 2015, 1:48 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8963cea 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 3f56b30 
>   src/tests/flags.hpp 06da36d 
>   src/tests/script.cpp bcc1fab 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-09-12 Thread Michael Park 

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


Some preliminary comments.


src/examples/dynamic_reservation_framework.cpp (line 40)


Remove newline.



src/examples/dynamic_reservation_framework.cpp (line 49)


Not used.



src/examples/dynamic_reservation_framework.cpp (lines 100 - 102)


Shouldn't we also make sure we only reserve `TASK_RESOURCES` amount of 
resources on only `totalTasks` number of agents?



src/examples/dynamic_reservation_framework.cpp (lines 111 - 112)


It seems like we should stay in the `START` state until we've reserved 
`TASK_RESOURCES` amount of resources for this agent before transitioning to the 
`RESERVED` state, right?

Looking at the `reserveResources` function, it doesn't even perform the 
`acceptOffers` if the offer resources doesn't contain sufficient resources. 
Shouldn't the `state = State::RESERVED` be dependent on the result of 
`reserveResources`?



src/examples/dynamic_reservation_framework.cpp (line 180)


`s/sid/slaveId/`



src/examples/dynamic_reservation_framework.cpp (line 183)


`s/executorID/executorId/`



src/examples/dynamic_reservation_framework.cpp (line 184)


`s/slaveID/slaveId/`



src/examples/dynamic_reservation_framework.cpp (line 228)


Why is this called `remaining`? I would think the "remaining" portion is 
`TASK_RESOURCES` - amount of already reserved resources?



src/examples/dynamic_reservation_framework.cpp (line 229)


We should be careful here, as `find` does not do exact match. It can return 
resources with a non-matching role, for example.



src/examples/dynamic_reservation_framework.cpp (line 242)


Why does this function perform side-effects? I think this function should 
look exactly the same as the one in `src/tests/mesos.hpp`.



src/examples/dynamic_reservation_framework.cpp (line 252)


It doesn't makes sense to print this here as this function doesn't actually 
reserve any resources.



src/examples/dynamic_reservation_framework.cpp (line 257)


Same comment as `RESERVE`.


- Michael Park


On Sept. 6, 2015, 4:11 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Sept. 6, 2015, 4:11 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5fdca0f 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 3f56b30 
>   src/tests/flags.hpp 06da36d 
>   src/tests/script.cpp bcc1fab 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-09-08 Thread Klaus Ma 

Hi Michael,

Do you have more comments about this example?

On 2015年09月06日 21:28, Klaus Ma wrote:
This is an automatically generated e-mail. To reply, visit: 
https://reviews.apache.org/r/37168/



On September 6th, 2015, 8:49 a.m. UTC, *Joerg Schad* wrote:

src/Makefile.am


(Diff revision 4)


1463

Any reason for this blank line here?

It's not necessary, will delete it.


On September 6th, 2015, 8:49 a.m. UTC, *Joerg Schad* wrote:

src/examples/dynamic_reservation_framework.cpp


(Diff revision 4)


281 

 START,   // The framework get the offer for the first time.

s/get/receives

OK :).


On September 6th, 2015, 8:49 a.m. UTC, *Joerg Schad* wrote:

src/examples/dynamic_reservation_framework.cpp


(Diff revision 4)


283 

 TASK_DONE,   // All tasks are done.

s/TASK_DONE/TASKS_DONE


Please correct me if I am wrong: TASK_DONE is a global state
across all slaves, while the other states can differ per
state, or? Maybe just extend the comment a bit explaining this.

Yes, TASK_DONE is a global state; when all task finished, framework 
will un-reserve all resources.



On September 6th, 2015, 8:49 a.m. UTC, *Joerg Schad* wrote:

src/tests/dynamic_reservation_framework_test.sh


(Diff revision 4)


30  

export MESOS_ROLES="test"

Do we need this here and below in script.cpp?

Yes, it's necessary because: 1. dynamic reservation is role based 2. 
can not use default role for dynamic reservation, neither * nor 
MESOS_DEFAULT_ROLE



- Klaus


On September 6th, 2015, 4:11 a.m. UTC, Klaus Ma wrote:

Review request for mesos and Michael Park.
By Klaus Ma.

/Updated Sept. 6, 2015, 4:11 a.m./

*Bugs: * MESOS-3063 
*Repository: * mesos


  Description

Provide example for dynamic reservation features.


  Testing

make make check


  Diffs

  * src/Makefile.am (5fdca0f)
  * src/examples/dynamic_reservation_framework.cpp (PRE-CREATION)
  * src/tests/dynamic_reservation_framework_test.sh (PRE-CREATION)
  * src/tests/examples_tests.cpp (3f56b30)
  * src/tests/flags.hpp (06da36d)
  * src/tests/script.cpp (bcc1fab)

View Diff 



--
Klaus Ma (马达), PMP® | http://www.cguru.net

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-09-06 Thread Joerg Schad 

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



src/Makefile.am (line 1463)


Any reason for this blank line here?



src/examples/dynamic_reservation_framework.cpp (line 281)


s/get/receives



src/examples/dynamic_reservation_framework.cpp (line 283)


s/TASK_DONE/TASKS_DONE

Please correct me if I am wrong: TASK_DONE is a global state across all 
slaves, while the other states can differ per state, or? Maybe just extend the 
comment a bit explaining this.



src/tests/dynamic_reservation_framework_test.sh (line 30)


Do we need this here and below in script.cpp?


- Joerg Schad


On Sept. 6, 2015, 4:11 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Sept. 6, 2015, 4:11 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5fdca0f 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 3f56b30 
>   src/tests/flags.hpp 06da36d 
>   src/tests/script.cpp bcc1fab 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-09-06 Thread Klaus Ma 


> On Sept. 6, 2015, 8:49 a.m., Joerg Schad wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 283
> > 
> >
> > s/TASK_DONE/TASKS_DONE
> > 
> > Please correct me if I am wrong: TASK_DONE is a global state across all 
> > slaves, while the other states can differ per state, or? Maybe just extend 
> > the comment a bit explaining this.

Yes, TASK_DONE is a global state; when all task finished, framework will 
un-reserve all resources.


> On Sept. 6, 2015, 8:49 a.m., Joerg Schad wrote:
> > src/tests/dynamic_reservation_framework_test.sh, line 30
> > 
> >
> > Do we need this here and below in script.cpp?

Yes, it's necessary because:
1. dynamic reservation is role based
2. can not use default role for dynamic reservation, neither * nor 
MESOS_DEFAULT_ROLE


> On Sept. 6, 2015, 8:49 a.m., Joerg Schad wrote:
> > src/Makefile.am, line 1463
> > 
> >
> > Any reason for this blank line here?

It's not necessary, will delete it.


> On Sept. 6, 2015, 8:49 a.m., Joerg Schad wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 281
> > 
> >
> > s/get/receives

OK :).


- Klaus


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


On Sept. 6, 2015, 4:11 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Sept. 6, 2015, 4:11 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5fdca0f 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 3f56b30 
>   src/tests/flags.hpp 06da36d 
>   src/tests/script.cpp bcc1fab 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-09-05 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [37168]

All tests passed.

- Mesos ReviewBot


On Sept. 6, 2015, 4:11 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated Sept. 6, 2015, 4:11 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5fdca0f 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 3f56b30 
>   src/tests/flags.hpp 06da36d 
>   src/tests/script.cpp bcc1fab 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-09-05 Thread Klaus Ma 

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

(Updated Sept. 6, 2015, 4:11 a.m.)


Review request for mesos and Michael Park.


Changes
---

Update the code diff to address commnets:
* Add comments to describe the example logic
* Use command executor instead of test_executor.cpp


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


Repository: mesos


Description
---

Provide example for dynamic reservation features.


Diffs (updated)
-

  src/Makefile.am 5fdca0f 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 3f56b30 
  src/tests/flags.hpp 06da36d 
  src/tests/script.cpp bcc1fab 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-09-04 Thread Klaus Ma 

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

(Updated Sept. 5, 2015, 3:14 a.m.)


Review request for mesos and Michael Park.


Changes
---

Update summary & description


Summary (updated)
-

MESOS-3063 (Add an example framework using dynamic reservation)


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


Repository: mesos


Description (updated)
---

Provide example for dynamic reservation features.


Diffs
-

  src/Makefile.am 7b620ff 
  src/examples/dynamic_reservation_executor.cpp PRE-CREATION 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 3f56b30 
  src/tests/flags.hpp 3644956 
  src/tests/script.cpp bcc1fab 

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


Testing
---

make
make check


Thanks,

Klaus Ma