You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2018/08/29 02:03:44 UTC

Review Request 68553: Set 'Connection: close' in the master's streaming API responses.

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


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


Repository: mesos


Description
-------

We've seen some HTTP intermediaries (e.g. ELB) decide to re-use
connections to mesos as an optimization to avoid re-connection
overhead. As a result, when the end-client of the streaming API
disconnects from the intermediary, the intermediary leaves the
connection to Mesos open in an attempt to re-use the connection
for another request once the response completes. Mesos then thinks
that the subscriber never disconnected and the intermediary happily
continues to read the streaming events even though there's no
end-client.

To help indicate to intermediaries that the connection SHOULD NOT
be re-used, we can set the 'Connection: close' header for streaming
API responses. It may not be respected (since the language seems to
be SHOULD NOT), but some intermediaries may respect it and close the
connection if the end-client disconnects.

Note that libprocess' http server currently doesn't close the the
connection based on a handler setting this header, but it doesn't
matter here since the master's operator / scheduler and agent's
executor streaming API responses are infinite.


Diffs
-----

  src/master/http.cpp e074a93666d82944774e7b0c3fa32d7021d93c97 


Diff: https://reviews.apache.org/r/68553/diff/1/


Testing
-------

Manually verified headers make it through:

```
$ telnet 10.0.49.2 5050
Trying 10.0.49.2...
Connected to 10.0.49.2.
Escape character is '^]'.
POST /master/api/v1 HTTP/1.1
Content-Type: application/json
Accept: application/json
Content-Length: 20

{"type":"SUBSCRIBE"}
HTTP/1.1 200 OK
Transfer-Encoding: chunked
Content-Type: application/json
Date: Wed, 29 Aug 2018 01:58:34 GMT
Connection: close

9e
154
{"type":"SUBSCRIBED","subscribed":{"get_state":{"get_tasks":{},"get_executors":{},"get_frameworks":{},"get_agents":{}},"heartbeat_interval_seconds":15.0}}
17
20
{"type":"HEARTBEAT"}
```


Thanks,

Benjamin Mahler


Re: Review Request 68553: Set 'Connection: close' in the master's streaming API responses.

Posted by Benjamin Mahler <bm...@apache.org>.

> On Sept. 5, 2018, 3:11 a.m., Gastón Kleiman wrote:
> > We should do the same in the executor API handler.

Yeah, originally I did that in this patch, but it ended up being more complicated than I anticipated. In any case, I commented in MESOS-9189 that I would leave the ticket open (although, probably I need to pull out another ticket for the sake of the CHANGELOG and backporting).


- Benjamin


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


On Aug. 29, 2018, 2:03 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68553/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2018, 2:03 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-9189
>     https://issues.apache.org/jira/browse/MESOS-9189
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> We've seen some HTTP intermediaries (e.g. ELB) decide to re-use
> connections to mesos as an optimization to avoid re-connection
> overhead. As a result, when the end-client of the streaming API
> disconnects from the intermediary, the intermediary leaves the
> connection to Mesos open in an attempt to re-use the connection
> for another request once the response completes. Mesos then thinks
> that the subscriber never disconnected and the intermediary happily
> continues to read the streaming events even though there's no
> end-client.
> 
> To help indicate to intermediaries that the connection SHOULD NOT
> be re-used, we can set the 'Connection: close' header for streaming
> API responses. It may not be respected (since the language seems to
> be SHOULD NOT), but some intermediaries may respect it and close the
> connection if the end-client disconnects.
> 
> Note that libprocess' http server currently doesn't close the the
> connection based on a handler setting this header, but it doesn't
> matter here since the master's operator / scheduler and agent's
> executor streaming API responses are infinite.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp e074a93666d82944774e7b0c3fa32d7021d93c97 
> 
> 
> Diff: https://reviews.apache.org/r/68553/diff/1/
> 
> 
> Testing
> -------
> 
> Manually verified headers make it through:
> 
> ```
> $ telnet 10.0.49.2 5050
> Trying 10.0.49.2...
> Connected to 10.0.49.2.
> Escape character is '^]'.
> POST /master/api/v1 HTTP/1.1
> Content-Type: application/json
> Accept: application/json
> Content-Length: 20
> 
> {"type":"SUBSCRIBE"}
> HTTP/1.1 200 OK
> Transfer-Encoding: chunked
> Content-Type: application/json
> Date: Wed, 29 Aug 2018 01:58:34 GMT
> Connection: close
> 
> 9e
> 154
> {"type":"SUBSCRIBED","subscribed":{"get_state":{"get_tasks":{},"get_executors":{},"get_frameworks":{},"get_agents":{}},"heartbeat_interval_seconds":15.0}}
> 17
> 20
> {"type":"HEARTBEAT"}
> ```
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 68553: Set 'Connection: close' in the master's streaming API responses.

Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68553/#review208340
-----------------------------------------------------------


