mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Timothy Chen" <tnac...@apache.org>
Subject Re: Review Request 24475: Add new Docker configurations
Date Mon, 11 Aug 2014 23:29:45 GMT


> On Aug. 11, 2014, 4:34 a.m., Benjamin Hindman wrote:
> > src/slave/containerizer/docker.cpp, lines 246-247
> > <https://reviews.apache.org/r/24475/diff/2/?file=657110#file657110line246>
> >
> >     It doesn't look like you're every putting containers into 'fetching'! Also,
this is starting to get a lot more complicated, so I'd suggest adding both comments and CHECKs
as much as possible (a CHECK would have caught your bug!). As in, IIUC, now you're expecting
that 'fetching' and 'promises' are disjoint, and that a container "transitions" through from
"fetching" to "launched", i.e., from the 'fetching' set to the 'promises' set. Am I correct?
Let's make it easier for other people to understand these semantics.
> >     
> >     Also, I'm assuming that you didn't want to just put the container in 'promises'
until after fetching is complete because it was easier to "cleanup" after fetching (whether
it's successful or an failure) by just removing from 'fetching', but I wanted to mention that
you can use .onFailed to cleanup 'promises' if fetching fails and otherwise just fall through
to the next continuation where 'promises' still has the container.

I originally don't want to put in promises as I felt I don't want to let docker wait yet as
it wasn't clear what I should put in the promise. But thinking about it as you said I'll just
use promises and clean up in _fetch.


- Timothy


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


On Aug. 10, 2014, 7:39 a.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24475/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2014, 7:39 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ian Downes, and Jie Yu.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added new DockerInfo for future docker options, and allow command uris to be fetched
and mapped into docker before launching docker container.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto cc9f20e 
>   src/docker/docker.hpp 98b2d60 
>   src/docker/docker.cpp 1cba381 
>   src/slave/containerizer/containerizer.hpp 02754cd 
>   src/slave/containerizer/containerizer.cpp c91ba38 
>   src/slave/containerizer/docker.cpp 904cdd3 
>   src/slave/containerizer/mesos/containerizer.cpp 694c9d1 
>   src/slave/slave.cpp 787bd05 
>   src/tests/docker_containerizer_tests.cpp a559836 
>   src/tests/docker_tests.cpp 4ef1df4 
> 
> Diff: https://reviews.apache.org/r/24475/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>


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