mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benno Evers <ben...@yandex-team.ru>
Subject Re: Review Request 61664: Libprocess: Added a timeout for send socket operation.
Date Thu, 17 Aug 2017 14:05:52 GMT

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




3rdparty/libprocess/src/process.cpp
Lines 254 (patched)
<https://reviews.apache.org/r/61664/#comment259134>

    I think in the case where the client drops the connection, the send will fail anyways
before the timeout with `ECONNRESET` (unless I misunderstand what you mean by dropping a connection)



3rdparty/libprocess/src/process.cpp
Lines 261 (patched)
<https://reviews.apache.org/r/61664/#comment259131>

    If you're hardcoding the value anyways, you could just make the "10" a part of the string.
But probably it's better to make these named constants instead.
    
    Also, I'm not sure that we want to prohibit low values: Presumably someone who wants to
set a non-default value knows what he's doing, so maybe a warning is enough here.



3rdparty/libprocess/src/process.cpp
Lines 2188 (patched)
<https://reviews.apache.org/r/61664/#comment259138>

    I'm not sure if I completely understand how SocketManager works here, but it looks like
this does not reset the underlying TCP connection. So in case of an accidental very long network
delay, does the intended receiver actually notice that the request was discarded and retry?
    
    If not, maybe we can think about setting the `SO_RCVTIMEO` and `SO_SNDTIMEO` socket options
to enforce this timeout at the OS level.


- Benno Evers


On Aug. 15, 2017, 3:45 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61664/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2017, 3:45 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7748
>     https://issues.apache.org/jira/browse/MESOS-7748
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Prior to this patch, a send socket operation can wait forever for
> a send to complete. Clients that drop connections or stop reading
> incoming data, aka "slow reader" attack, can eventually exhaust the
> resources of a libprocess-based application and cause denial of
> service or an OOM event.
> 
> This patch introduces an obligatory timeout for all send socket
> operations, after which the stalled connection is dropped. The
> timeout can be adjusted via the `LIBPROCESS_SOCKET_SEND_TIMEOUT`
> environment variable.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp dcd9c6738816764aae066fe56cd5f468c98fc9bd 
> 
> 
> Diff: https://reviews.apache.org/r/61664/diff/1/
> 
> 
> Testing
> -------
> 
> Manual testing with a rogue client.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


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