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