> On May 3, 2018, 3:52 p.m., Jan Schlicht wrote: > > src/tests/resource_provider_manager_tests.cpp > > Lines 1130-1131 (patched) > > <https://reviews.apache.org/r/66931/diff/1/?file=2016240#file2016240line1130> > > > > Nit: `disconnected` is called asynchronously. The `Clock::settle()` > > below will make sure that it is called. Not having the settle would > > introduce another flakyness though. To be extra sure, we could set a future > > and await that one before we reset `resourceProvider` with a new instance. > > Chun-Hung Hsiao wrote: > +1 for awaiting a future instead of using `Clock::settle()`. This would > make the intention more clear.
Added an explicition expectation on disconnected before restarting the agent. @chun The `Clock::settle` is still needed for the agent registration. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66931/#review202348 ----------------------------------------------------------- On May 4, 2018, 12:37 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66931/ > ----------------------------------------------------------- > > (Updated May 4, 2018, 12:37 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. > > > Bugs: MESOS-8874 > https://issues.apache.org/jira/browse/MESOS-8874 > > > Repository: mesos > > > Description > ------- > > We previously did not make ensure that after the simulated agent > failover in > `ResourceProviderManagerHttpApiTest.ResubscribeResourceProvider` the > mock resource provider created as part of the test did not reconnect > to the restarted agent (as opposed to the newly initialized resource > provider). This lead to unmet test expectations. > > With this patch we now explicitly tear down the mock resource provider > after we have detected that the agent went away to prevent the race. > > > Diffs > ----- > > src/tests/resource_provider_manager_tests.cpp > e8ca377fd0a927b99fdaf6a8ee0139025a41298e > > > Diff: https://reviews.apache.org/r/66931/diff/2/ > > > Testing > ------- > > `make check` > > Ran the test repeatedly under high system load without triggering the issue > again with this patch. > > > Thanks, > > Benjamin Bannier > >
