Re: Review Request 43588: Added allocator recovery tests in presence of quota.

[Klaus Ma]

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



ping :).

- Klaus Ma


On Feb. 23, 2016, 7:55 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 23, 2016, 7:55 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43588: Added allocator recovery tests in presence of quota.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43588]

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 Feb. 22, 2016, 11:55 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 22, 2016, 11:55 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43588: Added allocator recovery tests in presence of quota.

[Joerg Schad]

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

(Updated Feb. 22, 2016, 11:51 p.m.)


Review request for mesos, Alexander Rukletsov and Klaus Ma.


Changes
---

Addressed reviews.


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


Repository: mesos


Description
---

Added allocator recovery tests in presence of quota.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
5f771f02db9bd098f3cd36730cd84bf2f5e87a33 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 43588: Added allocator recovery tests in presence of quota.

[Joerg Schad]

> On Feb. 16, 2016, 3:28 a.m., Klaus Ma wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2417
> > 
> >
> > No allocation because there is no slaves. We have trigger allocation by 
> > `Clock::advance(flags.allocation_interval);`.

This is done above, not sure what you are looking for.


- Joerg


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


On Feb. 15, 2016, 9:23 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 15, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0acfc098750ff8ff9505207b983a34c1ccf3ad06 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43588: Added allocator recovery tests in presence of quota.

[Klaus Ma]

> On Feb. 16, 2016, 11:28 a.m., Klaus Ma wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2400
> > 
> >
> > Move to HierarchicalAllocatorTest.
> 
> Joerg Schad wrote:
> This will be done by/after https://reviews.apache.org/r/41950.

Just checked r41950, that's OK to me :).


- Klaus


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


On Feb. 16, 2016, 5:23 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 16, 2016, 5:23 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0acfc098750ff8ff9505207b983a34c1ccf3ad06 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43588: Added allocator recovery tests in presence of quota.

[Joerg Schad]

> On Feb. 16, 2016, 3:28 a.m., Klaus Ma wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2400
> > 
> >
> > Move to HierarchicalAllocatorTest.

This will be done by/after https://reviews.apache.org/r/41950.


- Joerg


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


On Feb. 15, 2016, 9:23 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 15, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0acfc098750ff8ff9505207b983a34c1ccf3ad06 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43588: Added allocator recovery tests in presence of quota.

[Klaus Ma]

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




src/tests/hierarchical_allocator_tests.cpp (line 2400)


Move to HierarchicalAllocatorTest.



src/tests/hierarchical_allocator_tests.cpp (line 2417)


No allocation because there is no slaves. We have trigger allocation by 
`Clock::advance(flags.allocation_interval);`.



src/tests/hierarchical_allocator_tests.cpp (line 2423)


Add var on half of agents; we can used it when add 80% resources.



src/tests/hierarchical_allocator_tests.cpp (line 2438)


delete emepty line.



src/tests/hierarchical_allocator_tests.cpp (line 2442)


Replace `4u` to var `10 * AGENT_RECOVERY_FACTOR - half_agents_num + 1`. So 
when we update the value of `AGENT_RECOVERY_FACTOR` or export it as flags, we 
did not need to re-calculate the number.



src/tests/hierarchical_allocator_tests.cpp (line 2459)


Add check on whether all resources are got.

@Alex/@Joerg, I'd like to confirm the behaviour of following two cases:

In `::addSlave`, the slaves after 80% will offer to framework firstly; the 
others (< 80%) will offer to framework until next allocation interval. I think 
we'd better to offer ALL recovered resources when 80% slaves recovered.

Another case that the counter did not update in `::removeSlave`. What's our 
expected behaviour on the following case?

* 100 slaves
* 70 slaves are added ino allocator
* 60 of 70 added slaves are down
* another 20 slaves are added into alloator
 
Current behaviour, allocator will continue to offer resources based on 30 
slaves because "90%" are added. We'd better to wait for timeout for this case.



src/tests/hierarchical_allocator_tests.cpp (line 2507)


ditto



src/tests/hierarchical_allocator_tests.cpp (line 2513)


ditto


- Klaus Ma


On Feb. 16, 2016, 5:23 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 16, 2016, 5:23 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0acfc098750ff8ff9505207b983a34c1ccf3ad06 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43588: Added allocator recovery tests in presence of quota.

[Guangya Liu]

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




src/tests/hierarchical_allocator_tests.cpp (line 2390)


s/we/the allocator will



src/tests/hierarchical_allocator_tests.cpp (line 2391)


s/RecoverPercentage/RecoverPercentageWithQuota?

How about move this above `DeactivateAndReactivateFramework` to make sure 
group all `Quota` related tests?



src/tests/hierarchical_allocator_tests.cpp (line 2434)


How about s/Test that the allocator still pauses./Wait for all `addSlave` 
messages to be dispatched and processed completely?



src/tests/hierarchical_allocator_tests.cpp (line 2457)


s/A we/The framework



src/tests/hierarchical_allocator_tests.cpp (line 2474)


s/we/the framework



src/tests/hierarchical_allocator_tests.cpp (line 2482)


s/the the/the



src/tests/hierarchical_allocator_tests.cpp (line 2483)


s/RecoverTimeout/RecoverTimeoutWithQuota?

How about move this above `DeactivateAndReactivateFramework` to make sure 
group all `Quota` related tests?



src/tests/hierarchical_allocator_tests.cpp (line 2524)


How about s/Test that the allocator still pauses./Wait for all `addSlave` 
messages to be dispatched and processed completely?


- Guangya Liu


On 二月 15, 2016, 9:23 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated 二月 15, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0acfc098750ff8ff9505207b983a34c1ccf3ad06 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43588: Added allocator recovery tests in presence of quota.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43588]

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 Feb. 15, 2016, 9:23 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 15, 2016, 9:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 0acfc098750ff8ff9505207b983a34c1ccf3ad06 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 43588: Added allocator recovery tests in presence of quota.

[Joerg Schad]

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

Review request for mesos, Alexander Rukletsov and Klaus Ma.


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


Repository: mesos


Description
---

Added allocator recovery tests in presence of quota.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
0acfc098750ff8ff9505207b983a34c1ccf3ad06 

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


Testing
---

make check


Thanks,

Joerg Schad