----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63842/#review191610 -----------------------------------------------------------
src/master/master.cpp Line 10925 (original), 10922 (patched) <https://reviews.apache.org/r/63842/#comment269466> Especially now that we have code in this function which switches on operation type, could we add a comment to the method's declaration briefly explaining its use? For example, it's not clear from reading the header that this method should not be called on LAUNCH or LAUNCH_GROUP operations. src/master/master.cpp Lines 10936-10939 (patched) <https://reviews.apache.org/r/63842/#comment269461> Nit: could we put this case at the end? I think we tend to place the UNKNOWN case last. src/master/master.cpp Lines 10940-10946 (patched) <https://reviews.apache.org/r/63842/#comment269459> Just leaving a note here for posterity: in recent discussions, we've converged on the idea that we should avoid operation speculation for all operations when feedback is requested. We'll need to update this code to reflect that once the framework-facing side of offer operation feedback is implemented. - Greg Mann On Nov. 17, 2017, 5:24 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63842/ > ----------------------------------------------------------- > > (Updated Nov. 17, 2017, 5:24 p.m.) > > > Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht. > > > Repository: mesos > > > Description > ------- > > During reconcilation we might be required to remove non-terminal offer > operations from bookkeeping. > > > Diffs > ----- > > src/master/master.hpp a309fc78ee2613762f3d5d22ac7559afc7aac4a3 > src/master/master.cpp 2ddd67ada3731803b00883b6a1f32b20c1bb238f > src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 > > > Diff: https://reviews.apache.org/r/63842/diff/2/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
