> On Jan. 15, 2020, 2:55 p.m., Andrei Sekretenko wrote: > > src/slave/slave.cpp > > Line 6583 (original), 6583 (patched) > > <https://reviews.apache.org/r/72003/diff/1/?file=2207139#file2207139line6583> > > > > If we are not validating that `ExecutorInfo` of a task does not have > > `generated_by_agent` set, maybe we can at least forcibly clear it? either > > here or in `Slave::runTask()`, and document (in the previous patch) that > > agent **ignores** this field when received from outside? > > Benjamin Bannier wrote: > That would be possible, but I am unsure it would be worth it. We > currently do perform mutations on user-provided executors, the documentation > of the added field clearly indicates that it should not be set nor be relied > on by frameworks, and even if it was set it would only lead to potentially > misbehaving tasks without wider effect. > > I feel we shouldn't do this, but please reopen if you believe it is > critical for some reason.
Added rejection of framework-specified `generated_by_agent` in the previous patch. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72003/#review219270 ----------------------------------------------------------- On Jan. 15, 2020, 4:12 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, 4:12 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/2/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
