Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-07-11 Thread Anindya Sinha
> On July 11, 2016, 4:31 p.m., Neil Conway wrote: > > src/slave/slave.cpp, line 4778 > > > > > > Should we also remove the target resources in the case when target and > > checkpointed resources are the same?

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-07-11 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48313/#review141657 --- Fix it, then Ship it! src/slave/slave.cpp (line 2506)

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-07-01 Thread Anindya Sinha
> On June 23, 2016, 5:35 p.m., Jiang Yan Xu wrote: > > src/slave/state.hpp, line 271 > > > > > > It looks a bit odd that we are explicitly initiating `target` but not > > `resources`. > > > > Note that

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-07-01 Thread Anindya Sinha
> On June 20, 2016, 5:16 p.m., Jiang Yan Xu wrote: > > In terms of testing, if we don't crash the agent within > > `syncCheckpointedResources()` but rather return a failure when its fails > > during recovery, we can capture this in `Slave::__recover` and verify the > > failed future right? >

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-07-01 Thread Anindya Sinha
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48313/ --- (Updated July 1, 2016, 9:39 p.m.) Review request for mesos, Neil Conway and

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-23 Thread Jiang Yan Xu
> On June 20, 2016, 10:16 a.m., Jiang Yan Xu wrote: > > In terms of testing, if we don't crash the agent within > > `syncCheckpointedResources()` but rather return a failure when its fails > > during recovery, we can capture this in `Slave::__recover` and verify the > > failed future right?

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-23 Thread Jiang Yan Xu
> On June 8, 2016, 6:28 a.m., Neil Conway wrote: > > Overall seems like a reasonable approach. > > > > One thing that isn't clear to me: what is the advantage of updating the > > checkpoint to reflect any partial work that was done before exiting? It > > seems that adds a bunch of complexity

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-23 Thread Jiang Yan Xu
> On June 20, 2016, 10:16 a.m., Jiang Yan Xu wrote: > > src/slave/slave.cpp, line 2513 > > > > > > The first argument is already a member variable and doesn't need to be > > passed around right? > > > >

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-23 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48313/#review139100 --- src/slave/paths.hpp (lines 86 - 89)

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-23 Thread Anindya Sinha
> On June 8, 2016, 1:28 p.m., Neil Conway wrote: > > Overall seems like a reasonable approach. > > > > One thing that isn't clear to me: what is the advantage of updating the > > checkpoint to reflect any partial work that was done before exiting? It > > seems that adds a bunch of complexity

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-23 Thread Jiang Yan Xu
> On June 8, 2016, 6:28 a.m., Neil Conway wrote: > > Overall seems like a reasonable approach. > > > > One thing that isn't clear to me: what is the advantage of updating the > > checkpoint to reflect any partial work that was done before exiting? It > > seems that adds a bunch of complexity

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-20 Thread Anindya Sinha
> On June 20, 2016, 5:16 p.m., Jiang Yan Xu wrote: > > src/slave/slave.cpp, line 2513 > > > > > > The first argument is already a member variable and doesn't need to be > > passed around right? > > > >

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-20 Thread Anindya Sinha
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48313/ --- (Updated June 20, 2016, 11:41 p.m.) Review request for mesos, Neil Conway and

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-20 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48313/#review138314 --- In terms of testing, if we don't crash the agent within

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-15 Thread Anindya Sinha
> On June 8, 2016, 1:28 p.m., Neil Conway wrote: > > Overall seems like a reasonable approach. > > > > One thing that isn't clear to me: what is the advantage of updating the > > checkpoint to reflect any partial work that was done before exiting? It > > seems that adds a bunch of complexity

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-15 Thread Anindya Sinha
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48313/ --- (Updated June 15, 2016, 11 p.m.) Review request for mesos, Neil Conway and

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-14 Thread Neil Conway
> On June 8, 2016, 1:28 p.m., Neil Conway wrote: > > Overall seems like a reasonable approach. > > > > One thing that isn't clear to me: what is the advantage of updating the > > checkpoint to reflect any partial work that was done before exiting? It > > seems that adds a bunch of complexity

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-13 Thread Anindya Sinha
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48313/ --- (Updated June 13, 2016, 9:03 p.m.) Review request for mesos, Neil Conway and

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-13 Thread Anindya Sinha
> On June 8, 2016, 1:28 p.m., Neil Conway wrote: > > Overall seems like a reasonable approach. > > > > One thing that isn't clear to me: what is the advantage of updating the > > checkpoint to reflect any partial work that was done before exiting? It > > seems that adds a bunch of complexity

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-13 Thread Neil Conway
> On June 8, 2016, 1:28 p.m., Neil Conway wrote: > > Overall seems like a reasonable approach. > > > > One thing that isn't clear to me: what is the advantage of updating the > > checkpoint to reflect any partial work that was done before exiting? It > > seems that adds a bunch of complexity

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-09 Thread Anindya Sinha
> On June 8, 2016, 1:28 p.m., Neil Conway wrote: > > Overall seems like a reasonable approach. > > > > One thing that isn't clear to me: what is the advantage of updating the > > checkpoint to reflect any partial work that was done before exiting? It > > seems that adds a bunch of complexity

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-09 Thread Neil Conway
> On June 8, 2016, 1:28 p.m., Neil Conway wrote: > > Overall seems like a reasonable approach. > > > > One thing that isn't clear to me: what is the advantage of updating the > > checkpoint to reflect any partial work that was done before exiting? It > > seems that adds a bunch of complexity

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-08 Thread Anindya Sinha
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48313/ --- (Updated June 9, 2016, 12:22 a.m.) Review request for mesos, Neil Conway and

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-08 Thread Anindya Sinha
> On June 8, 2016, 1:28 p.m., Neil Conway wrote: > > Overall seems like a reasonable approach. > > > > One thing that isn't clear to me: what is the advantage of updating the > > checkpoint to reflect any partial work that was done before exiting? It > > seems that adds a bunch of complexity

Re: Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-08 Thread Neil Conway
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48313/#review136638 --- Overall seems like a reasonable approach. One thing that isn't

Review Request 48313: Creation and deletion of persistent volumes across agent restart.

2016-06-06 Thread Anindya Sinha
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48313/ --- Review request for mesos. Bugs: MESOS-5448