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