mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 65057: Tested that op status updates dropped en route to master are resent.
Date Thu, 18 Jan 2018 19:20:36 GMT

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




src/tests/storage_local_resource_provider_tests.cpp
Lines 2132 (patched)
<https://reviews.apache.org/r/65057/#comment275032>

    Maybe "To accomplish this:"



src/tests/storage_local_resource_provider_tests.cpp
Lines 2134 (patched)
<https://reviews.apache.org/r/65057/#comment275033>

    s/message//



src/tests/storage_local_resource_provider_tests.cpp
Lines 2172-2179 (patched)
<https://reviews.apache.org/r/65057/#comment275034>

    Could you also add a note here regarding why the order of these two is reversed? Chun-Hung
and I have such a comment in ours if you want to borrow it for consistency :)



src/tests/storage_local_resource_provider_tests.cpp
Lines 2189-2191 (patched)
<https://reviews.apache.org/r/65057/#comment275035>

    Could you leave a more verbose comment here as to why this is necessary? Chun-Hung and
my patches have one if you care to borrow for consistency :)



src/tests/storage_local_resource_provider_tests.cpp
Lines 2226 (patched)
<https://reviews.apache.org/r/65057/#comment275036>

    Do we need this, or just being careful? If it is needed, it might not be if we add a long
filter to the accept call.



src/tests/storage_local_resource_provider_tests.cpp
Lines 2278 (patched)
<https://reviews.apache.org/r/65057/#comment275037>

    Benjamin suggested on my RR that we figure out the issue in `Slave::~Slave()` that creates
the need for this resume. Looking at some other paused tests in the codebase, many of them
do not need to resume.
    
    I agree that it's a good idea for us to fix this in 'cluster.cpp' and eliminate the need
for this resume.


- Greg Mann


On Jan. 10, 2018, 12:25 a.m., Gaston Kleiman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65057/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2018, 12:25 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8363 and MESOS-8420
>     https://issues.apache.org/jira/browse/MESOS-8363
>     https://issues.apache.org/jira/browse/MESOS-8420
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds
> `StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate` that
> verifies that operation status updates are resent to the master after
> being dropped en route to it.
> 
> 
> Diffs
> -----
> 
>   src/tests/storage_local_resource_provider_tests.cpp bbfe95e9818f25fdd5405db3ad2fe355e023f743

> 
> 
> Diff: https://reviews.apache.org/r/65057/diff/4/
> 
> 
> Testing
> -------
> 
> `sudo bin/mesos-tests.sh --gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate'
--gtest_repeat=100 --gtest_break_on_failure` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>


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