Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-07 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [37168]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 8, 2016, 2:33 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 8, 2016, 2:33 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 71c4308cccfa5c56b93f6c3928dd2a1cf3ba9741 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-07 Thread Klaus Ma 

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

(Updated April 8, 2016, 10:33 a.m.)


Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.


Changes
---

Address @mcypark's comments and rebase.


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 71c4308cccfa5c56b93f6c3928dd2a1cf3ba9741 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-07 Thread Michael Park 

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


Fix it, then Ship it!





src/examples/dynamic_reservation_framework.cpp (lines 389 - 392)


We need to remove this in order for this to work on OS X.

Refer to https://reviews.apache.org/r/39423.


- Michael Park


On April 7, 2016, 3:23 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 7, 2016, 3:23 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 71c4308cccfa5c56b93f6c3928dd2a1cf3ba9741 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-06 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [37168]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 7, 2016, 3:23 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 7, 2016, 3:23 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 71c4308cccfa5c56b93f6c3928dd2a1cf3ba9741 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-06 Thread haosdent huang 


> On April 6, 2016, 4:38 p.m., haosdent huang wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 181
> > 
> >
> > How about add a default for unexpected state? May just log the error 
> > status.
> 
> Klaus Ma wrote:
> @haosdent, I think we should avoid `default` in `switch`; so compiler 
> will check whether we missed any unexpected state.

Got it. Thanks for explanation. :-)


- haosdent


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


On April 7, 2016, 3:23 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 7, 2016, 3:23 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 71c4308cccfa5c56b93f6c3928dd2a1cf3ba9741 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-06 Thread Klaus Ma 

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

(Updated April 7, 2016, 11:23 a.m.)


Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.


Changes
---

Address haosdent's 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 71c4308cccfa5c56b93f6c3928dd2a1cf3ba9741 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-06 Thread Klaus Ma 


> On April 7, 2016, 12:38 a.m., haosdent huang wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 181
> > 
> >
> > How about add a default for unexpected state? May just log the error 
> > status.

@haosdent, I think we should avoid `default` in `switch`; so compiler will 
check whether we missed any unexpected state.


- Klaus


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


On April 6, 2016, 4:19 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 6, 2016, 4:19 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-06 Thread haosdent huang 

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




src/Makefile.am (line 2038)


How about make these sort by alphabetically?



src/examples/dynamic_reservation_framework.cpp (line 46)


Why we need this as we have already `using namespace mesos`?



src/examples/dynamic_reservation_framework.cpp (line 181)


How about add a default for unexpected state? May just log the error status.



src/examples/dynamic_reservation_framework.cpp (line 295)


How about adjust the order of variable here? For example, enum defination 
comes first, then static variable, member variable.



src/examples/dynamic_reservation_framework.cpp (line 366)


I think it would better to use
```
cerr << flags.usage('Missing --role') << endl;
```


- haosdent huang


On April 6, 2016, 8:19 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 6, 2016, 8:19 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-06 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [37168]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 6, 2016, 8:19 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 6, 2016, 8:19 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-06 Thread Klaus Ma 

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

(Updated April 6, 2016, 4:19 p.m.)


Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.


Changes
---

Address @mcypark's 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 f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-06 Thread Klaus Ma 


> On April 4, 2016, 9:40 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 69-70
> > 
> >
> > Initialize these in member init list.
> > 
> > ```cpp
> >   : ...
> > principal(_principal),
> > reservationInfo(createReservationInfo(principal)),
> > taskResources(TASK_RESOURCES(role, reservationInfo))
> > ```

`createReservationInfo` is also included in `test/mesos.hpp`, I'd like to avoid 
such dependency; so initialize it in constructor. I'd like to drop this 
comments, please feel free to re-open it if more comments :).


- Klaus


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


On April 5, 2016, 5:28 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 5, 2016, 5:28 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-06 Thread Klaus Ma 


> On April 4, 2016, 9:40 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 308
> > 
> >
> > `static const Resources TASK_RESOURCES;`
> 
> Klaus Ma wrote:
> If we mark it `const`, we have to move the following code into 
> constructor's init, is that OK?
> 
> ```
> Resources::parse(
> "cpus:" + stringify(CPUS_PER_TASK) +
> ";mem:" + stringify(MEM_PER_TASK)).get();
> ```

