mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam B" <a...@mesosphere.io>
Subject Re: Review Request 39338: Added code that appends the fetcher log to the agent log upon fetcher failure.
Date Fri, 16 Oct 2015 09:00:51 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message