mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ian Downes" <ian.dow...@gmail.com>
Subject Re: Review Request 34737: Added test to verify fix for MESOS-2771
Date Thu, 28 May 2015 18:31:56 GMT

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

Ship it!


Looks good. Main issue is that this can and should be a generally useful test not one to verify
we don't segfault after the fix. Please reword comments and add expectations for response
output. Other comments are stylistic and most are copied from the other monitor test.


src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137178>

    I'd state this more generally as a test for correct handling of the statistics.json endpoint
when monitoring of a container is stopped, i.e., the test is useful independently of the bug
revealed in MESOS-2771.



src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137171>

    s/Setup/Set up/



src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137173>

    No yours but add a macro to tests/mesos.hpp?



src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137172>

    Can you use the DEFAULT_EXECUTOR_ID macro from tests/mesos.hpp?



src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137174>

    DEFAULT_EXECUTOR_INFO?



src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137176>

    s/setup/set up/



src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137179>

    Again, not yours but
    
    Single line?
    
    AWAIT_READY(monitor.start())?



src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137184>

    s/Cause/Induce a/



src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137186>

    "Stop monitoring the container"?



src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137185>

    AWAIT_READY()?



src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137188>

    Reword this comment to make it general and not specific to the bug in MESOS-2771.



src/tests/monitor_tests.cpp
<https://reviews.apache.org/r/34737/#comment137182>

    To make this test general, please check the expected output in response, i.e., the container
will *not* be included.


- Ian Downes


On May 27, 2015, 4:14 p.m., Niklas Nielsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34737/
> -----------------------------------------------------------
> 
> (Updated May 27, 2015, 4:14 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2771
>     https://issues.apache.org/jira/browse/MESOS-2771
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added test to verify fix for MESOS-2771
> 
> 
> Diffs
> -----
> 
>   src/tests/monitor_tests.cpp ca3b7f4a85824697a4e6701285f69ba337506147 
> 
> Diff: https://reviews.apache.org/r/34737/diff/
> 
> 
> Testing
> -------
> 
> Test verifies fix for MESOS-2771, where references to executor infos and container ids
could get invalidated if monitoring of a container stopped during a statistics collection.
> 
> Tested with make check before and after fix.
> 
> Before:
> 
> I0527 15:51:57.943672 388640768 process.cpp:2201] Cleaning up __http__(1)@10.0.68.255:50624
> I0527 15:51:57.943717 387567616 process.cpp:2094] Resuming __gc__@10.0.68.255:50624 at
2015-05-27 22:51:57.943728896+00:00
> PC: @        0x11122bb3d std::__1::operator<< <>()
> *** SIGSEGV (@0x0) received by PID 42293 (TID 0x7fff7cdb2300) stack trace: ***
>     @     0x7fff8eaebf1a _sigtramp
>     @     0x7fd785800ee8 (unknown)
>     @        0x1112e97d0 mesos::operator<<()
>     @        0x111b41731 mesos::internal::slave::ResourceMonitorProcess::usage()::$_0::operator()()
>     @        0x111b415ed _ZZNK7process6FutureIN5mesos8internal5slave15ResourceMonitor5UsageEE8onFailedIZNS3_22ResourceMonitorProcess5usageENS1_11ContainerIDEE3$_0vEERKS6_OT_NS6_6PreferEENUlRKNSt3__112basic_stringIcNSG_11char_traitsIcEENSG_9allocatorIcEEEEE_clESO_
>     @        0x111b4152c _ZNSt3__110__function6__funcIZNK7process6FutureIN5mesos8internal5slave15ResourceMonitor5UsageEE8onFailedIZNS6_22ResourceMonitorProcess5usageENS4_11ContainerIDEE3$_0vEERKS9_OT_NS9_6PreferEEUlRKNS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEE_NSM_ISR_EEFvSQ_EEclESQ_
>     @        0x112106544 std::__1::function<>::operator()()
>     @        0x1112067ab _ZN7process8internal3runINSt3__18functionIFvRKNS2_12basic_stringIcNS2_11char_traitsIcEENS2_9allocatorIcEEEEEEEJRS9_EEEvRKNS2_6vectorIT_NS7_ISG_EEEEDpOT0_
   @        0x111b86400 process::Future<>::fail()