Fixed. My bad memory, static const member is ok to assign later.


- Klaus


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


On April 5, 2016, 5:28 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 5, 2016, 5:28 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-05 Thread Michael Park 


> On April 4, 2016, 1:40 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 311-320
> > 
> >
> > Conceptutally, `Try` is not something we pass around like this. A 
> > function returning a `Try` is emulating a function that throws an 
> > exception. So if a `Try` results in an error, we need to handle it 
> > immediately, or propagate the error. We do not pass it along to a 
> > subsequent function.
> > 
> > There are other issues here as well. Even if we were to pass it around, 
> > it should have been passed by `const &`. What does `rc` stand for?
> > 
> > Functionally speaking, this says that if `rc` is an error, we log 
> > saying we failed to update the state of an agent. But `reserveResources` 
> > returns `Error` if the offer doesn't contain sufficient amount of 
> > resources. This has really little to do with "failing to update the state 
> > of an agent."
> 
> Klaus Ma wrote:
> Thanks very much for your explaintation on `Try`; that's clear to me :).
> 
> BTW, `rc` means `return code`.

I see. Thanks.


> On April 4, 2016, 1:40 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 350-364
> > 
> >
> > We should be able to just use the ones in `src/tests/mesos.hpp`.
> 
> Klaus Ma wrote:
> I'd like to de-couple example from Mesos internal code, e.g. 
> `tests/mesos.hpp`. Ideally, the user can build example with installed Mesos 
> binaries & headers.

I see. Ok, we can keep these then. Thanks!


- Michael


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


On April 5, 2016, 9:28 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 5, 2016, 9:28 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-05 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [37168]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 5, 2016, 9:28 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated April 5, 2016, 9:28 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-05 Thread Klaus Ma 

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

(Updated April 5, 2016, 5:28 p.m.)


Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.


Changes
---

Address @mcypark's 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 f22ae5b3bd9336a56c802e0e51d39d6cb675caf2 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-04 Thread Klaus Ma 


> On April 4, 2016, 9:40 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 308
> > 
> >
> > `static const Resources TASK_RESOURCES;`

If we mark it `const`, we have to move the following code into constructor's 
init, is that OK?

```
Resources::parse(
"cpus:" + stringify(CPUS_PER_TASK) +
";mem:" + stringify(MEM_PER_TASK)).get();
```


- Klaus


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


On March 22, 2016, 11:44 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 22, 2016, 11:44 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-04 Thread Klaus Ma 


> On April 4, 2016, 9:40 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 311-320
> > 
> >
> > Conceptutally, `Try` is not something we pass around like this. A 
> > function returning a `Try` is emulating a function that throws an 
> > exception. So if a `Try` results in an error, we need to handle it 
> > immediately, or propagate the error. We do not pass it along to a 
> > subsequent function.
> > 
> > There are other issues here as well. Even if we were to pass it around, 
> > it should have been passed by `const &`. What does `rc` stand for?
> > 
> > Functionally speaking, this says that if `rc` is an error, we log 
> > saying we failed to update the state of an agent. But `reserveResources` 
> > returns `Error` if the offer doesn't contain sufficient amount of 
> > resources. This has really little to do with "failing to update the state 
> > of an agent."

Thanks very much for your explaintation on `Try`; that's clear to me :).

BTW, `rc` means `return code`.


- Klaus


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


On March 22, 2016, 11:44 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 22, 2016, 11:44 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-04 Thread Klaus Ma 


> On April 4, 2016, 9:40 a.m., Michael Park wrote:
> > src/examples/dynamic_reservation_framework.cpp, lines 350-364
> > 
> >
> > We should be able to just use the ones in `src/tests/mesos.hpp`.

I'd like to de-couple example from Mesos internal code, e.g. `tests/mesos.hpp`. 
Ideally, the user can build example with installed Mesos binaries & headers.


- Klaus


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


On March 22, 2016, 11:44 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 22, 2016, 11:44 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-04-03 Thread Michael Park 

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




src/examples/dynamic_reservation_framework.cpp (line 52)


`s/slave/agent/`
`s/when offered to framework/when they are offered to the framework/`



src/examples/dynamic_reservation_framework.cpp (lines 69 - 70)


Initialize these in member init list.

