mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 68354: Added a test to verify agent authentication retry backoff logic.
Date Wed, 15 Aug 2018 22:45:18 GMT

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



Probably also want to run this test in repetition to help make sure it's not flaky?


src/tests/authentication_tests.cpp
Lines 411-425 (patched)
<https://reviews.apache.org/r/68354/#comment290736>

    Can we push down the start into the loop (only for the 1st iteration) and avoid the 1st
special casing here? It also seems a little odd to be doing manual calling into the function,
can we also use the default mock behavior of calling through to avoid the manual calling?



src/tests/authentication_tests.cpp
Lines 412-413 (patched)
<https://reviews.apache.org/r/68354/#comment290733>

    Why are the underscores needed? It's confusing since `_authenticate` is also the continuation
of `authenticate`



src/tests/authentication_tests.cpp
Lines 419 (patched)
<https://reviews.apache.org/r/68354/#comment290735>

    Do you actually still need this retirement on all of these?



src/tests/authentication_tests.cpp
Lines 427 (patched)
<https://reviews.apache.org/r/68354/#comment290734>

    Why 10 here? Does it take that many to reach the max? Should probably explain to the reader
the math on why 10?
    
    Maybe it's easier if this is just a while loop until we hit max or see two maxes?



src/tests/authentication_tests.cpp
Lines 443-444 (patched)
<https://reviews.apache.org/r/68354/#comment290737>

    Maybe use milliseconds here? Not sure if this level of timer precision is supported on
windows, etc.



src/tests/authentication_tests.cpp
Lines 462-463 (patched)
<https://reviews.apache.org/r/68354/#comment290741>

    Hm.. can't we test that max follows `min+b*2^n`?



src/tests/authentication_tests.cpp
Lines 468-471 (patched)
<https://reviews.apache.org/r/68354/#comment290742>

    I guess there's no delay between authentication success and registration?
    
    Doing the advance before the FUTURE_PROTOBUF is racy, it might miss the message and cause
a flaky test.


- Benjamin Mahler


On Aug. 15, 2018, 6:50 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68354/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2018, 6:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This test verifies that the agent backs-off properly when
> retrying authentication according to the configured parameters.
> 
> Also mocked `Slave::authenticate()` for this test.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.hpp 802d4eb9e9eba2f1175dd85e56dcc80a61e32f74 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
>   src/tests/mock_slave.hpp 9a74bf35d2cab0a72ba6376392239d8080a49304 
>   src/tests/mock_slave.cpp 94a5b0d20475f49dde99108a009682b520175aa4 
> 
> 
> Diff: https://reviews.apache.org/r/68354/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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