> On Oct. 16, 2015, 12: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.

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..."?


- Adam


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39338/#review102876
-----------------------------------------------------------


On Oct. 16, 2015, 1: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, 1: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
> 
>

Reply via email to