```cpp
  : ...
principal(_principal),
reservationInfo(createReservationInfo(principal)),
taskResources(TASK_RESOURCES(role, reservationInfo))
```



src/examples/dynamic_reservation_framework.cpp (lines 90 - 91)


```cpp
LOG(INFO) << "Received offer " << offer.id() << " with "
  << offer.resources();
```



src/examples/dynamic_reservation_framework.cpp (line 93)


`s/If framework/If the framework/`



src/examples/dynamic_reservation_framework.cpp (line 94)


Replace quotes around `State::INIT` with backticks.



src/examples/dynamic_reservation_framework.cpp (line 97)


`s/did not/do not/`
`s/waiting/wait/`



src/examples/dynamic_reservation_framework.cpp (line 106)


`const State`



src/examples/dynamic_reservation_framework.cpp (lines 109 - 117)


We format cases with scopes like this:
```cpp
case State::INIT: {
  /* ... */
  break;
}
```

Here and below.



src/examples/dynamic_reservation_framework.cpp (lines 114 - 115)


I think we can get rid of `reserveResources` and `updateState` and just do 
the following. See below for I don't think these are useful abstractions.
```cpp
const Resources& resources = offer.resources();
Resources unreserved = resources.unreserved();

if (!unreserved.contains(TASK_RESOURCES)) {
  LOG(INFO) << "Insufficient resources for task in offer " + 
stringify(offer.id());
  break;
}

driver->acceptOffers({offer.id()}, {RESERVE(taskResources)});
states[offer.slave_id()] = State::RESERVING;
break;
```



src/examples/dynamic_reservation_framework.cpp (lines 121 - 144)


How about the following control flow?

```cpp
const Resources& resources = offer.resources();

...

case State::RESERVING: {
  Resources reserved = resources.reserved(role);
  if (reserved.contains(taskResources)) {
states[offer.slave_id()] = State::RESERVED;
  }
  // We fallthorugh here to save an offer cycle.
  // [[fallthrough]]
}
case State::RESERVED: {
  Resources reserved = resources.reserved(role);
  
  if (tasksLaunched == totalTasks) {
/* unreserve resources */
break;
  }

  CHECK(reserved.contains(taskResources));
  CHECK(tasksLaunched < totalTasks);
  /* launch tasks */
  break;
}
```



src/examples/dynamic_reservation_framework.cpp (lines 146 - 147)


```cpp
LOG(INFO) << "The task on " << offer.slave_id()
  << " is running, waiting for task done";
```



src/examples/dynamic_reservation_framework.cpp (line 154)


```cpp
states[offer.slave_id()] = State::UNRESERVED;
```



src/examples/dynamic_reservation_framework.cpp (line 187)


`++tasksFinished;`



src/examples/dynamic_reservation_framework.cpp (line 189)


```cpp
CHECK(states[status.slave_id()] == State::TASK_RUNNING);
states[status.slave_id()] = State::RESERVED;
```



src/examples/dynamic_reservation_framework.cpp (lines 191 - 192)


```cpp
LOG(INFO) << "Task " << taskId << " is finished at slave "
  << status.slave_id();
```



src/examples/dynamic_reservation_framework.cpp (lines 200 - 204)


```cpp
LOG(INFO) << "Aborting because task " << taskId
  << " is in unexpected state " << status.state()
  << " with reason " << status.reason()
  << " from source " << status.source()
  << " with message '" << status.message() << "'";
```



src/examples

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-24 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [37168]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 22, 2016, 3:44 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 22, 2016, 3:44 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-24 Thread Klaus Ma 

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



ping @mcypark, would you help to review it again?

- Klaus Ma


On March 22, 2016, 11:44 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 22, 2016, 11:44 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-21 Thread Klaus Ma 

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

(Updated March 22, 2016, 11:44 a.m.)


Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.


Changes
---

Address Greg's 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-21 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [37168]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 18, 2016, 6:47 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 18, 2016, 6:47 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-21 Thread Greg Mann 

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


Fix it, then Ship it!





src/examples/dynamic_reservation_framework.cpp (line 118)


s/launch task/launch a task/



src/examples/dynamic_reservation_framework.cpp (line 120)


s/lanuch/launch/


- Greg Mann


