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() << "'";
```

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-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 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 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-16 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-16 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-16 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