----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70530/#review215329 -----------------------------------------------------------
Patch looks great! Reviews applied: [70583, 70531, 70530] Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh - Mesos Reviewbot On May 10, 2019, 2:50 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70530/ > ----------------------------------------------------------- > > (Updated May 10, 2019, 2:50 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-7258 > https://issues.apache.org/jira/browse/MESOS-7258 > > > Repository: mesos > > > Description > ------- > > The main concern of this patch is the behaviour of the framework > re-subscription and of the upcoming UPDATE_FRAMEWORK call in cases when > the framework attempts to change the "immutable" fields of > FrameworkInfo, namely: `user`, `checkpoint`, `principal` and `id`. > > The changes introduced by this patch: > - The method `Framework::update()` is simplified: the logic of ignoring > the immutable fields is removed, this method fails the program if the > new values differ from the old ones. This is needed to simplify > keeping UPDATE_FRAMEWORK consistent with re-subscription. > - The method `Master::validateFrameworkSubscription()` now returns > validation errors on attempts to change `user` or `checkpoint` fields. > This is needed to enable validating them in the UPDATE_FRAMEWORK call. > - A method `Master::sanitizeFrameworkSubscription()` is introduced to > preserve the legacy behaviour of re-subscription (which ignores the > updates of `user` and `checkpoint`). > - A second call of `validateFrameworkSubscription()` is performed after > the authorization. This is a crude fix of the already existing race > between two re-subscriptions against an empty master (see MESOS-9763). > It is necessary because otherwise the change in `Framework::update()` > would exacerbate this race (namely, it would be possible to crash the > master). > > > Diffs > ----- > > src/master/framework.cpp 05f5514c589b2dba08afe77281e5fbc4e29f232b > src/master/master.hpp 7d9732f1e432f6f0290d234242864cbdbf381fa8 > src/master/master.cpp a8ee6297e1587c160a47b806914d3c3aa4f35cd7 > src/master/validation.hpp 95638a17052ece6c957aa76e4cead8d7bfe82024 > src/master/validation.cpp 9fb0850987ce385d345302cac9721adead7181b8 > > > Diff: https://reviews.apache.org/r/70530/diff/4/ > > > Testing > ------- > > > Thanks, > > Andrei Sekretenko > >
