You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2018/07/03 21:35:27 UTC
Re: Review Request 66856: Tracked completed framework metrics in the
allocator.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66856/
-----------------------------------------------------------
(Updated July 3, 2018, 9:35 p.m.)
Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and Vinod Kone.
Repository: mesos
Description
-------
This ensures that per-framework metrics which are tracked in the
allocator will be retained as long as the per-framework metrics
which are tracked in the master.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp c19ab64ff1422a68cfb6eecea7e2cc4132da8dcc
src/master/allocator/mesos/allocator.hpp 900c8ee405da6e44532dee598edaa42373ebd4e5
src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26
src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b
src/master/master.cpp ddc8df0ea82241be6c733237feef1553c7669eb2
src/tests/allocator.hpp 73fc06043746a0498d0dd1846fae9433db136d49
src/tests/api_tests.cpp f343991a5d23ac665429456471ac06a5315fc692
src/tests/master_allocator_tests.cpp 824a7554858fb8356751f3469960dddd7505bd98
src/tests/master_quota_tests.cpp d836482f5593b462ad235620741148678ac2651d
src/tests/reservation_tests.cpp 058a66d7914c7a84f0ba86dfd3ff2e3c0bbcb5c6
src/tests/resource_offers_tests.cpp 6e24cddce65e7f5a0ce8ea4fde8397a19236d8b8
src/tests/slave_recovery_tests.cpp e8333402d8524a4bf302872d246fe5f5c006bdc5
Diff: https://reviews.apache.org/r/66856/diff/4/
Changes: https://reviews.apache.org/r/66856/diff/3-4/
Testing (updated)
-------
Testing details can be found at the end of this chain.
Thanks,
Greg Mann
Re: Review Request 66856: Tracked completed framework metrics in the
allocator.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66856/#review206465
-----------------------------------------------------------
Ship it!
Ship It!
- Benjamin Mahler
On July 24, 2018, 5:30 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66856/
> -----------------------------------------------------------
>
> (Updated July 24, 2018, 5:30 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This ensures that per-framework metrics which are tracked in the
> allocator will be retained as long as the per-framework metrics
> which are tracked in the master.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp c19ab64ff1422a68cfb6eecea7e2cc4132da8dcc
> src/master/allocator/mesos/allocator.hpp 4c337470c5722a5bd1f53c67b5d81a497a7b8023
> src/master/allocator/mesos/hierarchical.hpp c1a6789f1808a57dd94ede7bbd2636031f136ea3
> src/master/allocator/mesos/hierarchical.cpp 7b4e9db3435b51c9ce025b7b522e10db4b907ebb
> src/master/master.cpp 2af976f7ea7f81d4b06a45ce13286dbd61b9b144
> src/tests/allocator.hpp 6139492bf4d18f31e558bd0216bcccd2705af4d7
> src/tests/api_tests.cpp 182622a62d350ebefc891a385de3f2d35a7c0243
> src/tests/master_allocator_tests.cpp 9d802c8ba04374ac6df7aac668bcf05d5eb6c407
> src/tests/master_quota_tests.cpp d4a8db8b34bcd6793af024e998054b795e900832
> src/tests/reservation_tests.cpp 6ae0055ad3ca2f848c613a293177d02b3140355e
> src/tests/resource_offers_tests.cpp 6e24cddce65e7f5a0ce8ea4fde8397a19236d8b8
> src/tests/slave_recovery_tests.cpp 69b49220e00a37b0824203ae208813a60b67862e
>
>
> Diff: https://reviews.apache.org/r/66856/diff/8/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 66856: Tracked completed framework metrics in the
allocator.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66856/
-----------------------------------------------------------
(Updated July 24, 2018, 5:30 p.m.)
Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and Vinod Kone.
Repository: mesos
Description
-------
This ensures that per-framework metrics which are tracked in the
allocator will be retained as long as the per-framework metrics
which are tracked in the master.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp c19ab64ff1422a68cfb6eecea7e2cc4132da8dcc
src/master/allocator/mesos/allocator.hpp 4c337470c5722a5bd1f53c67b5d81a497a7b8023
src/master/allocator/mesos/hierarchical.hpp c1a6789f1808a57dd94ede7bbd2636031f136ea3
src/master/allocator/mesos/hierarchical.cpp 7b4e9db3435b51c9ce025b7b522e10db4b907ebb
src/master/master.cpp 2af976f7ea7f81d4b06a45ce13286dbd61b9b144
src/tests/allocator.hpp 6139492bf4d18f31e558bd0216bcccd2705af4d7
src/tests/api_tests.cpp 182622a62d350ebefc891a385de3f2d35a7c0243
src/tests/master_allocator_tests.cpp 9d802c8ba04374ac6df7aac668bcf05d5eb6c407
src/tests/master_quota_tests.cpp d4a8db8b34bcd6793af024e998054b795e900832
src/tests/reservation_tests.cpp 6ae0055ad3ca2f848c613a293177d02b3140355e
src/tests/resource_offers_tests.cpp 6e24cddce65e7f5a0ce8ea4fde8397a19236d8b8
src/tests/slave_recovery_tests.cpp 69b49220e00a37b0824203ae208813a60b67862e
Diff: https://reviews.apache.org/r/66856/diff/8/
Changes: https://reviews.apache.org/r/66856/diff/7-8/
Testing
-------
Testing details can be found at the end of this chain.
Thanks,
Greg Mann
Re: Review Request 66856: Tracked completed framework metrics in the
allocator.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66856/
-----------------------------------------------------------
(Updated July 20, 2018, 8:58 p.m.)
Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and Vinod Kone.
Changes
-------
Rebase to accommodate our new usage of the `override` keyword.
Repository: mesos
Description
-------
This ensures that per-framework metrics which are tracked in the
allocator will be retained as long as the per-framework metrics
which are tracked in the master.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp c19ab64ff1422a68cfb6eecea7e2cc4132da8dcc
src/master/allocator/mesos/allocator.hpp 4c337470c5722a5bd1f53c67b5d81a497a7b8023
src/master/allocator/mesos/hierarchical.hpp 702e7c0aa84b4b672d82c759c25a28a77c78ad50
src/master/allocator/mesos/hierarchical.cpp 707dd6bd0f255a64d759ce87cbf75be57d86b392
src/master/master.cpp 2af976f7ea7f81d4b06a45ce13286dbd61b9b144
src/tests/allocator.hpp 6139492bf4d18f31e558bd0216bcccd2705af4d7
src/tests/api_tests.cpp 182622a62d350ebefc891a385de3f2d35a7c0243
src/tests/master_allocator_tests.cpp 9d802c8ba04374ac6df7aac668bcf05d5eb6c407
src/tests/master_quota_tests.cpp d4a8db8b34bcd6793af024e998054b795e900832
src/tests/reservation_tests.cpp 6ae0055ad3ca2f848c613a293177d02b3140355e
src/tests/resource_offers_tests.cpp 6e24cddce65e7f5a0ce8ea4fde8397a19236d8b8
src/tests/slave_recovery_tests.cpp 69b49220e00a37b0824203ae208813a60b67862e
Diff: https://reviews.apache.org/r/66856/diff/7/
Changes: https://reviews.apache.org/r/66856/diff/6-7/
Testing
-------
Testing details can be found at the end of this chain.
Thanks,
Greg Mann
Re: Review Request 66856: Tracked completed framework metrics in the
allocator.
Posted by Greg Mann <gr...@mesosphere.io>.
> On July 19, 2018, 10:50 p.m., Gastón Kleiman wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 346-350 (patched)
> > <https://reviews.apache.org/r/66856/diff/6/?file=2061394#file2061394line346>
> >
> > This looks good, but the framework removal flow is not straightforward. I'd feel much more confident in the change if there was a test that verifies that completed frameworks are evicted once `max_completed_frameworks` is reached.
> >
> > The test could start a master with `--max_completed_frameworks=2`, register three frameworks and then check the metrics.
> >
> > I think you could add that test to https://reviews.apache.org/r/67878/ or in a new patch.
I ended up updating the existing test for the `max_completed_frameworks` flag to validate the metrics as well; it's in this patch: https://reviews.apache.org/r/67878/
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66856/#review206252
-----------------------------------------------------------
On July 24, 2018, 5:30 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66856/
> -----------------------------------------------------------
>
> (Updated July 24, 2018, 5:30 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This ensures that per-framework metrics which are tracked in the
> allocator will be retained as long as the per-framework metrics
> which are tracked in the master.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp c19ab64ff1422a68cfb6eecea7e2cc4132da8dcc
> src/master/allocator/mesos/allocator.hpp 4c337470c5722a5bd1f53c67b5d81a497a7b8023
> src/master/allocator/mesos/hierarchical.hpp c1a6789f1808a57dd94ede7bbd2636031f136ea3
> src/master/allocator/mesos/hierarchical.cpp 7b4e9db3435b51c9ce025b7b522e10db4b907ebb
> src/master/master.cpp 2af976f7ea7f81d4b06a45ce13286dbd61b9b144
> src/tests/allocator.hpp 6139492bf4d18f31e558bd0216bcccd2705af4d7
> src/tests/api_tests.cpp 182622a62d350ebefc891a385de3f2d35a7c0243
> src/tests/master_allocator_tests.cpp 9d802c8ba04374ac6df7aac668bcf05d5eb6c407
> src/tests/master_quota_tests.cpp d4a8db8b34bcd6793af024e998054b795e900832
> src/tests/reservation_tests.cpp 6ae0055ad3ca2f848c613a293177d02b3140355e
> src/tests/resource_offers_tests.cpp 6e24cddce65e7f5a0ce8ea4fde8397a19236d8b8
> src/tests/slave_recovery_tests.cpp 69b49220e00a37b0824203ae208813a60b67862e
>
>
> Diff: https://reviews.apache.org/r/66856/diff/8/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 66856: Tracked completed framework metrics in the
allocator.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66856/#review206252
-----------------------------------------------------------
Fix it, then Ship it!
src/master/allocator/mesos/hierarchical.cpp
Lines 346-350 (patched)
<https://reviews.apache.org/r/66856/#comment289156>
This looks good, but the framework removal flow is not straightforward. I'd feel much more confident in the change if there was a test that verifies that completed frameworks are evicted once `max_completed_frameworks` is reached.
The test could start a master with `--max_completed_frameworks=2`, register three frameworks and then check the metrics.
I think you could add that test to https://reviews.apache.org/r/67878/ or in a new patch.
- Gastón Kleiman
On July 17, 2018, 6:45 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66856/
> -----------------------------------------------------------
>
> (Updated July 17, 2018, 6:45 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This ensures that per-framework metrics which are tracked in the
> allocator will be retained as long as the per-framework metrics
> which are tracked in the master.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp c19ab64ff1422a68cfb6eecea7e2cc4132da8dcc
> src/master/allocator/mesos/allocator.hpp 900c8ee405da6e44532dee598edaa42373ebd4e5
> src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26
> src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b
> src/master/master.cpp 487ee349ef0cd78903ff6bcdea3f24688810cdc5
> src/tests/allocator.hpp 73fc06043746a0498d0dd1846fae9433db136d49
> src/tests/api_tests.cpp 728fb4c38c17c98b6c2a85447d3f32519c035e56
> src/tests/master_allocator_tests.cpp 824a7554858fb8356751f3469960dddd7505bd98
> src/tests/master_quota_tests.cpp d836482f5593b462ad235620741148678ac2651d
> src/tests/reservation_tests.cpp 058a66d7914c7a84f0ba86dfd3ff2e3c0bbcb5c6
> src/tests/resource_offers_tests.cpp 6e24cddce65e7f5a0ce8ea4fde8397a19236d8b8
> src/tests/slave_recovery_tests.cpp e8333402d8524a4bf302872d246fe5f5c006bdc5
>
>
> Diff: https://reviews.apache.org/r/66856/diff/6/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 66856: Tracked completed framework metrics in the
allocator.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66856/
-----------------------------------------------------------
(Updated July 18, 2018, 1:45 a.m.)
Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and Vinod Kone.
Repository: mesos
Description
-------
This ensures that per-framework metrics which are tracked in the
allocator will be retained as long as the per-framework metrics
which are tracked in the master.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp c19ab64ff1422a68cfb6eecea7e2cc4132da8dcc
src/master/allocator/mesos/allocator.hpp 900c8ee405da6e44532dee598edaa42373ebd4e5
src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26
src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b
src/master/master.cpp 487ee349ef0cd78903ff6bcdea3f24688810cdc5
src/tests/allocator.hpp 73fc06043746a0498d0dd1846fae9433db136d49
src/tests/api_tests.cpp 728fb4c38c17c98b6c2a85447d3f32519c035e56
src/tests/master_allocator_tests.cpp 824a7554858fb8356751f3469960dddd7505bd98
src/tests/master_quota_tests.cpp d836482f5593b462ad235620741148678ac2651d
src/tests/reservation_tests.cpp 058a66d7914c7a84f0ba86dfd3ff2e3c0bbcb5c6
src/tests/resource_offers_tests.cpp 6e24cddce65e7f5a0ce8ea4fde8397a19236d8b8
src/tests/slave_recovery_tests.cpp e8333402d8524a4bf302872d246fe5f5c006bdc5
Diff: https://reviews.apache.org/r/66856/diff/6/
Changes: https://reviews.apache.org/r/66856/diff/5-6/
Testing
-------
Testing details can be found at the end of this chain.
Thanks,
Greg Mann
Re: Review Request 66856: Tracked completed framework metrics in the
allocator.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66856/
-----------------------------------------------------------
(Updated July 9, 2018, 3:45 p.m.)
Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and Vinod Kone.
Repository: mesos
Description
-------
This ensures that per-framework metrics which are tracked in the
allocator will be retained as long as the per-framework metrics
which are tracked in the master.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp c19ab64ff1422a68cfb6eecea7e2cc4132da8dcc
src/master/allocator/mesos/allocator.hpp 900c8ee405da6e44532dee598edaa42373ebd4e5
src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26
src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b
src/master/master.cpp ddc8df0ea82241be6c733237feef1553c7669eb2
src/tests/allocator.hpp 73fc06043746a0498d0dd1846fae9433db136d49
src/tests/api_tests.cpp f343991a5d23ac665429456471ac06a5315fc692
src/tests/master_allocator_tests.cpp 824a7554858fb8356751f3469960dddd7505bd98
src/tests/master_quota_tests.cpp d836482f5593b462ad235620741148678ac2651d
src/tests/reservation_tests.cpp 058a66d7914c7a84f0ba86dfd3ff2e3c0bbcb5c6
src/tests/resource_offers_tests.cpp 6e24cddce65e7f5a0ce8ea4fde8397a19236d8b8
src/tests/slave_recovery_tests.cpp e8333402d8524a4bf302872d246fe5f5c006bdc5
Diff: https://reviews.apache.org/r/66856/diff/5/
Changes: https://reviews.apache.org/r/66856/diff/4-5/
Testing
-------
Testing details can be found at the end of this chain.
Thanks,
Greg Mann