mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 69307: Changed master to hold subscribers in a circular buffer.
Date Wed, 28 Nov 2018 00:38:14 GMT


> On Nov. 27, 2018, 3:33 p.m., Benno Evers wrote:
> > src/tests/api_tests.cpp
> > Lines 3688 (patched)
> > <https://reviews.apache.org/r/69307/diff/1/?file=2106790#file2106790line3688>
> >
> >     To be honest, I don't completely understand if this is creating some special
"unclean shutdown" condition for the third connection, or if it is just the simplest way to
force removal of the connection on the master side.
> >     
> >     In the former case, this might be better suited for a separate unit test.
> >     
> >     In the latter case, it seems like this some  missing API in Mesos (i.e. an `UNSUBSCRIBE`
call), since intuitively this is something that *should* be easy to do from the clients perspective.

This little hack is meant to close the client's connection, which triggers the some code in
the master waiting for disconnections.  I resorted to this because our HTTP helpers (i.e.
`process::http::streaming::post`) returns a `http::Response` object, but this Response object
does not control the lifetime of the connection (presumably because the same object is returned
for single-requests, streaming responses, and pipelined requests).

I suppose another way to go about this is explicitly create the `Connection` object and then
send the request over that connection, effectively unwrapping `process::http::streaming::post`
into the test body.

Also, the `Connection` object does not do anything when a streaming response's reader is closed.
 We could probably change the library/helper's behavior here.  Right now, the `Connection`
object is kept alive like this:
```
      // This is a non Keep-Alive request which means the connection
      // will be closed when the response is received. Since the
      // 'Connection' is reference-counted, we must maintain a copy
      // until the disconnection occurs.
      connection.disconnected()
        .onAny([connection]() {});
```


- Joseph


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


On Nov. 26, 2018, 3:40 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69307/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2018, 3:40 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-9258
>     https://issues.apache.org/jira/browse/MESOS-9258
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This adds a flag (--max_event_stream_subscribers) to the master which
> controls how many active subscribers on the Master's event stream will
> be allowed at any time.  The default is 25 subscribers.
> 
> 
> Diffs
> -----
> 
>   docs/configuration/master.md 575476728079d884fe86b1a3a56614647d1b572e 
>   src/master/constants.hpp 76ad0c3b1494cb97888af5545acc998b8fb15bf6 
>   src/master/flags.hpp ed2d76af2831d1b9718724936fd3e4850e99b332 
>   src/master/flags.cpp 2677738d19caa7d600e591aeb4fb37f0c2318d45 
>   src/master/master.hpp e77babf22126838c63cd05e483875c9beb3ac5ff 
>   src/master/master.cpp 1e326ec42a7f79a0835529a4655e7ec272f1cf40 
>   src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133 
> 
> 
> Diff: https://reviews.apache.org/r/69307/diff/1/
> 
> 
> Testing
> -------
> 
> ```
> make check
> 
> src/mesos-tests --gtest_filter="*MaxEventStreamSubscribers*" --gtest_repeat=-1 --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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