On March 18, 2016, 6:47 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 18, 2016, 6:47 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-19 Thread Klaus Ma 


> On March 17, 2016, 2:27 a.m., Greg Mann wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 119
> > 
> >
> > Could you clarify for me how the slave gets into the RESERVED state the 
> > first time? It seems to me we should have something here similar to what's 
> > in the switch for `State::UNRESERVING`, i.e., check the offer and see if 
> > the reserved resources are contained in it? The framework test passes for 
> > me though, so it seems to be working nonetheless?
> 
> Klaus Ma wrote:
> Good catch! I think I missed `updateState(dispatched, offer.slave_id(), 
> State::RESERVED);` just before launching task; it works because both 
> `State::RESERVED` and `State::RESERVING` are using the same logic.
> 
> When slave transfer from `State::RESERVING` to `State::RESERVED`, it's 
> better to launch tasks directly; otherwise, we have to wait for another 
> allocation interval to launch task. I used to try following code to seperate 
> action in those two state, but `case State::RESERVING:` without `break;` is 
> not a good style :).
> 
> ```
> case State::RESERVING:
>   {
> Resources resources = offer.resources();
> Resources reserved = resources.reserved(role);
> 
> if (reserved.contains(taskResources)) {
>   updateState(dispatched, offer.slave_id(), State::RESERVED);
> }
>   }
>   // NOTE: No `break;`. Launch task after transfering to
>   // `State::RESERVED`.
> case State::RESERVED:
>   {
> if (tasksLaunched == totalTasks) {
>   // If all tasks were launched, unreserve those resources.
>   Try reserved = unreserveResources(driver, offer);
>   updateState(reserved, offer.slave_id(), State::UNRESERVING);
> } else if (tasksLaunched < totalTasks) {
>   // Framework dispatches task on the reserved resources.
>   Try dispatched = dispatchTasks(driver, offer);
>   updateState(dispatched, offer.slave_id(), 
> State::TASK_RUNNING);
> }
>   }
>   break;
> ```
> 
> Greg Mann wrote:
> Ah yea, this makes sense :-) Thanks for the explanation! Do you think you 
> could add a comment to the code here to explain this reasoning?

Agree; updated the patches for that :).


- Klaus


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


On March 18, 2016, 2:47 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 18, 2016, 2:47 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-19 Thread Greg Mann 

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




src/examples/dynamic_reservation_framework.cpp (line 32)


Looks like this can be removed?



src/examples/dynamic_reservation_framework.cpp (line 119)


Could you clarify for me how the slave gets into the RESERVED state the 
first time? It seems to me we should have something here similar to what's in 
the switch for `State::UNRESERVING`, i.e., check the offer and see if the 
reserved resources are contained in it? The framework test passes for me 
though, so it seems to be working nonetheless?


- Greg Mann


On March 16, 2016, 6:32 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 16, 2016, 6:32 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-19 Thread Klaus Ma 


> On March 17, 2016, 2:27 a.m., Greg Mann wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 119
> > 
> >
> > Could you clarify for me how the slave gets into the RESERVED state the 
> > first time? It seems to me we should have something here similar to what's 
> > in the switch for `State::UNRESERVING`, i.e., check the offer and see if 
> > the reserved resources are contained in it? The framework test passes for 
> > me though, so it seems to be working nonetheless?

Good catch! I think I missed `updateState(dispatched, offer.slave_id(), 
State::RESERVED);` just before launching task; it works because both 
`State::RESERVED` and `State::RESERVING` are using the same logic.

When slave transfer from `State::RESERVING` to `State::RESERVED`, it's better 
to launch tasks directly; otherwise, we have to wait for another allocation 
interval to launch task. I used to try following code to seperate action in 
those two state, but `case State::RESERVING:` without `break;` is not a good 
style :).

```
case State::RESERVING:
  {
Resources resources = offer.resources();
Resources reserved = resources.reserved(role);

if (reserved.contains(taskResources)) {
  updateState(dispatched, offer.slave_id(), State::RESERVED);
}
  }
  // NOTE: No `break;`. Launch task after transfering to
  // `State::RESERVED`.
case State::RESERVED:
  {
if (tasksLaunched == totalTasks) {
  // If all tasks were launched, unreserve those resources.
  Try reserved = unreserveResources(driver, offer);
  updateState(reserved, offer.slave_id(), State::UNRESERVING);
} else if (tasksLaunched < totalTasks) {
  // Framework dispatches task on the reserved resources.
  Try dispatched = dispatchTasks(driver, offer);
  updateState(dispatched, offer.slave_id(), State::TASK_RUNNING);
}
  }
  break;
```


