Re: Review Request 47192: Fixed a head-of-line blocking bug in libevent SSL socket.

2016-05-11 Thread Alexander Rukletsov

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


Ship it!




I'd suggest we discard https://reviews.apache.org/r/47207/ in favour of this 
approach.

- Alexander Rukletsov


On May 10, 2016, 10:44 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47192/
> ---
> 
> (Updated May 10, 2016, 10:44 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Joris Van Remoortere, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5340
> https://issues.apache.org/jira/browse/MESOS-5340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, the `accept_queue` is used to store connected sockets.
> However, we push socket futures into this queue *before* they
> complete the SSL handshake or are downgraded. This means that
> a future returned from the queue may remain pending. If the user
> writes a `Socket::accept` loop consuming accepted sockets they
> will experience head-of-line blocking while a slow handshake or
> downgrade is in progress.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> e773fadeb31ca384264a425bdc8d093804e45a82 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> b829e7dd2c0e7730ac1579c042f79f40666b19b9 
> 
> Diff: https://reviews.apache.org/r/47192/diff/
> 
> 
> Testing
> ---
> 
> make check passes
> 
> Manually tested SSL blocking issue:
> 
> Start the master with SSL enabled:
> ```
> $ SSL_CERT_FILE=~/certificates/localhost.crt 
> SSL_KEY_FILE=~/certificates/localhost.key SSL_ENABLED=true 
> ./bin/mesos-master.sh --work_dir=/tmp
> ```
> 
> Inject a pending socket into the Queue:
> ```
> $ telnet localhost 5050
> Trying ::1...
> telnet: connect to address ::1: Connection refused
> Trying 127.0.0.1...
> Connected to localhost.
> Escape character is '^]'.
> ```
> 
> Hit the endpoints:
> ```
> $ curl --insecure https://localhost:5050/health
> ```
> 
> These curl requests hangs without this fix.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Re: Review Request 47192: Fixed a head-of-line blocking bug in libevent SSL socket.

2016-05-10 Thread haosdent huang

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



Verify this works on my machine both `SSL_SUPPORT_DOWNGRADE` enable and 
disable. @tillt have a different proposal for this in 
https://reviews.apache.org/r/47207/ as well.

- haosdent huang


On May 10, 2016, 10:44 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47192/
> ---
> 
> (Updated May 10, 2016, 10:44 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Joris Van Remoortere, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5340
> https://issues.apache.org/jira/browse/MESOS-5340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, the `accept_queue` is used to store connected sockets.
> However, we push socket futures into this queue *before* they
> complete the SSL handshake or are downgraded. This means that
> a future returned from the queue may remain pending. If the user
> writes a `Socket::accept` loop consuming accepted sockets they
> will experience head-of-line blocking while a slow handshake or
> downgrade is in progress.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> e773fadeb31ca384264a425bdc8d093804e45a82 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> b829e7dd2c0e7730ac1579c042f79f40666b19b9 
> 
> Diff: https://reviews.apache.org/r/47192/diff/
> 
> 
> Testing
> ---
> 
> make check passes
> 
> Manually tested SSL blocking issue:
> 
> Start the master with SSL enabled:
> ```
> $ SSL_CERT_FILE=~/certificates/localhost.crt 
> SSL_KEY_FILE=~/certificates/localhost.key SSL_ENABLED=true 
> ./bin/mesos-master.sh --work_dir=/tmp
> ```
> 
> Inject a pending socket into the Queue:
> ```
> $ telnet localhost 5050
> Trying ::1...
> telnet: connect to address ::1: Connection refused
> Trying 127.0.0.1...
> Connected to localhost.
> Escape character is '^]'.
> ```
> 
> Hit the endpoints:
> ```
> $ curl --insecure https://localhost:5050/health
> ```
> 
> These curl requests hangs without this fix.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Re: Review Request 47192: Fixed a head-of-line blocking bug in libevent SSL socket.

2016-05-10 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [47192]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 10, 2016, 10:44 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47192/
> ---
> 
> (Updated May 10, 2016, 10:44 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Joris Van Remoortere, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5340
> https://issues.apache.org/jira/browse/MESOS-5340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, the `accept_queue` is used to store connected sockets.
> However, we push socket futures into this queue *before* they
> complete the SSL handshake or are downgraded. This means that
> a future returned from the queue may remain pending. If the user
> writes a `Socket::accept` loop consuming accepted sockets they
> will experience head-of-line blocking while a slow handshake or
> downgrade is in progress.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> e773fadeb31ca384264a425bdc8d093804e45a82 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> b829e7dd2c0e7730ac1579c042f79f40666b19b9 
> 
> Diff: https://reviews.apache.org/r/47192/diff/
> 
> 
> Testing
> ---
> 
> make check passes
> 
> Manually tested SSL blocking issue:
> 
> Start the master with SSL enabled:
> ```
> $ SSL_CERT_FILE=~/certificates/localhost.crt 
> SSL_KEY_FILE=~/certificates/localhost.key SSL_ENABLED=true 
> ./bin/mesos-master.sh --work_dir=/tmp
> ```
> 
> Inject a pending socket into the Queue:
> ```
> $ telnet localhost 5050
> Trying ::1...
> telnet: connect to address ::1: Connection refused
> Trying 127.0.0.1...
> Connected to localhost.
> Escape character is '^]'.
> ```
> 
> Hit the endpoints:
> ```
> $ curl --insecure https://localhost:5050/health
> ```
> 
> These curl requests hangs without this fix.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>



Review Request 47192: Fixed a head-of-line blocking bug in libevent SSL socket.

2016-05-10 Thread Ben Mahler

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

Review request for mesos, haosdent huang, Joris Van Remoortere, and Till 
Toenshoff.


Bugs: MESOS-5340
https://issues.apache.org/jira/browse/MESOS-5340


Repository: mesos


Description
---

Currently, the `accept_queue` is used to store connected sockets.
However, we push socket futures into this queue *before* they
complete the SSL handshake or are downgraded. This means that
a future returned from the queue may remain pending. If the user
writes a `Socket::accept` loop consuming accepted sockets they
will experience head-of-line blocking while a slow handshake or
downgrade is in progress.


Diffs
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
e773fadeb31ca384264a425bdc8d093804e45a82 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
b829e7dd2c0e7730ac1579c042f79f40666b19b9 

Diff: https://reviews.apache.org/r/47192/diff/


Testing
---

make check passes

Manually tested SSL blocking issue:

Start the master with SSL enabled:
```
$ SSL_CERT_FILE=~/certificates/localhost.crt 
SSL_KEY_FILE=~/certificates/localhost.key SSL_ENABLED=true 
./bin/mesos-master.sh --work_dir=/tmp
```

Inject a pending socket into the Queue:
```
$ telnet localhost 5050
Trying ::1...
telnet: connect to address ::1: Connection refused
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
```

Hit the endpoints:
```
$ curl --insecure https://localhost:5050/health
```

These curl requests hangs without this fix.


Thanks,

Ben Mahler