mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ilya Pronin <ipro...@twopensource.com>
Subject Re: Review Request 64098: Send status updates when agent re-registers.
Date Mon, 11 Dec 2017 21:14:00 GMT


> On Dec. 7, 2017, 1:36 p.m., Ilya Pronin wrote:
> > src/tests/master_tests.cpp
> > Lines 7477 (patched)
> > <https://reviews.apache.org/r/64098/diff/11/?file=1908759#file1908759line7499>
> >
> >     Ditto re replicated log based registry.
> 
> Megha Sharma wrote:
>     I added this specifically to tests that have partitioned agents re-registering to
ensure that the additional status updates still happen with the registry in use. That we don't
have to look through the tests to see if the master restarted or not and have the test always
do the right thing. Also I don't see any harm being done here by using registry, for one it
keeps the master configuration consistent across tests.
> 
> Megha Sharma wrote:
>     Ilya, I am dropping these issues based on my above argument. Please feel free to
re-open them if needed.

I agree that using the replicated log based registry in these tests does no harm. Here instead
of changing a specific test when it's required, you did a blanket change in a bunch of (now)
unaffected tests. I just prefer to keep tests minimal to avoid accidentally hiding something.
But that's just my preference, feel free to ignore it.


> On Dec. 7, 2017, 1:36 p.m., Ilya Pronin wrote:
> > src/tests/partition_tests.cpp
> > Lines 820 (patched)
> > <https://reviews.apache.org/r/64098/diff/11/?file=1908760#file1908760line843>
> >
> >     Ditto re replicated log based registry.
> 
> Megha Sharma wrote:
>     Test description says that master did failover here `PartitionedSlaveReregistrationMasterFailover`.

Yep, found it. Sorry.


> On Dec. 7, 2017, 1:36 p.m., Ilya Pronin wrote:
> > src/tests/upgrade_tests.cpp
> > Lines 106 (patched)
> > <https://reviews.apache.org/r/64098/diff/11/?file=1908763#file1908763line106>
> >
> >     Ditto re replicated log based registry.
> 
> Megha Sharma wrote:
>     Yes it does, look for `StartMaster(masterFlags)` in this test, it will appear twice.

Found it. Seems that my search skills failed me, sorry.


- Ilya


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


On Dec. 8, 2017, 10:42 a.m., Megha Sharma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64098/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2017, 10:42 a.m.)
> 
> 
> Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6406
>     https://issues.apache.org/jira/browse/MESOS-6406
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Master will send task status updates to frameworks when an agent
> which has been previously removed by the master for being unreachable
> or is unknown to the master due to the garbage collection of
> the unreachable and gone agents in the registry re-registers.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp dfe60ef670edcaefa0c1241df2e2870f650fcf9e 
>   src/tests/master_allocator_tests.cpp 3400d70bb0ba564eac43c4639eee0efd4d8059e6 
>   src/tests/master_tests.cpp 57eae320a7a398527cd3623c89bf67f319a8e955 
>   src/tests/partition_tests.cpp 31ebfe1655438eceae74d72a223df03a9dbd282d 
>   src/tests/persistent_volume_tests.cpp 4aa3c2e8b0f461cd78053707cff8bcb2e6f2b0d7 
>   src/tests/slave_recovery_tests.cpp f14c6ef69eb20a03454c8197df79b572a3c6d050 
>   src/tests/upgrade_tests.cpp 7f434dbba858f636719eec24e92b306b76430c4c 
> 
> 
> Diff: https://reviews.apache.org/r/64098/diff/12/
> 
> 
> Testing
> -------
> 
> with make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>


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