----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63523/#review190160 -----------------------------------------------------------
Fix it, then Ship it! src/master/allocator/mesos/hierarchical.cpp Line 828 (original), 768 (patched) <https://reviews.apache.org/r/63523/#comment267424> This can be a `const` ref. src/master/allocator/mesos/hierarchical.cpp Lines 888-896 (original), 809-817 (patched) <https://reviews.apache.org/r/63523/#comment267425> This could be more general, and I am not sure I agree with the general direction the `TODO` outlines. In the end we only need to handle conversions which actually consume resources here, and can drop all which don't. This can in principle be independent from e.g., launches on shared resources. src/master/master.cpp Lines 4890 (patched) <https://reviews.apache.org/r/63523/#comment267419> Not yours, but this could be a `const` ref. src/master/master.cpp Lines 4888-4889 (original) <https://reviews.apache.org/r/63523/#comment267422> This is weird. Any idea why we did inject this information into allocators? Is there any expectation that a custom allocator should be able to access this information? src/tests/hierarchical_allocator_tests.cpp Lines 5001-5003 (original), 5015-5017 (patched) <https://reviews.apache.org/r/63523/#comment267426> This is weird now since `launch` is only used to calculate the expectation below. We should either try to inject it in the allocator just like `create`, or try to get rid of it completely. - Benjamin Bannier On Nov. 2, 2017, 8:36 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63523/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2017, 8:36 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, > Greg Mann, Jan Schlicht, Vinod Kone, and Jiang Yan Xu. > > > Repository: mesos > > > Description > ------- > > Instead of taking offer operations, it's more clear to make allocator > not aware of offer operations. This patch changes the interface of the > `updateAllocation` call to take `ResourceConversion`s. > > > Diffs > ----- > > include/mesos/allocator/allocator.hpp > 537658bf4e68ac7f271248af0e1e35df2b19aef8 > src/master/allocator/mesos/allocator.hpp > 903edf68e812e022cddedd40a2cb51726b2a181b > src/master/allocator/mesos/hierarchical.hpp > c2346054b2c98516f15ab8ce2dc798224ff4def4 > src/master/allocator/mesos/hierarchical.cpp > 848e9da1b61f411449c0cd2409e98ec733ac10ee > src/master/master.cpp e047462018af50b9047b8939e922d80c4771fb28 > src/tests/allocator.hpp f1c0d148886b7b2936918b7d7c77c03ee766ee4a > src/tests/hierarchical_allocator_tests.cpp > 48b48ad70f44cc2232c2a29699267027f9937b8a > > > Diff: https://reviews.apache.org/r/63523/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
