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 40266: Libprocess Reinit: Cleanup SocketManager alongside ProcessManager.
Date Thu, 04 Aug 2016 23:48:38 GMT

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


Fix it, then Ship it!





3rdparty/libprocess/src/process.cpp (line 1113)
<https://reviews.apache.org/r/40266/#comment210981>

    Remove `gc` from this list, since it now gets deleted in the destructor



3rdparty/libprocess/src/process.cpp (lines 1116 - 1118)
<https://reviews.apache.org/r/40266/#comment210982>

    It's not clear to me precisely which logic this comment refers to - HTTP authentication
logic occurs in `ProcessBase` rather than `ProcessManager`, so perhaps it's a typo. AFAIK
all HTTP authentication happens within libprocess processes, so terminating all processes
should be sufficient to complete any pending authentication requests and exit safely, but
perhaps it's worth checking with Alexander to see exactly what he meant here?



3rdparty/libprocess/src/process.cpp (line 2433)
<https://reviews.apache.org/r/40266/#comment210980>

    s/ProcessManager/`ProcessManager`/


- Greg Mann


On July 29, 2016, 11:53 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40266/
> -----------------------------------------------------------
> 
> (Updated July 29, 2016, 11:53 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, and Vinod
Kone.
> 
> 
> Bugs: MESOS-3910
>     https://issues.apache.org/jira/browse/MESOS-3910
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `SocketManager` and `ProcessManager` are highly inter-dependent, 
> which requires some untangling in `process::finalize`.
> 
> * Logic originally found in `~ProcessManager` has been split into 
>   `ProcessManager::finalize` due to what happens during cleanup.
> * The future from `__s__->accept()` must be explicitly discarded as 
>   libevent does not detect a locally closed socket.
> * Terminating `HttpProxy`s must close the associated socket.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp 7f331b812de2f0437838f48e0959441c8e04c358 
> 
> Diff: https://reviews.apache.org/r/40266/diff/
> 
> 
> Testing
> -------
> 
> `make check` (libev)
> `make check` (--enable-libevent --enable-ssl)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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