> On Jan. 17, 2018, 8:44 a.m., Benjamin Bannier wrote: > > src/tests/storage_local_resource_provider_tests.cpp > > Lines 1656 (patched) > > <https://reviews.apache.org/r/65039/diff/2/?file=1936333#file1936333line1656> > > > > Would it make sense to have the whole test run with paused clock? > > Greg Mann wrote: > I need to leave a comment here on why I do this. Unfortunately, the > standalone container running the CSI plugin complicates things a bit when > running with the clock paused. The SLRP runs a `loop()` which looks for the > CSI container's domain socket; once the loop finds it, SLRP init continues. > Ideally, we would obtain a `Future` which is fulfilled when that container > comes online; at that point we could advance the clock enough for the SLRP's > loop to execute, and SLRP initialization would continue. However, I don't > think there is a way to obtain such a `Future`. I think the easiest way > around this is to resume the clock while the CSI container is launched, but I > need a nice comment here explaining the need for this.
Another option could be to poll `containerizer->containers()` periodically and wait for a container to appear, but resuming/pausing the clock is more concise, so I would prefer to leave as-is. > On Jan. 17, 2018, 8:44 a.m., Benjamin Bannier wrote: > > src/tests/storage_local_resource_provider_tests.cpp > > Lines 1797 (patched) > > <https://reviews.apache.org/r/65039/diff/2/?file=1936333#file1936333line1797> > > > > This seems unneeded. If the clock isn't resumed at the end of the test, teardown will not complete successfully. The destruction of containers in `Slave::~Slave()` in 'cluster.cpp' requires the clock to be running in order to finish. - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65039/#review195539 ----------------------------------------------------------- On Jan. 18, 2018, 1:57 a.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65039/ > ----------------------------------------------------------- > > (Updated Jan. 18, 2018, 1:57 a.m.) > > > Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, > and Jie Yu. > > > Bugs: MESOS-8373 > https://issues.apache.org/jira/browse/MESOS-8373 > > > Repository: mesos > > > Description > ------- > > This patch adds > StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation > in order to verify that reconciliation is performed correctly > when an operation is dropped on its way from the master to the > agent. > > > Diffs > ----- > > src/tests/storage_local_resource_provider_tests.cpp > bbfe95e9818f25fdd5405db3ad2fe355e023f743 > > > Diff: https://reviews.apache.org/r/65039/diff/3/ > > > Testing > ------- > > sudo make check > > > Thanks, > > Greg Mann > >
