Re: Review Request 66856: Tracked completed framework metrics in the allocator.

2018-07-25 Thread Benjamin Mahler 

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

2018-07-25 Thread Greg Mann 


> On July 19, 2018, 10:50 p.m., Gastón Kleiman wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 346-350 (patched)
> > 
> >
> > 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.

2018-07-24 Thread Greg Mann 

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

2018-07-20 Thread Greg Mann 

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

2018-07-19 Thread Gastón Kleiman 

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


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 
> 824a7554858fb8356751f34699607505bd98 
>   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.

2018-07-17 Thread Greg Mann 

---
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 824a7554858fb8356751f34699607505bd98 
  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.

2018-07-09 Thread Greg Mann 

---
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 824a7554858fb8356751f34699607505bd98 
  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

Re: Review Request 66856: Tracked completed framework metrics in the allocator.

2018-07-03 Thread Greg Mann 

---
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 824a7554858fb8356751f34699607505bd98 
  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.

2018-05-14 Thread Greg Mann 

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

(Updated May 14, 2018, 5:28 p.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
Vinod Kone.


Changes
---

Rebase.


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 
e5a5355646c834280008867e89eb258eaefc2f1d 
  src/master/allocator/mesos/allocator.hpp 
c453c015b234deff7efd00269da25dcec8cbf1ae 
  src/master/allocator/mesos/hierarchical.hpp 
955ae3e6a9e3c790fb260311ec4b4ef725a826d3 
  src/master/allocator/mesos/hierarchical.cpp 
1000968be6a2935a4cac571414d7f06d7df7acf0 
  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/tests/allocator.hpp 341efa665ad0ce897e087fb8d73ec50fd041d559 
  src/tests/api_tests.cpp 84368707e2c0bcf66bbfb308a4b863112119d328 
  src/tests/master_allocator_tests.cpp e1aef8a9625a805e7ad2dfad37bfeedee82f160d 
  src/tests/master_quota_tests.cpp ddb2903c0b60f2929e39d6aa23dc924aec454c69 
  src/tests/reservation_tests.cpp 7d121bf56b913c3217dec00c57f81663e9831351 
  src/tests/resource_offers_tests.cpp 54aafdb4258ad7713c5f1a59956e7f76f0e84d5b 
  src/tests/slave_recovery_tests.cpp afe8b8a6cc21421a37164016d7fe01bf96a559da 


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

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


Testing
---


Thanks,

Greg Mann