2016年3月23日(水) 14:23 James Peach <jpe...@apache.org>:
>
> > On Mar 21, 2016, at 6:57 PM, masa...@apache.org wrote:
> >
> > Repository: trafficserver
> > Updated Branches:
> > refs/heads/master 1e9c9484c -> 0e6e5c151
> >
> >
> > TS-4087: Reduce SETTINGS_MAX_CONCURRENT_STREAMS when too many streams
> >
> > Add below variables in records.config
> > - proxy.config.http2.min_concurrent_streams_in
> > - proxy.config.http2.max_active_streams_in
> >
> > When connection wide active stream are larger than
> proxy.config.http2.max_active_streams_in,
> > SETTINGS_MAX_CONCURRENT_STREAMS is reduced to
> proxy.config.http2.min_concurrent_streams_in
> > in new connections.
> >
> > If the value of proxy.config.http2.max_active_streams_in is 0, there is
> no limit.
> >
> > This closes #485
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> > Commit:
> http://git-wip-us.apache.org/repos/asf/trafficserver/commit/0e6e5c15
> > Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/0e6e5c15
> > Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/0e6e5c15
> >
> > Branch: refs/heads/master
> > Commit: 0e6e5c151cde5f06c15e295f663a98b2b7d37a6d
> > Parents: 1e9c948
> > Author: Masaori Koshiba <masa...@apache.org>
> > Authored: Mon Feb 15 20:57:25 2016 +0900
> > Committer: Masaori Koshiba <masa...@apache.org>
> > Committed: Tue Mar 22 10:52:37 2016 +0900
> >
> > ----------------------------------------------------------------------
> > mgmt/RecordsConfig.cc | 4 ++++
> > proxy/http2/HTTP2.cc | 12 ++++++++---
> > proxy/http2/HTTP2.h | 5 ++++-
> > proxy/http2/Http2ConnectionState.cc | 35 ++++++++++++++++++++++++++++++++
> > proxy/http2/Http2ConnectionState.h | 4 +++-
> > 5 files changed, 55 insertions(+), 5 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0e6e5c15/mgmt/RecordsConfig.cc
> > ----------------------------------------------------------------------
> > diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
> > index 0e963c9..b0e0e6a 100644
> > --- a/mgmt/RecordsConfig.cc
> > +++ b/mgmt/RecordsConfig.cc
> > @@ -1977,6 +1977,10 @@ static const RecordElement RecordsConfig[] =
> > ,
> > {RECT_CONFIG, "proxy.config.http2.max_concurrent_streams_in",
> RECD_INT, "100", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
> > ,
> > + {RECT_CONFIG, "proxy.config.http2.min_concurrent_streams_in",
> RECD_INT, "10", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
> > + ,
> > + {RECT_CONFIG, "proxy.config.http2.max_active_streams_in", RECD_INT,
> "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
> > + ,
>
> Please document these new settings.
>
Sure.
>
> > {RECT_CONFIG, "proxy.config.http2.initial_window_size_in", RECD_INT,
> "1048576", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
> > ,
> > {RECT_CONFIG, "proxy.config.http2.max_frame_size", RECD_INT, "16384",
> RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
> >
> >
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0e6e5c15/proxy/http2/HTTP2.cc
> > ----------------------------------------------------------------------
> > diff --git a/proxy/http2/HTTP2.cc b/proxy/http2/HTTP2.cc
> > index 5cb9d79..c9ea2bc 100644
> > --- a/proxy/http2/HTTP2.cc
> > +++ b/proxy/http2/HTTP2.cc
> > @@ -727,7 +727,10 @@ http2_decode_header_blocks(HTTPHdr *hdr, const
> uint8_t *buf_start, const uint8_t
> > }
> >
> > // Initialize this subsystem with librecords configs (for now)
> > -uint32_t Http2::max_concurrent_streams = 100;
> > +uint32_t Http2::max_concurrent_streams_in = 100;
> > +uint32_t Http2::min_concurrent_streams_in = 10;
> > +uint32_t Http2::max_active_streams_in = 0;
> > +bool Http2::throttling = false;
> > uint32_t Http2::initial_window_size = 1048576;
> > uint32_t Http2::max_frame_size = 16384;
> > uint32_t Http2::header_table_size = 4096;
> > @@ -740,7 +743,9 @@ uint32_t Http2::active_timeout_in = 0;
> > void
> > Http2::init()
> > {
> > - REC_EstablishStaticConfigInt32U(max_concurrent_streams,
> "proxy.config.http2.max_concurrent_streams_in");
> > + REC_EstablishStaticConfigInt32U(max_concurrent_streams_in,
> "proxy.config.http2.max_concurrent_streams_in");
> > + REC_EstablishStaticConfigInt32U(min_concurrent_streams_in,
> "proxy.config.http2.min_concurrent_streams_in");
> > + REC_EstablishStaticConfigInt32U(max_active_streams_in,
> "proxy.config.http2.max_active_streams_in");
> > REC_EstablishStaticConfigInt32U(initial_window_size,
> "proxy.config.http2.initial_window_size_in");
> > REC_EstablishStaticConfigInt32U(max_frame_size,
> "proxy.config.http2.max_frame_size");
> > REC_EstablishStaticConfigInt32U(header_table_size,
> "proxy.config.http2.header_table_size");
> > @@ -751,7 +756,8 @@ Http2::init()
> > REC_EstablishStaticConfigInt32U(active_timeout_in,
> "proxy.config.http2.active_timeout_in");
> >
> > // If any settings is broken, ATS should not start
> > -
> ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS,
> max_concurrent_streams}) &&
> > +
> ink_release_assert(http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS,
> max_concurrent_streams_in}) &&
> > +
> http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS,
> min_concurrent_streams_in}) &&
> >
> http2_settings_parameter_is_valid({HTTP2_SETTINGS_INITIAL_WINDOW_SIZE,
> initial_window_size}) &&
> >
> http2_settings_parameter_is_valid({HTTP2_SETTINGS_MAX_FRAME_SIZE,
> max_frame_size}) &&
> >
> http2_settings_parameter_is_valid({HTTP2_SETTINGS_HEADER_TABLE_SIZE,
> header_table_size}) &&
>
> This should be multiple ink_release_asserts, otherwise it will be
> difficult to figure out which one failed.
>
Yes, it should be. I'll fix.
>
> >
> >
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0e6e5c15/proxy/http2/HTTP2.h
> > ----------------------------------------------------------------------
> > diff --git a/proxy/http2/HTTP2.h b/proxy/http2/HTTP2.h
> > index 10e22e3..9b62b17 100644
> > --- a/proxy/http2/HTTP2.h
> > +++ b/proxy/http2/HTTP2.h
> > @@ -342,7 +342,10 @@ int64_t http2_write_header_fragment(HTTPHdr *,
> MIMEFieldIter &, uint8_t *, uint6
> > class Http2
> > {
> > public:
> > - static uint32_t max_concurrent_streams;
> > + static uint32_t max_concurrent_streams_in;
> > + static uint32_t min_concurrent_streams_in;
> > + static uint32_t max_active_streams_in;
> > + static bool throttling;
> > static uint32_t initial_window_size;
> > static uint32_t max_frame_size;
> > static uint32_t header_table_size;
> >
> >
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0e6e5c15/proxy/http2/Http2ConnectionState.cc
> > ----------------------------------------------------------------------
> > diff --git a/proxy/http2/Http2ConnectionState.cc
> b/proxy/http2/Http2ConnectionState.cc
> > index 077f8ea..5180b3f 100644
> > --- a/proxy/http2/Http2ConnectionState.cc
> > +++ b/proxy/http2/Http2ConnectionState.cc
> > @@ -727,6 +727,8 @@ Http2ConnectionState::main_event_handler(int event,
> void *edata)
> > // settings.
> > Http2ConnectionSettings configured_settings;
> > configured_settings.settings_from_configs();
> > + configured_settings.set(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS,
> _adjust_concurrent_stream());
> > +
> > send_settings_frame(configured_settings);
> >
> > if (server_settings.get(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE) >
> HTTP2_INITIAL_WINDOW_SIZE) {
> > @@ -1167,3 +1169,36 @@
> Http2ConnectionState::send_window_update_frame(Http2StreamId id, uint32_t
> size)
> > SCOPED_MUTEX_LOCK(lock, this->ua_session->mutex, this_ethread());
> > this->ua_session->handleEvent(HTTP2_SESSION_EVENT_XMIT,
> &window_update);
> > }
> > +
> > +// Return min_concurrent_streams_in when current client streams number
> is larger than max_active_streams_in.
> > +// Main purpose of this is preventing DDoS Attacks.
> > +unsigned
> > +Http2ConnectionState::_adjust_concurrent_stream()
> > +{
> > + if (Http2::max_active_streams_in == 0) {
> > + // Throttling down is disabled.
> > + return Http2::max_concurrent_streams_in;
> > + }
> > +
> > + int64_t current_client_streams = 0;
> > + RecGetRawStatSum(http2_rsb, HTTP2_STAT_CURRENT_CLIENT_STREAM_COUNT,
> ¤t_client_streams);
> > +
> > + DebugHttp2Con(ua_session, "current client streams: %" PRId64,
> current_client_streams);
> > +
> > + if (current_client_streams >= Http2::max_active_streams_in) {
> > + if (!Http2::throttling) {
> > + Warning("too many streams: %" PRId64 ", reduce
> SETTINGS_MAX_CONCURRENT_STREAMS to %d", current_client_streams,
> > + Http2::min_concurrent_streams_in);
> > + Http2::throttling = true;
> > + }
> > +
> > + return Http2::min_concurrent_streams_in;
> > + } else {
> > + if (Http2::throttling) {
> > + Note("revert SETTINGS_MAX_CONCURRENT_STREAMS to %d",
> Http2::max_concurrent_streams_in);
>
> I’m not sure that we want to log these. Consider a throttling metric
> instead?
>
throttling metric? Would you give me more details?
ATS logs "too many connections, throttling" when connections are over
proxy.config.net.connections_throttle. I followed this.
>
> > + Http2::throttling = false;
> > + }
> > + }
> > +
> > + return Http2::max_concurrent_streams_in;
> > +}
> >
> >
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/0e6e5c15/proxy/http2/Http2ConnectionState.h
> > ----------------------------------------------------------------------
> > diff --git a/proxy/http2/Http2ConnectionState.h
> b/proxy/http2/Http2ConnectionState.h
> > index 9423c4e..afd0cf2 100644
> > --- a/proxy/http2/Http2ConnectionState.h
> > +++ b/proxy/http2/Http2ConnectionState.h
> > @@ -51,7 +51,7 @@ public:
> > void
> > settings_from_configs()
> > {
> > - settings[indexof(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS)] =
> Http2::max_concurrent_streams;
> > + settings[indexof(HTTP2_SETTINGS_MAX_CONCURRENT_STREAMS)] =
> Http2::max_concurrent_streams_in;
> > settings[indexof(HTTP2_SETTINGS_INITIAL_WINDOW_SIZE)] =
> Http2::initial_window_size;
> > settings[indexof(HTTP2_SETTINGS_MAX_FRAME_SIZE)] =
> Http2::max_frame_size;
> > settings[indexof(HTTP2_SETTINGS_HEADER_TABLE_SIZE)] =
> Http2::header_table_size;
> > @@ -199,6 +199,8 @@ private:
> > Http2ConnectionState(const Http2ConnectionState &); //
> noncopyable
> > Http2ConnectionState &operator=(const Http2ConnectionState &); //
> noncopyable
> >
> > + unsigned _adjust_concurrent_stream();
> > +
> > // NOTE: 'stream_list' has only active streams.
> > // If given Stream Identifier is not found in stream_list and it is
> less
> > // than or equal to latest_streamid, the state of Stream
> >
>
>
