> On Dec. 19, 2016, 12: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. > > Anindya Sinha wrote: > 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). > ```
Alright then. But then we'd better adjust the definition of `Shard::RUNNING`. :) - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45962/#review159590 ----------------------------------------------------------- On Jan. 9, 2017, 2:37 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45962/ > ----------------------------------------------------------- > > (Updated Jan. 9, 2017, 2:37 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 > >
