> On Jan. 15, 2020, 2:55 p.m., Andrei Sekretenko wrote: > > src/slave/slave.cpp > > Lines 6593 (patched) > > <https://reviews.apache.org/r/72003/diff/1/?file=2207139#file2207139line6593> > > > > The logic "executor is generated by agent -> master receives no > > updates" is not covered by any of the local tests; if we are not > > introducing the test right now, consider adding TODO (and, probably, a > > separate ticket). > > > > Personally, I'm more or less OK with `upgradeExecutorInfo(...)` not > > covered: the necessary setup is difficult to create, that code is hacky > > anyway, and we want to phase it out.
Added a `TODO` at the usage site in `Slave::doReliableRegistration`. > On Jan. 15, 2020, 2:55 p.m., Andrei Sekretenko wrote: > > src/slave/slave.cpp > > Line 10601 (original), 10626 (patched) > > <https://reviews.apache.org/r/72003/diff/1/?file=2207139#file2207139line10626> > > > > Maybe add/move CHECK_NOTNULL to this line, so that it does not rely on > > the fact that `slave` is **declared** before `info`? I added another `CHECK_NOTNULL` here. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72003/#review219270 ----------------------------------------------------------- On Jan. 15, 2020, 12:25 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72003/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2020, 12:25 p.m.) > > > Review request for mesos, Andrei Sekretenko and Benjamin Mahler. > > > Bugs: MESOS-10084 > https://issues.apache.org/jira/browse/MESOS-10084 > > > Repository: mesos > > > Description > ------- > > This patch modifies the agent code so it uses > `ExecutorInfo::generated_by_agent` instead of the previous heuristic to > detect agent-generated executors for command tasks. For that we add > recovery logic for upgrade scenarios to inject a correct value for > recovered executors, and set the appropriate value for newly created > executors. > > > Diffs > ----- > > src/slave/slave.hpp 3d191dcbdd58e851ad5e86fbb01a890407409c64 > src/slave/slave.cpp 0005971717f5b90da368b45caad8e209ada95fa5 > > > Diff: https://reviews.apache.org/r/72003/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