>     @        0x111ba05bd process::Promise<>::fail()
>     @        0x111b9722c process::internal::thenf<>()
>     @        0x111b9f823 _ZNSt3__110__function6__funcINS_6__bindIPFvRKNS_8functionIFN7process6FutureIN5mesos8internal5slave15ResourceMonitor5UsageEEERKNS6_18ResourceStatisticsEEEERKNS_10shared_ptrINS4_7PromiseISA_EEEERKNS5_ISC_EEEJSI_RSM_RNS_12placeholders4__phILi1EEEEEENS_9allocatorISZ_EEFvSR_EEclESR_
>     @        0x1121db254 std::__1::function<>::operator()()
>     @        0x111b9c90d _ZZNK7process6FutureIN5mesos18ResourceStatisticsEE5onAnyIRNSt3__18functionIFvRKS3_EEEvEES8_OT_NS3_6PreferEENUlS8_E_clES8_
>     @        0x111b9c84c _ZNSt3__110__function6__funcIZNK7process6FutureIN5mesos18ResourceStatisticsEE5onAnyIRNS_8functionIFvRKS6_EEEvEESA_OT_NS6_6PreferEEUlSA_E_NS_9allocatorISH_EESB_EclESA_
>     @        0x10e73b0d4 std::__1::function<>::operator()()
>     @        0x10e73a50b _ZN7process8internal3runINSt3__18functionIFvRKNS_6FutureIN5mesos18ResourceStatisticsEEEEEEJRS7_EEEvRKNS2_6vectorIT_NS2_9allocatorISE_EEEEDpOT0_
>     @        0x10efd83c1 process::Future<>::fail()
>     @        0x10efdd49d _ZZNK7process6FutureIN5mesos18ResourceStatisticsEE8onFailedINSt3__16__bindIMS3_FbRKNS5_12basic_stringIcNS5_11char_traitsIcEENS5_9allocatorIcEEEEEJRS3_RNS5_12placeholders4__phILi1EEEEEEbEERKS3_OT_NS3_6PreferEENUlSE_E_clESE_
>     @        0x10efdd1fc _ZNSt3__110__function6__funcIZNK7process6FutureIN5mesos18ResourceStatisticsEE8onFailedINS_6__bindIMS6_FbRKNS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEEJRS6_RNS_12placeholders4__phILi1EEEEEEbEERKS6_OT_NS6_6PreferEEUlSG_E_NSC_ISU_EEFvSG_EEclESG_
>     @        0x10e5b2304 std::__1::function<>::operator()()
>     @        0x10efdb9f9 process::Future<>::onFailed()
>     @        0x10efdb6c8 _ZNK7process6FutureIN5mesos18ResourceStatisticsEE8onFailedINSt3__16__bindIMS3_FbRKNS5_12basic_stringIcNS5_11char_traitsIcEENS5_9allocatorIcEEEEEJRS3_RNS5_12placeholders4__phILi1EEEEEEbEERKS3_OT_NS3_6PreferE
>     @        0x10efd9145 _ZNK7process6FutureIN5mesos18ResourceStatisticsEE8onFailedINSt3__16__bindIMS3_FbRKNS5_12basic_stringIcNS5_11char_traitsIcEENS5_9allocatorIcEEEEEJRS3_RNS5_12placeholders4__phILi1EEEEEEEERKS3_OT_
>     @        0x10efd8b43 process::Promise<>::associate()
>     @        0x10efd81bd process::Promise<>::set()
>     @        0x10efd684e mesos::internal::tests::MonitorTest_UsageFailure_Test::TestBody()
>     @        0x10f4155b3 testing::internal::HandleSehExceptionsInMethodIfSupported<>()
>     @        0x10f3fd667 testing::internal::HandleExceptionsInMethodIfSupported<>()
>     @        0x10f3d7135 testing::Test::Run()
>     @        0x10f3d80eb testing::TestInfo::Run()
>     @        0x10f3d8cd7 testing::TestCase::Run()
> ^C^C
> [1]    42293 segmentation fault  GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="MonitorTest.UsageFailure"
> 
> After:
> 
> [ RUN      ] MonitorTest.UsageFailure
> I0527 15:55:41.984792 2094736128 process.cpp:2084] Spawned process __limiter__(2)@10.0.68.255:50644
> I0527 15:55:41.984801 242876416 process.cpp:2094] Resuming __limiter__(2)@10.0.68.255:50644
at 2015-05-27 22:55:41.984818944+00:00
> I0527 15:55:41.984833 243949568 process.cpp:2094] Resuming monitor@10.0.68.255:50644
at 2015-05-27 22:55:41.984848128+00:00
> I0527 15:55:41.984894 2094736128 process.cpp:2084] Spawned process monitor@10.0.68.255:50644
> I0527 15:55:41.984953 242876416 process.cpp:2094] Resuming help@10.0.68.255:50644 at
2015-05-27 22:55:41.984967936+00:00
> I0527 15:55:41.985086 245559296 process.cpp:2094] Resuming monitor@10.0.68.255:50644
at 2015-05-27 22:55:41.985101824+00:00
> I0527 15:55:41.985436 242876416 process.cpp:2094] Resuming __gc__@10.0.68.255:50644 at
2015-05-27 22:55:41.985452800+00:00
> I0527 15:55:41.985443 246632448 process.cpp:2094] Resuming __latch__(1)@10.0.68.255:50644
at 2015-05-27 22:55:41.985459200+00:00
> I0527 15:55:41.985455 2094736128 process.cpp:2084] Spawned process __latch__(1)@10.0.68.255:50644
> I0527 15:55:41.985589 2094736128 process.cpp:2084] Spawned process __waiter__(1)@10.0.68.255:50644
> I0527 15:55:41.985604 246095872 process.cpp:2094] Resuming __waiter__(1)@10.0.68.255:50644
at 2015-05-27 22:55:41.985619968+00:00
> I0527 15:55:41.986281 244486144 process.cpp:2094] Resuming monitor@10.0.68.255:50644
at 2015-05-27 22:55:41.986298112+00:00
> I0527 15:55:41.986320 244486144 process.cpp:2704] Handling HTTP event for process 'monitor'
with path: '/monitor/statistics.json'
> I0527 15:55:41.986418 243412992 process.cpp:2094] Resuming __http__(1)@10.0.68.255:50644
at 2015-05-27 22:55:41.986438912+00:00
> I0527 15:55:41.986419 245559296 process.cpp:2094] Resuming __gc__@10.0.68.255:50644 at
2015-05-27 22:55:41.986437120+00:00
> I0527 15:55:41.986429 244486144 process.cpp:2084] Spawned process __http__(1)@10.0.68.255:50644
> I0527 15:55:41.986556 245559296 process.cpp:2094] Resuming __http__(1)@10.0.68.255:50644
at 2015-05-27 22:55:41.986571008+00:00
> I0527 15:55:41.986595 246095872 process.cpp:2094] Resuming __limiter__(2)@10.0.68.255:50644
at 2015-05-27 22:55:41.986610944+00:00
> I0527 15:55:41.986896 243412992 process.cpp:2094] Resuming __latch__(1)@10.0.68.255:50644
at 2015-05-27 22:55:41.986912000+00:00
> I0527 15:55:41.986939 243412992 process.cpp:2201] Cleaning up __latch__(1)@10.0.68.255:50644
> I0527 15:55:41.987043 242876416 process.cpp:2094] Resuming __waiter__(1)@10.0.68.255:50644
at 2015-05-27 22:55:41.987060992+00:00
> I0527 15:55:41.987062 245022720 process.cpp:2094] Resuming __gc__@10.0.68.255:50644 at
2015-05-27 22:55:41.987087104+00:00
> I0527 15:55:41.987117 242876416 process.cpp:2201] Cleaning up __waiter__(1)@10.0.68.255:50644
> I0527 15:55:41.987154 246632448 process.cpp:2094] Resuming (1)@10.0.68.255:50644 at 2015-05-27
22:55:41.987164928+00:00
> I0527 15:55:41.987150 243412992 process.cpp:2094] Resuming __gc__@10.0.68.255:50644 at
2015-05-27 22:55:41.987164928+00:00
> I0527 15:55:41.987179 244486144 process.cpp:2084] Spawned process (1)@10.0.68.255:50644
> W0527 15:55:41.987236 2094736128 monitor.cpp:127] Failed to get resource usage for  container
container for executor executor of framework framework: Injected failure
> I0527 15:55:41.987267 246632448 process.cpp:2201] Cleaning up (1)@10.0.68.255:50644
> I0527 15:55:41.987316 242876416 process.cpp:2094] Resuming __gc__@10.0.68.255:50644 at
2015-05-27 22:55:41.987325952+00:00
> I0527 15:55:41.987344 243949568 process.cpp:2094] Resuming __latch__(2)@10.0.68.255:50644
at 2015-05-27 22:55:41.987360000+00:00
> I0527 15:55:41.987383 2094736128 process.cpp:2084] Spawned process __latch__(2)@10.0.68.255:50644
> I0527 15:55:41.987475 2094736128 process.cpp:2084] Spawned process __waiter__(2)@10.0.68.255:50644
> I0527 15:55:41.987481 246632448 process.cpp:2094] Resuming __waiter__(2)@10.0.68.255:50644
at 2015-05-27 22:55:41.987494912+00:00
> I0527 15:55:41.987632 246095872 process.cpp:2094] Resuming __http__(1)@10.0.68.255:50644
at 2015-05-27 22:55:41.987646208+00:00
> I0527 15:55:41.987908 242876416 process.cpp:2094] Resuming __http__(1)@10.0.68.255:50644
at 2015-05-27 22:55:41.987922944+00:00
> I0527 15:55:41.987939 242876416 process.cpp:2201] Cleaning up __http__(1)@10.0.68.255:50644
> I0527 15:55:41.988015 243412992 process.cpp:2094] Resuming __gc__@10.0.68.255:50644 at
2015-05-27 22:55:41.988030976+00:00
> I0527 15:55:41.988276 242876416 process.cpp:2094] Resuming __latch__(2)@10.0.68.255:50644
at 2015-05-27 22:55:41.988286976+00:00
> I0527 15:55:41.988303 242876416 process.cpp:2201] Cleaning up __latch__(2)@10.0.68.255:50644
> I0527 15:55:41.988381 243949568 process.cpp:2094] Resuming __gc__@10.0.68.255:50644 at
2015-05-27 22:55:41.988404992+00:00
> I0527 15:55:41.988381 245022720 process.cpp:2094] Resuming __waiter__(2)@10.0.68.255:50644
at 2015-05-27 22:55:41.988413184+00:00
> I0527 15:55:41.988435 245022720 process.cpp:2201] Cleaning up __waiter__(2)@10.0.68.255:50644
> I0527 15:55:41.988528 245559296 process.cpp:2094] Resuming monitor@10.0.68.255:50644
at 2015-05-27 22:55:41.988540928+00:00
> I0527 15:55:41.988564 245559296 process.cpp:2201] Cleaning up monitor@10.0.68.255:50644
> I0527 15:55:41.988675 245022720 process.cpp:2094] Resuming __limiter__(2)@10.0.68.255:50644
at 2015-05-27 22:55:41.988684032+00:00
> I0527 15:55:41.988703 245022720 process.cpp:2201] Cleaning up __limiter__(2)@10.0.68.255:50644
> [       OK ] MonitorTest.UsageFailure (4 ms)
> [----------] 1 test from MonitorTest (4 ms total)
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>


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