> On Dec. 19, 2016, 8:01 a.m., Jiang Yan Xu wrote: > > src/examples/persistent_volume_framework.cpp, lines 244-248 > > <https://reviews.apache.org/r/45962/diff/27-28/?file=1581707#file1581707line244> > > > > Good find. However I think the fix would be cleaner if we just update > > the state of the shard to `Shard::STAGING` in this block code (in > > `resourceOffers()`), after the shard has launched all the required tasks. > > > > Note that this is consistent with the definition of the state: > > > > ``` > > RUNNING, // The shard is running (i.e., all tasks have launched). > > ``` > > > > The definition suggests we should update the state after "all tasks > > have launched". In this framework we assume all launched tasks are going to > > successful run.
I think you meant `Shard::RUNNING`. I think changing state to `Shard::RUNNING` (and `Shard::TERMINATING`) is better in a single place, i.e. when the task status updates is received. I understand this is a test framework when we expect all launched tasks to be successful but IMO, making state changes when we are sure of the event (successfully launching or terminating a task) is preferable. I would prefer to restate `Shard::RUNNING` as: ``` RUNNING, // The shard is running (i.e., all tasks have successfully launched). ``` - Anindya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45962/#review159590 ----------------------------------------------------------- On Dec. 12, 2016, 10:35 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45962/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2016, 10:35 p.m.) > > > Review request for mesos, Greg Mann, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-4431 > https://issues.apache.org/jira/browse/MESOS-4431 > > > Repository: mesos > > > Description > ------- > > Updated a persistent volume test framework to include shared volumes. > > > Diffs > ----- > > src/examples/persistent_volume_framework.cpp > 9d45bb496c4cf378af429b5aa970bf81450e482a > > Diff: https://reviews.apache.org/r/45962/diff/ > > > Testing > ------- > > New test framework for shared resources added. > Tests successful. > > > Thanks, > > Anindya Sinha > >