Ship it!




We should do the same in the executor API handler.

- Gastón Kleiman


On Aug. 28, 2018, 7:03 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68553/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2018, 7:03 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-9189
>     https://issues.apache.org/jira/browse/MESOS-9189
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> We've seen some HTTP intermediaries (e.g. ELB) decide to re-use
> connections to mesos as an optimization to avoid re-connection
> overhead. As a result, when the end-client of the streaming API
> disconnects from the intermediary, the intermediary leaves the
> connection to Mesos open in an attempt to re-use the connection
> for another request once the response completes. Mesos then thinks
> that the subscriber never disconnected and the intermediary happily
> continues to read the streaming events even though there's no
> end-client.
> 
> To help indicate to intermediaries that the connection SHOULD NOT
> be re-used, we can set the 'Connection: close' header for streaming
> API responses. It may not be respected (since the language seems to
> be SHOULD NOT), but some intermediaries may respect it and close the
> connection if the end-client disconnects.
> 
> Note that libprocess' http server currently doesn't close the the
> connection based on a handler setting this header, but it doesn't
> matter here since the master's operator / scheduler and agent's
> executor streaming API responses are infinite.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp e074a93666d82944774e7b0c3fa32d7021d93c97 
> 
> 
> Diff: https://reviews.apache.org/r/68553/diff/1/
> 
> 
> Testing
> -------
> 
> Manually verified headers make it through:
> 
> ```
> $ telnet 10.0.49.2 5050
> Trying 10.0.49.2...
> Connected to 10.0.49.2.
> Escape character is '^]'.
> POST /master/api/v1 HTTP/1.1
> Content-Type: application/json
> Accept: application/json
> Content-Length: 20
> 
> {"type":"SUBSCRIBE"}
> HTTP/1.1 200 OK
> Transfer-Encoding: chunked
> Content-Type: application/json
> Date: Wed, 29 Aug 2018 01:58:34 GMT
> Connection: close
> 
> 9e
> 154
> {"type":"SUBSCRIBED","subscribed":{"get_state":{"get_tasks":{},"get_executors":{},"get_frameworks":{},"get_agents":{}},"heartbeat_interval_seconds":15.0}}
> 17
> 20
> {"type":"HEARTBEAT"}
> ```
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 68553: Set 'Connection: close' in the master's streaming API responses.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68553/#review208180
-----------------------------------------------------------



Patch looks great!

