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 52330: Fixed the race between launch and destroy in composing containerizer.
Date Wed, 28 Sep 2016 04:03:05 GMT

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




src/slave/containerizer/composing.cpp (lines 352 - 356)
<https://reviews.apache.org/r/52330/#comment218709>

    Hm.. it doesn't seem correct to return satisfy the destroy future and clean up here. If
launch succeeded and we're in DESTROYING we can just wait for the destroy to complete and
satisfy the future according to the actual destroy result, no?



src/tests/containerizer/composing_containerizer_tests.cpp (line 126)
<https://reviews.apache.org/r/52330/#comment218705>

    Seems like "same as the above test" is prone to become stale if additional tests are added
or the above test is changed?



src/tests/containerizer/composing_containerizer_tests.cpp (lines 127 - 128)
<https://reviews.apache.org/r/52330/#comment218706>

    'this' means the destroy?
    
    The wording here seems a bit inaccurate? It seems like the destroy doesn't stop the loop,
since the first one was able to launch the container.
    
    Maybe something like `DestroyDuringSuccessfulLaunch`?



src/tests/containerizer/composing_containerizer_tests.cpp (lines 184 - 188)
<https://reviews.apache.org/r/52330/#comment218708>

    Hm.. this seems wrong? It seems like if destroy returns false (or fails) then the destroyed
future should surface as false (or fail) as well, since the underlying containerizer was not
able to find (or destroy) the container.


- Benjamin Mahler


On Sept. 28, 2016, 3:15 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52330/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2016, 3:15 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fixed the race between launch and destroy in composing containerizer.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/composing.cpp 17eb34cf9764bd9475dee8cc8c894ede55058dd8 
>   src/tests/containerizer/composing_containerizer_tests.cpp 631328ac54bd6c17abc82f2ea78dc2f5b5750253

> 
> Diff: https://reviews.apache.org/r/52330/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


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