Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-18 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/#review119636 --- Fix it, then Ship it! src/slave/containerizer/docker.cpp (line

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-18 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/ --- (Updated Feb. 18, 2016, 6:16 p.m.) Review request for mesos and Jie Yu. Bugs:

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-18 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/ --- (Updated Feb. 18, 2016, 9:12 a.m.) Review request for mesos and Jie Yu. Bugs:

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-17 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/#review119501 --- src/slave/containerizer/docker.cpp (lines 1785 - 1790)

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-17 Thread Timothy Chen
> On Feb. 16, 2016, 11:46 p.m., Jie Yu wrote: > > src/slave/containerizer/docker.cpp, lines 1785-1790 > > > > > > Hum, I think we shouldn't remove the volumes if the container might be > > still running. So can we

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-17 Thread Timothy Chen
> On Feb. 16, 2016, 11:46 p.m., Jie Yu wrote: > > src/slave/containerizer/docker.cpp, lines 1701-1708 > > > > > > Why do you need to do this? Looks like we don't mount until pulling is > > done. Am I missing someth

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-17 Thread Timothy Chen
> On Feb. 16, 2016, 11:46 p.m., Jie Yu wrote: > > src/slave/containerizer/docker.cpp, lines 473-488 > > > > > > There's a os::exists check in linux fs isolator which you removed. This > > is fine because we won't up

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-17 Thread Timothy Chen
> On Feb. 16, 2016, 11:46 p.m., Jie Yu wrote: > > src/slave/containerizer/docker.cpp, lines 413-414 > > > > > > Should we ignore those volumes that have absolute/nested container path > > (like we did in linux fs is

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-17 Thread Timothy Chen
> On Feb. 16, 2016, 11:46 p.m., Jie Yu wrote: > > src/slave/containerizer/docker.cpp, lines 473-488 > > > > > > There's a os::exists check in linux fs isolator which you removed. This > > is fine because we won't up

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-17 Thread Timothy Chen
> On Feb. 16, 2016, 11:46 p.m., Jie Yu wrote: > > src/slave/containerizer/docker.cpp, lines 161-162 > > > > > > Can you wrap using 70 char width here? Sure, I thought we removed that restriction now? - Timothy -

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-16 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/#review119340 --- src/slave/containerizer/docker.cpp (lines 161 - 162)

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/ --- (Updated Feb. 16, 2016, 3:33 a.m.) Review request for mesos and Jie Yu. Bugs:

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/ --- (Updated Feb. 16, 2016, 3:01 a.m.) Review request for mesos and Jie Yu. Bugs:

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Timothy Chen
> On Feb. 16, 2016, 12:29 a.m., Jie Yu wrote: > > src/slave/containerizer/docker.cpp, lines 1707-1719 > > > > > > Do you need this? Looks like 'unmountPersistentVolumes' will take care > > of the umount. We do sti

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/#review119250 --- src/slave/containerizer/docker.cpp (line 396)

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/ --- (Updated Feb. 15, 2016, 3:39 p.m.) Review request for mesos and Jie Yu. Bugs:

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Timothy Chen
> On Feb. 15, 2016, 3:47 a.m., Guangya Liu wrote: > > src/slave/containerizer/docker.cpp, line 505 > > > > > > What about add `containerId` in the log message? It's not added in all the other places. I see you have

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-15 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/ --- (Updated Feb. 15, 2016, 3:38 p.m.) Review request for mesos and Jie Yu. Bugs:

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-14 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/#review119208 --- src/slave/containerizer/docker.cpp (line 505)

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-14 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/#review119176 --- Patch looks great! Reviews applied: [43015] Passed command: expo

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-14 Thread Timothy Chen
> On Feb. 8, 2016, 8:44 p.m., Jie Yu wrote: > > I realized a tricky part in the recovery path while dealing with potential > > conflicts with MesosContainerizer (LinuxFilesystemIsolator). For instance, > > will the LinuxFilesystemIsolator try to umount some persistent volume > > mounts that be

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-14 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/ --- (Updated Feb. 14, 2016, 8:38 a.m.) Review request for mesos and Jie Yu. Bugs:

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-08 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/ --- (Updated Feb. 9, 2016, 2:32 a.m.) Review request for mesos and Jie Yu. Bugs:

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-08 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/#review118285 --- I realized a tricky part in the recovery path while dealing with p

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-01-30 Thread Timothy Chen
> On Jan. 30, 2016, 7:31 p.m., Joerg Schad wrote: > > src/docker/executor.cpp, line 135 > > > > > > I am not sure why we are printing this, but shouldn't we at least check > > for errors on os::shell? Thought I alr

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-01-30 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/ --- (Updated Jan. 30, 2016, 10:43 p.m.) Review request for mesos and Jie Yu. Repo

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-01-30 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/#review117125 --- src/slave/containerizer/docker.cpp (line 992)

Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-01-30 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/#review117116 --- Thanks for fixing this! Is there a ticket with the issue? I will d

Review Request 43015: Fixed persistent volumes with docker tasks.

2016-01-30 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43015/ --- Review request for mesos and Jie Yu. Repository: mesos Description --- F