mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 65039: Tested reconciliation when operation is dropped en route to agent.
Date Thu, 18 Jan 2018 09:23:08 GMT


> On Jan. 17, 2018, 9: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.
> 
> Greg Mann wrote:
>     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.

That sounds like a bug in `cluster::Slave::~Slave`, and I do not see this explicit `resume`
pattern elsewhere a lot either.

Adding an explicit `resume` also does not help in cases where an assertion before the `resume`
leads to the rest of the test being skipped. The only way to make sure it is called is to
invoke it in some destructor RAII style.

I'd suggest to drop the `resume` here for consistency, and instead fix the destructor of `~Slave`
to make it self-sufficient (either file a ticket or add a cleanup).


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65039/#review195539
-----------------------------------------------------------


On Jan. 18, 2018, 2: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, 2: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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message