----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70378/#review214387 -----------------------------------------------------------
src/master/master.hpp Lines 100-158 (patched) <https://reviews.apache.org/r/70378/#comment300594> I think that I would drop this patch. However whenever we add new data structures, we add them in stout (https://github.com/apache/mesos/blob/003e47d852f6eb9a1cd7c86750d91b37269aec26/3rdparty/stout/README.md) instead of in the Mesos codebase. - Gastón Kleiman On April 3, 2019, 9:01 a.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70378/ > ----------------------------------------------------------- > > (Updated April 3, 2019, 9:01 a.m.) > > > Review request for mesos and Gastón Kleiman. > > > Bugs: MESOS-2842 > https://issues.apache.org/jira/browse/MESOS-2842 > > > Repository: mesos > > > Description > ------- > > !! This code is completely disposable and possibly not properly located; the > next patch actually does not depend on this one. > > ------ > > After having a look at the Master code, I realized that the hashmaps > `frameworks.principlas` and `authorized` should satisfy the following > property: the value corresponding to a given key never changes. > > Currently, this invariant is enforced MANUALLY with hacks of three kinds. > The first hack is constructs like this: > > CHECK(!h.contains(key)); > h[key] = newValue; > > The second hack is much worse: it is avoiding calling non-const > `hashmap::operator[]()` for nonexisting keys. > > And the third one is even worse: it is avoiding this situation: > h.erase(key); > h[key] = newValue; > > To make sure that at least the first two hacks has been used properly, I > added this wrapper around hashmap. > Actually, this is a dirty hack itself, and not a proper decomposition of the > God Object antipattern that, IMO, has somehow started to grow inside of the > Master. > > Unfortunately, there is no such simple way to validate that the third kind of > hacks is used properly. (In fact, it is not - and this is the direct cause of > MESOS-2842.) > > > Diffs > ----- > > src/master/master.hpp 94891af9deeaddb3333fc9d6eabb243aed97f7b7 > src/master/master.cpp cf5caa0893ba1387a1f3a9d129ecd7d974f776bd > > > Diff: https://reviews.apache.org/r/70378/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Andrei Sekretenko > >
