You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2018/09/21 21:55:06 UTC
Re: Review Request 68138: Added tests to ensure correct quota
accounting.
-----------------------------------------------------------
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
Re: Review Request 68138: Added tests to ensure correct quota
accounting.
Posted by Meng Zhu <mz...@mesosphere.io>.
> On Dec. 4, 2018, 12:51 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 1702 (patched)
> > <https://reviews.apache.org/r/68138/diff/2/?file=2091014#file2091014line1702>
> >
> > 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)
> > <https://reviews.apache.org/r/68138/diff/2/?file=2091014#file2091014line1751>
> >
> > 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)
> > <https://reviews.apache.org/r/68138/diff/2/?file=2091014#file2091014line1767>
> >
> > 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.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/68138/#comment295874>
Maybe: QuotaAccountingReserveAllocatedResources?
src/tests/hierarchical_allocator_tests.cpp
Lines 1712 (patched)
<https://reviews.apache.org/r/68138/#comment295872>
isn't the quota setting above this comment?
src/tests/hierarchical_allocator_tests.cpp
Lines 1751-1761 (patched)
<https://reviews.apache.org/r/68138/#comment295873>
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)
<https://reviews.apache.org/r/68138/#comment295875>
Maybe: QuotaAccountingUnreserveAllocatedResources?
src/tests/hierarchical_allocator_tests.cpp
Lines 1775 (patched)
<https://reviews.apache.org/r/68138/#comment295876>
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.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
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-0000 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-0000 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-0000 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-0000
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.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
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.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
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