- Klaus


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


On March 16, 2016, 2:32 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 16, 2016, 2:32 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-19 Thread Klaus Ma 

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

(Updated March 18, 2016, 2:47 p.m.)


Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.


Changes
---

Add comments on `RESERVING` and `RESERVED` handling.


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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-18 Thread Greg Mann 


> On March 16, 2016, 6:27 p.m., Greg Mann wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 119
> > 
> >
> > Could you clarify for me how the slave gets into the RESERVED state the 
> > first time? It seems to me we should have something here similar to what's 
> > in the switch for `State::UNRESERVING`, i.e., check the offer and see if 
> > the reserved resources are contained in it? The framework test passes for 
> > me though, so it seems to be working nonetheless?
> 
> Klaus Ma wrote:
> Good catch! I think I missed `updateState(dispatched, offer.slave_id(), 
> State::RESERVED);` just before launching task; it works because both 
> `State::RESERVED` and `State::RESERVING` are using the same logic.
> 
> When slave transfer from `State::RESERVING` to `State::RESERVED`, it's 
> better to launch tasks directly; otherwise, we have to wait for another 
> allocation interval to launch task. I used to try following code to seperate 
> action in those two state, but `case State::RESERVING:` without `break;` is 
> not a good style :).
> 
> ```
> case State::RESERVING:
>   {
> Resources resources = offer.resources();
> Resources reserved = resources.reserved(role);
> 
> if (reserved.contains(taskResources)) {
>   updateState(dispatched, offer.slave_id(), State::RESERVED);
> }
>   }
>   // NOTE: No `break;`. Launch task after transfering to
>   // `State::RESERVED`.
> case State::RESERVED:
>   {
> if (tasksLaunched == totalTasks) {
>   // If all tasks were launched, unreserve those resources.
>   Try reserved = unreserveResources(driver, offer);
>   updateState(reserved, offer.slave_id(), State::UNRESERVING);
> } else if (tasksLaunched < totalTasks) {
>   // Framework dispatches task on the reserved resources.
>   Try dispatched = dispatchTasks(driver, offer);
>   updateState(dispatched, offer.slave_id(), 
> State::TASK_RUNNING);
> }
>   }
>   break;
> ```

Ah yea, this makes sense :-) Thanks for the explanation! Do you think you could 
add a comment to the code here to explain this reasoning?


- Greg


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


On March 17, 2016, 6:15 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 17, 2016, 6:15 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-18 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [37168]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 16, 2016, 6:32 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 16, 2016, 6:32 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-18 Thread Klaus Ma 

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

(Updated March 17, 2016, 2:15 p.m.)


Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.


Changes
---

Remove "numify.hpp"


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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-15 Thread Klaus Ma 

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

(Updated March 16, 2016, 2:32 p.m.)


Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-15 Thread Klaus Ma 


> On March 16, 2016, 1:40 a.m., Greg Mann wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 56
> > 
> >
> > Is this true? It looks like we will launch multiple tasks on some 
> > slaves.

It should be `at most one task at a time on each slave.`


- Klaus


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


On March 16, 2016, 2:27 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 16, 2016, 2:27 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-15 Thread Klaus Ma 

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

(Updated March 16, 2016, 2:27 p.m.)


Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.


Changes
---

Address Greg's 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 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-15 Thread Anand Mazumdar 


> On March 15, 2016, 5:40 p.m., Greg Mann wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 42
> > 
> >
> > Instead of using boost here, perhaps we could use members of the STL 
> > like `std::stoi` and `std::to_string`?

We already have `numify` in stout that internally uses `lexical_cast`.


- Anand


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


On March 7, 2016, 9:20 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 7, 2016, 9:20 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-15 Thread Greg Mann 

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



Thanks Klaus, this is great! It will be awesome to have an example framework 
for dynamic reservation :-)


src/Makefile.am (line 1600)


It looks like hyphens rather than underscores here would be more consistent 
with the naming of other example frameworks: `dynamic-reservation-framework`.



