----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22284/#review44959 -----------------------------------------------------------
src/master/master.hpp <https://reviews.apache.org/r/22284/#comment79568> Same comment as previous review and yan's review, how about having this return a validation result? i.e. // Returns the validation result. Future<Option<Error>> validate(...); src/master/master.cpp <https://reviews.apache.org/r/22284/#comment79561> How much does this slow down the tests? One suggestion: what if we add a backoff (including initial jitter) akin to what's done in the slave? That way, we can scale better to a large number of frameworks and we don't have to have this hack in the master. Of course, we would want to make the backoff for framework registration smaller than what's done in the slaves. If the slowdown is not so bad, we could remove this re-queueing, leave a ticket and TODO in sched.cpp and follow up after this change. Each time I look at this, it's difficult to understand the invariants that make the re-queuing correct so my brain would appreciate it :) It means that we could move this case into 'validate' as an invalid case, which makes the code simpler. src/master/master.cpp <https://reviews.apache.org/r/22284/#comment79569> Curious if these checks here and in _reregisterFramework are necessary for correctness or just good to have? Not saying that we should remove them, but if necessary for correctness might be good to spell out why. src/master/master.cpp <https://reviews.apache.org/r/22284/#comment79563> How about "in the interim" instead of "before we are here", seems to more precisely describe the situation? src/master/master.cpp <https://reviews.apache.org/r/22284/#comment79564> Ditto. src/master/master.cpp <https://reviews.apache.org/r/22284/#comment79565> ditto src/master/master.cpp <https://reviews.apache.org/r/22284/#comment79566> ditto - Ben Mahler On June 6, 2014, 12:12 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22284/ > ----------------------------------------------------------- > > (Updated June 6, 2014, 12:12 a.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-1307 > https://issues.apache.org/jira/browse/MESOS-1307 > > > Repository: mesos-git > > > Description > ------- > > Added authorization for roles during framework (re-)registration. > > > Diffs > ----- > > src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149 > src/master/master.hpp e2448310aa714b5fae49b9bf4fc95859ae9d7ec3 > src/master/master.cpp 89f426c14de365369b900864f1983b1f9260953f > src/tests/master_authorization_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/22284/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
