Re: Review Request 68138: Added tests to ensure correct quota accounting.

2018-12-05 Thread Meng Zhu 


> On Dec. 4, 2018, 12:51 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 1702 (patched)
> > 
> >
> > Maybe: QuotaAccountingReserveAllocatedResources?

Sounds good.


> On Dec. 4, 2018, 12:51 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 1751-1761 (patched)
> > 
> >
> > The way this test is written only catches the case where making the 
> > reservation could double-charge the quota, but it doesn't catch a bug where 
> > the reservation would lead to no-charge of the quota. One way to resolve 
> > this easily is to make the second agent larger, in order to make sure that 
> > the allocation is getting chopped to match the unsatisfied quota exactly.

We already have tests to ensure reservations are charged towards the quota. But 
yeah, does not hurt to double verify it here.


> On Dec. 4, 2018, 12:51 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 1767 (patched)
> > 
> >
> > Maybe: QuotaAccountingUnreserveAllocatedResources?

Sounds good.


- Meng


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


On Dec. 5, 2018, 9:03 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68138/
> ---
> 
> (Updated Dec. 5, 2018, 9:03 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9099
> https://issues.apache.org/jira/browse/MESOS-9099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added two allocator tests to ensure reserving and
> unreserving allocated resources do not affect
> quota accounting.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3034d460dd48b5134cfd4a24c54775222a348e32 
> 
> 
> Diff: https://reviews.apache.org/r/68138/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68138: Added tests to ensure correct quota accounting.

2018-12-05 Thread Meng Zhu 

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

(Updated Dec. 5, 2018, 9:03 a.m.)


Review request for mesos, Benjamin Mahler and Gastón Kleiman.


Changes
---

Addressed Ben's comment.


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


Repository: mesos


Description
---

Added two allocator tests to ensure reserving and
unreserving allocated resources do not affect
quota accounting.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
3034d460dd48b5134cfd4a24c54775222a348e32 


Diff: https://reviews.apache.org/r/68138/diff/3/

Changes: https://reviews.apache.org/r/68138/diff/2-3/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 68138: Added tests to ensure correct quota accounting.

2018-12-04 Thread Benjamin Mahler 

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


Fix it, then Ship it!





src/tests/hierarchical_allocator_tests.cpp
Lines 1702 (patched)


Maybe: QuotaAccountingReserveAllocatedResources?



src/tests/hierarchical_allocator_tests.cpp
Lines 1712 (patched)


isn't the quota setting above this comment?



src/tests/hierarchical_allocator_tests.cpp
Lines 1751-1761 (patched)


The way this test is written only catches the case where making the 
reservation could double-charge the quota, but it doesn't catch a bug where the 
reservation would lead to no-charge of the quota. One way to resolve this 
easily is to make the second agent larger, in order to make sure that the 
allocation is getting chopped to match the unsatisfied quota exactly.



src/tests/hierarchical_allocator_tests.cpp
Lines 1767 (patched)


Maybe: QuotaAccountingUnreserveAllocatedResources?



src/tests/hierarchical_allocator_tests.cpp
Lines 1775 (patched)


Ditto here, the quota setting is actually above this comment?


- Benjamin Mahler


On Sept. 21, 2018, 10:52 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68138/
> ---
> 
> (Updated Sept. 21, 2018, 10:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9099
> https://issues.apache.org/jira/browse/MESOS-9099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added two allocator tests to ensure reserving and
> unreserving allocated resources do not affect
> quota accounting.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 27fbd9cf0c4442e7675362a806d35bad141ffb6d 
> 
> 
> Diff: https://reviews.apache.org/r/68138/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68138: Added tests to ensure correct quota accounting.

2018-09-21 Thread Mesos Reviewbot Windows 

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['68138']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2374/mesos-review-68138

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2374/mesos-review-68138/logs/mesos-tests.log):

```
I0922 00:04:44.316161  7144 slave.cpp:6640] Shutting down executor 
'77fb10d3-76b5-4a11-bb67-dd2f334f2086' of framework 
cb4ff189-6aac-4ce3-ba26-f2046773664d- at executor(1)@192.10.1.5:59808
I0922 00:04:44.319162  7144 slave.cpp:909] Agent terminating
W0922 00:04:44.319162  7144 slave.cpp:3917] Ignoring shutdown framework 
cb4ff189-6aac-4ce3-ba26-f2046773664d- because it is terminating
I0922 00:04:44.319162  3904 master.cpp:11030] Removing task 
77fb10d3-76b5-4a11-bb67-dd2f334f2086 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework cb4ff189-6aac-4ce3-ba26-f2046773664d- on 
agent cb4ff189-6aac-4ce3-ba26-f2046773664d-S0 at slave(463)@192.10.1.5:58039 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I0922 00:04:44.322178  3904 master.cpp:1251] Agent 
cb4ff189-6aac-4ce3-ba26-f2046773664d-S0 at slave(463)@192.10.1.5:58039 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net) disconnected
I0922 00:04:44.322178  3904 master.cpp:3267] Disconnecting agent 
cb4ff189-6aac-4ce3-ba26-f2046773664d-S0 at slave(463)@192.10.1.5:58039 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I0922 00:04:44.322178  3904 master.cpp:3286] Deactivating agent 
cb4ff189-6aac-4ce3-ba26-f2046773664d-S0 at slave(463)@192.10.1.5:58039 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I0922 00:04:44.323175  5276 hierarchical.cpp:359] Removed framework 
cb4ff189-6aac-4ce3-ba26-f2046773664d-
I0922 00:04:44.323175  5276 hierarchical.cpp:795] Agent 
cb4ff189-6aac-4ce3-ba26-f2046773664d-S0 deactivated
I0922 00:04:44.323175  6728 co[   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (683 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (702 ms total)

[--] Global test environment tear-down
[==] 1054 tests from 103 test cases ran. (611262 ms total)
[  PASSED  ] 1052 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName

 2 FAILED TESTS
  YOU HAVE 231 DISABLED TESTS

ntainerizer.cpp:2455] Destroying container d765d07f-2252-4605-bb30-fe21888d1552 
in RUNNING state
I0922 00:04:44.324163  6728 containerizer.cpp:3118] Transitioning the state of 
container d765d07f-2252-4605-bb30-fe21888d1552 from RUNNING to DESTROYING
I0922 00:04:44.324163  6728 launcher.cpp:166] Asked to destroy container 
d765d07f-2252-4605-bb30-fe21888d1552
I0922 00:04:44.395115  7144 containerizer.cpp:2957] Container 
d765d07f-2252-4605-bb30-fe21888d1552 has exited
I0922 00:04:44.424129  9176 master.cpp:1093] Master terminating
I0922 00:04:44.425124  8448 hierarchical.cpp:637] Removed agent 
cb4ff189-6aac-4ce3-ba26-f2046773664d-S0
I0922 00:04:44.682111  7236 process.cpp:926] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Sept. 21, 2018, 3:52 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68138/
> ---
> 
> (Updated Sept. 21, 2018, 3:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9099
> https://issues.apache.org/jira/browse/MESOS-9099
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added two allocator tests to ensure reserving and
> unreserving allocated resources do not affect
> quota accounting.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 27fbd9cf0c4442e7675362a806d35bad141ffb6d 
> 
> 
> Diff: https://reviews.apache.org/r/68138/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68138: Added tests to ensure correct quota accounting.

2018-09-21 Thread Meng Zhu 

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

(Updated Sept. 21, 2018, 3:52 p.m.)


Review request for mesos, Benjamin Mahler and Gastón Kleiman.


Changes
---

Removed dependencies.


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


Repository: mesos


Description
---

Added two allocator tests to ensure reserving and
unreserving allocated resources do not affect
quota accounting.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
27fbd9cf0c4442e7675362a806d35bad141ffb6d 


Diff: https://reviews.apache.org/r/68138/diff/2/


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 68138: Added tests to ensure correct quota accounting.

2018-09-21 Thread Meng Zhu 

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

(Updated Sept. 21, 2018, 2:55 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased. Fixed some comments and removed dependencies.


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


Repository: mesos


Description (updated)
---

Added two allocator tests to ensure reserving and
unreserving allocated resources do not affect
quota accounting.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
27fbd9cf0c4442e7675362a806d35bad141ffb6d 


Diff: https://reviews.apache.org/r/68138/diff/2/

Changes: https://reviews.apache.org/r/68138/diff/1-2/


Testing
---

make check


Thanks,

Meng Zhu

Review Request 68138: Added tests to ensure correct quota accounting.

2018-07-31 Thread Meng Zhu 

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Added two allocator tests to ensure reserving and
unreserving allocated resources should not affect
quota accounting.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
27fbd9cf0c4442e7675362a806d35bad141ffb6d 


Diff: https://reviews.apache.org/r/68138/diff/1/


Testing
---

make check


Thanks,

Meng Zhu