src/examples/dynamic_reservation_framework.cpp (line 27)


The ACL definitions were recently moved into 
include/mesos/authorizer/acls.hpp, so this include needs to be changed.



src/examples/dynamic_reservation_framework.cpp (line 42)


Instead of using boost here, perhaps we could use members of the STL like 
`std::stoi` and `std::to_string`?



src/examples/dynamic_reservation_framework.cpp (line 56)


Is this true? It looks like we will launch multiple tasks on some slaves.



src/examples/dynamic_reservation_framework.cpp (lines 57 - 58)


s/when it's offered to framework at the first time/when offered to the 
framework for the first time/

s/tasks done/tasks are done/



src/examples/dynamic_reservation_framework.cpp (line 84)


Would you mind changing the logging messages in this file to use the glog 
library?



src/examples/dynamic_reservation_framework.cpp (line 101)


If you want, you could use `hashmap` for `states` and take advantage of the 
`contains` method here.



src/examples/dynamic_reservation_framework.cpp (line 117)


s/reserved resources re-offer/reserved resources are re-offered/



src/examples/dynamic_reservation_framework.cpp (line 130)


s/this resources/these resources/



src/examples/dynamic_reservation_framework.cpp (line 143)


s/waiting for task done/waiting for task to finish/

Also, should remove the period from the end of the logging message



src/examples/dynamic_reservation_framework.cpp (line 157)


s/tasks done/tasks are done/

s/resources unreserved/resources are unreserved/



src/examples/dynamic_reservation_framework.cpp (line 207)


s/for unreserving resources/for resources to be unreserved/

Also, remove period at end



src/examples/dynamic_reservation_framework.cpp (line 255)


s/Fail to/Failed to/

Also, remove period at end



src/examples/dynamic_reservation_framework.cpp (line 423)


This error message could be a bit more descriptive, something like: "The 
default '*' role cannot be used"

Also, the period at the end of the error message should be removed.



src/examples/dynamic_reservation_framework.cpp (line 429)


Should this comment be a TODO?



src/examples/dynamic_reservation_framework.cpp (line 441)


Now that we have implicit roles, I don't think this is necessary?


- Greg Mann


On March 7, 2016, 9:20 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 7, 2016, 9:20 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-07 Thread Klaus Ma 


> On Feb. 23, 2016, 7:23 a.m., Joerg Schad wrote:
> > src/tests/dynamic_reservation_framework_test.sh, line 30
> > 
> >
> > Thy do we need this? Comparing to persistent_volume_framework_test.sh

After implicit role, it's unnecessary; removed. But before implicit role, it's 
required for master to accept more role; it's a litte different with 
persistent_volume_framework test: persistent_volume_frameowork_test uses test 
as default role, and considered it as reserved, but we can not dynamical 
reserve resources on default role.


> On Feb. 23, 2016, 7:23 a.m., Joerg Schad wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 217
> > 
> >
> > LOG(ERROR) << message; or at least add output indicating this is an 
> > error.

Add "Error: " to indicate it's an error.


- Klaus


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


On March 7, 2016, 5:20 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 7, 2016, 5:20 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, 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 a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-06 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [37168]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 6, 2016, 11:25 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 6, 2016, 11:25 a.m.)
> 
> 
> Review request for mesos, Adam B, Greg Mann, Joerg Schad, 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 a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-03-06 Thread Klaus Ma 

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

(Updated March 6, 2016, 7:25 p.m.)


Review request for mesos, Adam B, Greg Mann, Joerg Schad, and Michael Park.


Changes
---

Address joerg84's comments.


Summary (updated)
-

Add an example framework using dynamic reservation.


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 a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 37168: Add an example framework using dynamic reservation.

2016-02-21 Thread Klaus Ma 

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

(Updated Feb. 21, 2016, 9:49 p.m.)


Review request for mesos and Michael Park.


Changes
---

rebase & ping @mcypark :).


Summary (updated)
-

Add an example framework using dynamic reservation.


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 73e7ff06ba064c9b04f191009522d7808a7ab58e 
  src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
  src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
  src/tests/examples_tests.cpp 9b8b30f3718aa0fa9d02a3aa344634f0c03e61e6 

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


Testing
---

make
make check


Thanks,

Klaus Ma