> On Oct. 16, 2015, 7:29 a.m., Adam B wrote: > > src/slave/containerizer/fetcher.cpp, line 794 > > <https://reviews.apache.org/r/39338/diff/1/?file=1098840#file1098840line794> > > > > What's this string parameter that you're ignoring? If it's the Failure > > message, I'd think you'd want to log that too. > > > > Should this lambda return a Future<Nothing> too? Otherwise, how will it > > run the following onAny cleanup below? > > Bernd Mathiske wrote: > onFailed() takes a parameter of this type: > > typedef lambda::function<void(const std::string&)> FailedCallback; > > The string parameter of this callback receives the error message from the > failed future as argument when onFailed is called. The latter is declared > this way: > > const Future<T>& onFailed(FailedCallback&& callback) const; > > The future you refer to is passed through every onFailed call itself, not > through its callback, which must return void. > > Bernd Mathiske wrote: > The failure message has already been logged at this point. > > Adam B wrote: > Ok, that makes more sense to me. Just trying to figure out what happens > to the Failure message if you're ignoring it in onFailed(). So onFailed() > automatically passes on the (failed) Future to the onAny() below, and after > that it becomes the final object in the `return fetcherSubprocess...` > statement on line 779. Then whatever called FetcherProcess::Run() waits on > that future and hopefully logs the Failure message somewhere? So will you see > the "Begin fetcher log..." before "Failed to fetch all URIs for container..."?
It should compile without needing to have 'const string&' as an argument, was that not the case bernd? - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39338/#review102876 ----------------------------------------------------------- On Oct. 16, 2015, 8:56 a.m., Bernd Mathiske wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39338/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2015, 8:56 a.m.) > > > Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and > Timothy Chen. > > > Bugs: MESOS-3743 > https://issues.apache.org/jira/browse/MESOS-3743 > > > Repository: mesos > > > Description > ------- > > Added an onFailed() clause to the inspection of the fetcher subprocess run. > This clause copies the fetcher log from <task sandbox>/stderr and appends it > to the agent log. > > This is to facilitate debugging spurious fetch failures in production or CI. > > Similar, but not the same: https://reviews.apache.org/r/37813/ (see > MESOS-3743 for an explanation). > > > Diffs > ----- > > src/slave/containerizer/fetcher.cpp > 2b2298c329ed5fb5863cb0fed1491e478c3e5d5a > > Diff: https://reviews.apache.org/r/39338/diff/ > > > Testing > ------- > > Ran make check. As expected no change in behavior. > When I modified the fetcher to fail, > I observed the expected extra output. > > > Thanks, > > Bernd Mathiske > >