Reviews applied: [68553]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Aug. 28, 2018, 9:03 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68553/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2018, 9:03 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-9189
>     https://issues.apache.org/jira/browse/MESOS-9189
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> We've seen some HTTP intermediaries (e.g. ELB) decide to re-use
> connections to mesos as an optimization to avoid re-connection
> overhead. As a result, when the end-client of the streaming API
> disconnects from the intermediary, the intermediary leaves the
> connection to Mesos open in an attempt to re-use the connection
> for another request once the response completes. Mesos then thinks
> that the subscriber never disconnected and the intermediary happily
> continues to read the streaming events even though there's no
> end-client.
> 
> To help indicate to intermediaries that the connection SHOULD NOT
> be re-used, we can set the 'Connection: close' header for streaming
> API responses. It may not be respected (since the language seems to
> be SHOULD NOT), but some intermediaries may respect it and close the
> connection if the end-client disconnects.
> 
> Note that libprocess' http server currently doesn't close the the
> connection based on a handler setting this header, but it doesn't
> matter here since the master's operator / scheduler and agent's
> executor streaming API responses are infinite.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp e074a93666d82944774e7b0c3fa32d7021d93c97 
> 
> 
> Diff: https://reviews.apache.org/r/68553/diff/1/
> 
> 
> Testing
> -------
> 
> Manually verified headers make it through:
> 
> ```
> $ telnet 10.0.49.2 5050
> Trying 10.0.49.2...
> Connected to 10.0.49.2.
> Escape character is '^]'.
> POST /master/api/v1 HTTP/1.1
> Content-Type: application/json
> Accept: application/json
> Content-Length: 20
> 
> {"type":"SUBSCRIBE"}
> HTTP/1.1 200 OK
> Transfer-Encoding: chunked
> Content-Type: application/json
> Date: Wed, 29 Aug 2018 01:58:34 GMT
> Connection: close
> 
> 9e
> 154
> {"type":"SUBSCRIBED","subscribed":{"get_state":{"get_tasks":{},"get_executors":{},"get_frameworks":{},"get_agents":{}},"heartbeat_interval_seconds":15.0}}
> 17
> 20
> {"type":"HEARTBEAT"}
> ```
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 68553: Set 'Connection: close' in the master's streaming API responses.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68553/#review208076
-----------------------------------------------------------



PASS: Mesos patch 68553 was successfully built and tested.

Reviews applied: `['68553']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2256/mesos-review-68553

- Mesos Reviewbot Windows


On Aug. 29, 2018, 2:03 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68553/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2018, 2:03 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-9189
>     https://issues.apache.org/jira/browse/MESOS-9189
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> We've seen some HTTP intermediaries (e.g. ELB) decide to re-use
> connections to mesos as an optimization to avoid re-connection
> overhead. As a result, when the end-client of the streaming API
> disconnects from the intermediary, the intermediary leaves the
> connection to Mesos open in an attempt to re-use the connection
> for another request once the response completes. Mesos then thinks
> that the subscriber never disconnected and the intermediary happily
> continues to read the streaming events even though there's no
> end-client.
> 
> To help indicate to intermediaries that the connection SHOULD NOT
> be re-used, we can set the 'Connection: close' header for streaming
> API responses. It may not be respected (since the language seems to
> be SHOULD NOT), but some intermediaries may respect it and close the
> connection if the end-client disconnects.
> 
> Note that libprocess' http server currently doesn't close the the
> connection based on a handler setting this header, but it doesn't
> matter here since the master's operator / scheduler and agent's
> executor streaming API responses are infinite.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp e074a93666d82944774e7b0c3fa32d7021d93c97 
> 
> 
> Diff: https://reviews.apache.org/r/68553/diff/1/
> 
> 
> Testing
> -------
> 
> Manually verified headers make it through:
> 
> ```
> $ telnet 10.0.49.2 5050
> Trying 10.0.49.2...
> Connected to 10.0.49.2.
> Escape character is '^]'.
> POST /master/api/v1 HTTP/1.1
> Content-Type: application/json
> Accept: application/json
> Content-Length: 20
> 
> {"type":"SUBSCRIBE"}
> HTTP/1.1 200 OK
> Transfer-Encoding: chunked
> Content-Type: application/json
> Date: Wed, 29 Aug 2018 01:58:34 GMT
> Connection: close
> 
> 9e
> 154
> {"type":"SUBSCRIBED","subscribed":{"get_state":{"get_tasks":{},"get_executors":{},"get_frameworks":{},"get_agents":{}},"heartbeat_interval_seconds":15.0}}
> 17
> 20
> {"type":"HEARTBEAT"}
> ```
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>