----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69989/#review212845 -----------------------------------------------------------
Fix it, then Ship it! src/master/allocator/mesos/hierarchical.cpp Lines 2575-2577 (patched) <https://reviews.apache.org/r/69989/#comment298732> Probably want to add some comments here for posterity? We may also want to update the invariant in the comment https://github.com/apache/mesos/blob/master/src/master/allocator/mesos/hierarchical.hpp#L546 i.e. s/non-empty reservations/non-empty scalar resource reservations/ - Meng Zhu On Feb. 14, 2019, 12:55 p.m., Benjamin Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69989/ > ----------------------------------------------------------- > > (Updated Feb. 14, 2019, 12:55 p.m.) > > > Review request for mesos and Meng Zhu. > > > Bugs: MESOS-9555 > https://issues.apache.org/jira/browse/MESOS-9555 > > > Repository: mesos > > > Description > ------- > > The allocator tracks reservation scalar quantities in a per-role > map. The track and untrack functions are supposed to ensure that > empty entries are never present, however the track function will > insert an empty entry in the case of a reservation of non-scalar > resources. The untrack function will expect an entry for the role > when untracking, even if untracking a non-scalar reservation. > > So, the following situation leads to a crash: > > (1) track ports reservation1 for role: {role: {}} > (2) track ports reservation2 for role: {role: {}} > (3) untrack ports reservation1 for role: {} > (4) untrack ports reservation2 for role: CHECK failure, > untrackReservation expects an entry for role. > > This patch ensures we ignore reservations that are empty once > stripped of non scalar quantities. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > 862dbb90bdfa39ead4b185104a308eabe249d734 > > > Diff: https://reviews.apache.org/r/69989/diff/1/ > > > Testing > ------- > > make check, regression test added in subsequent patch > > > Thanks, > > Benjamin Mahler > >
