Skip to content

Make proxy.config.http.per_server.min_keep_alive_connections overridable #5830

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sudheerv merged 1 commit into from
Aug 19, 2019
Merged

Make proxy.config.http.per_server.min_keep_alive_connections overridable #5830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion doc/admin-guide/files/records.config.en.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1507,8 +1507,9 @@ Origin Server Connect Attempts
Throttle alerts per upstream server group to be no more often than this many seconds. Summary
data is provided per alert to allow log scrubbing to generate accurate data.

.. ts:cv:: CONFIG proxy.config.http.per_server.min_keep_alive_connections INT 0
.. ts:cv:: CONFIG proxy.config.http.per_server.connection.min INT 0
:reloadable:
:overridable:

Set a target for the minimum number of active connections to an upstream server group. When an
outbound connection is in keep alive state and the inactivity timer expires, if there are fewer
Expand Down
1 change: 1 addition & 0 deletions include/ts/apidefs.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,7 @@ typedef enum {
TS_CONFIG_HTTP_ALLOW_MULTI_RANGE,
TS_CONFIG_HTTP_REQUEST_BUFFER_ENABLED,
TS_CONFIG_HTTP_ALLOW_HALF_OPEN,
TS_CONFIG_HTTP_SERVER_MIN_KEEP_ALIVE_CONNS,
TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MAX,
TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MATCH,
TS_CONFIG_SSL_CLIENT_VERIFY_SERVER,
Expand Down
2 changes: 1 addition & 1 deletion mgmt/RecordsConfig.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ static const RecordElement RecordsConfig[] =
,
{RECT_CONFIG, "proxy.config.http.per_server.connection.queue_delay", RECD_INT, "100", RECU_DYNAMIC, RR_NULL, RECC_STR, "^-?[0-9]+$", RECA_NULL}
,
{RECT_CONFIG, "proxy.config.http.per_server.min_keep_alive", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
{RECT_CONFIG, "proxy.config.http.per_server.connection.min", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
,
{RECT_CONFIG, "proxy.config.http.attach_server_session_to_client", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
,
Expand Down
2 changes: 2 additions & 0 deletions plugins/lua/ts_lua_http_config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ typedef enum {
TS_LUA_CONFIG_HTTP_TRANSACTION_NO_ACTIVITY_TIMEOUT_IN = TS_CONFIG_HTTP_TRANSACTION_NO_ACTIVITY_TIMEOUT_IN,
TS_LUA_CONFIG_HTTP_TRANSACTION_NO_ACTIVITY_TIMEOUT_OUT = TS_CONFIG_HTTP_TRANSACTION_NO_ACTIVITY_TIMEOUT_OUT,
TS_LUA_CONFIG_HTTP_TRANSACTION_ACTIVE_TIMEOUT_OUT = TS_CONFIG_HTTP_TRANSACTION_ACTIVE_TIMEOUT_OUT,
TS_LUA_CONFIG_HTTP_SERVER_MIN_KEEP_ALIVE_CONNS = TS_CONFIG_HTTP_SERVER_MIN_KEEP_ALIVE_CONNS,
TS_LUA_CONFIG_HTTP_PER_SERVER_CONNECTION_MAX = TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MAX,
TS_LUA_CONFIG_HTTP_PER_SERVER_CONNECTION_MATCH = TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MATCH,
TS_LUA_CONFIG_HTTP_CONNECT_ATTEMPTS_MAX_RETRIES = TS_CONFIG_HTTP_CONNECT_ATTEMPTS_MAX_RETRIES,
Expand Down Expand Up @@ -260,6 +261,7 @@ ts_lua_var_item ts_lua_http_config_vars[] = {
TS_LUA_MAKE_VAR_ITEM(TS_CONFIG_SSL_CLIENT_SNI_POLICY),
TS_LUA_MAKE_VAR_ITEM(TS_CONFIG_SSL_CLIENT_PRIVATE_KEY_FILENAME),
TS_LUA_MAKE_VAR_ITEM(TS_CONFIG_SSL_CLIENT_CA_CERT_FILENAME),
TS_LUA_MAKE_VAR_ITEM(TS_CONFIG_HTTP_SERVER_MIN_KEEP_ALIVE_CONNS),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_PER_SERVER_CONNECTION_MAX),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_HTTP_PER_SERVER_CONNECTION_MATCH),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_LAST_ENTRY),
Expand Down
17 changes: 8 additions & 9 deletions proxy/http/HttpConfig.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,6 @@ HttpConfig::startup()

HttpEstablishStaticConfigLongLong(c.server_max_connections, "proxy.config.http.server_max_connections");
HttpEstablishStaticConfigLongLong(c.max_websocket_connections, "proxy.config.http.websocket.max_number_of_connections");
HttpEstablishStaticConfigLongLong(c.origin_min_keep_alive_connections, "proxy.config.http.per_server.min_keep_alive");
HttpEstablishStaticConfigByte(c.oride.attach_server_session_to_client, "proxy.config.http.attach_server_session_to_client");

HttpEstablishStaticConfigLongLong(c.http_request_line_max_size, "proxy.config.http.request_line_max_size");
Expand Down Expand Up @@ -1268,22 +1267,21 @@ HttpConfig::reconfigure()
params->oride.outbound_conntrack = m_master.oride.outbound_conntrack;
// If queuing for outbound connection tracking is enabled without enabling max connections, it is meaningless, so we'll warn
if (params->outbound_conntrack.queue_size > 0 &&
!(params->oride.outbound_conntrack.max > 0 || params->origin_min_keep_alive_connections)) {
Warning("'%s' is set, but neither '%s' nor 'per_server.min_keep_alive_connections' are "
!(params->oride.outbound_conntrack.max > 0 || params->oride.outbound_conntrack.min > 0)) {
Warning("'%s' is set, but neither '%s' nor '%s' are "
"set, please correct your records.config",
OutboundConnTrack::CONFIG_VAR_QUEUE_SIZE.data(), OutboundConnTrack::CONFIG_VAR_MAX.data());
OutboundConnTrack::CONFIG_VAR_QUEUE_SIZE.data(), OutboundConnTrack::CONFIG_VAR_MAX.data(),
OutboundConnTrack::CONFIG_VAR_MIN.data());
}
params->origin_min_keep_alive_connections = m_master.origin_min_keep_alive_connections;
params->oride.attach_server_session_to_client = m_master.oride.attach_server_session_to_client;

params->http_request_line_max_size = m_master.http_request_line_max_size;
params->http_hdr_field_max_size = m_master.http_hdr_field_max_size;

if (params->oride.outbound_conntrack.max > 0 &&
params->oride.outbound_conntrack.max < params->origin_min_keep_alive_connections) {
Warning("'%s' < origin_min_keep_alive_connections, setting min=max , please correct your records.config",
if (params->oride.outbound_conntrack.max > 0 && params->oride.outbound_conntrack.max < params->oride.outbound_conntrack.min) {
Warning("'%s' < per_server.min_keep_alive_connections, setting min=max , please correct your records.config",
OutboundConnTrack::CONFIG_VAR_MAX.data());
params->origin_min_keep_alive_connections = params->oride.outbound_conntrack.max;
params->oride.outbound_conntrack.min = params->oride.outbound_conntrack.max;
}

params->oride.insert_request_via_string = m_master.oride.insert_request_via_string;
Expand Down Expand Up @@ -1324,6 +1322,7 @@ HttpConfig::reconfigure()
}

params->oride.server_session_sharing_match = m_master.oride.server_session_sharing_match;
params->oride.server_min_keep_alive_conns = m_master.oride.server_min_keep_alive_conns;
params->server_session_sharing_pool = m_master.server_session_sharing_pool;
params->oride.keep_alive_post_out = m_master.oride.keep_alive_post_out;

Expand Down
6 changes: 3 additions & 3 deletions proxy/http/HttpConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ struct OverridableHttpConfigParams {
MgmtByte keep_alive_enabled_out = 1;
MgmtByte keep_alive_post_out = 1; // share server sessions for post

MgmtInt server_min_keep_alive_conns = 0;
MgmtByte server_session_sharing_match = TS_SERVER_SESSION_SHARING_MATCH_BOTH;
// MgmtByte share_server_sessions;
MgmtByte auth_server_session_private = 1;
Expand Down Expand Up @@ -705,9 +706,8 @@ struct HttpConfigParams : public ConfigInfo {
IpAddr proxy_protocol_ip4, proxy_protocol_ip6;
IpMap config_proxy_protocol_ipmap;

MgmtInt server_max_connections = 0;
MgmtInt origin_min_keep_alive_connections = 0; // TODO: This one really ought to be overridable, but difficult right now.
MgmtInt max_websocket_connections = -1;
MgmtInt server_max_connections = 0;
MgmtInt max_websocket_connections = -1;

char *proxy_request_via_string = nullptr;
char *proxy_response_via_string = nullptr;
Expand Down
19 changes: 18 additions & 1 deletion proxy/http/HttpConnectionCount.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ const MgmtConverter OutboundConnTrack::MAX_CONV(
[](void *data) -> MgmtInt { return static_cast<MgmtInt>(*static_cast<decltype(TxnConfig::max) *>(data)); },
[](void *data, MgmtInt i) -> void { *static_cast<decltype(TxnConfig::max) *>(data) = static_cast<decltype(TxnConfig::max)>(i); });

const MgmtConverter OutboundConnTrack::MIN_CONV(
[](void *data) -> MgmtInt { return static_cast<MgmtInt>(*static_cast<decltype(TxnConfig::min) *>(data)); },
[](void *data, MgmtInt i) -> void { *static_cast<decltype(TxnConfig::min) *>(data) = static_cast<decltype(TxnConfig::min)>(i); });

// Do integer and string conversions.
const MgmtConverter OutboundConnTrack::MATCH_CONV{
[](void *data) -> MgmtInt { return static_cast<MgmtInt>(*static_cast<decltype(TxnConfig::match) *>(data)); },
Expand Down Expand Up @@ -72,6 +76,17 @@ static_assert(OutboundConnTrack::Group::Clock::period::den >= 1000);
// Configuration callback functions.
namespace
{
int
Config_Update_Conntrack_Min(const char *name, RecDataT dtype, RecData data, void *cookie)
{
auto config = static_cast<OutboundConnTrack::TxnConfig *>(cookie);

if (RECD_INT == dtype) {
config->min = data.rec_int;
}
return REC_ERR_OKAY;
}

int
Config_Update_Conntrack_Max(const char *name, RecDataT dtype, RecData data, void *cookie)
{
Expand Down Expand Up @@ -154,13 +169,15 @@ OutboundConnTrack::config_init(GlobalConfig *global, TxnConfig *txn)
_global_config = global; // remember this for later retrieval.
// Per transaction lookup must be done at call time because it changes.

RecRegisterConfigUpdateCb(CONFIG_VAR_MIN.data(), &Config_Update_Conntrack_Min, txn);
RecRegisterConfigUpdateCb(CONFIG_VAR_MAX.data(), &Config_Update_Conntrack_Max, txn);
RecRegisterConfigUpdateCb(CONFIG_VAR_MATCH.data(), &Config_Update_Conntrack_Match, txn);
RecRegisterConfigUpdateCb(CONFIG_VAR_QUEUE_SIZE.data(), &Config_Update_Conntrack_Queue_Size, global);
RecRegisterConfigUpdateCb(CONFIG_VAR_QUEUE_DELAY.data(), &Config_Update_Conntrack_Queue_Delay, global);
RecRegisterConfigUpdateCb(CONFIG_VAR_ALERT_DELAY.data(), &Config_Update_Conntrack_Alert_Delay, global);

// Load 'em up by firing off the config update callback.
RecLookupRecord(CONFIG_VAR_MIN.data(), &Load_Config_Var, nullptr, true);
RecLookupRecord(CONFIG_VAR_MAX.data(), &Load_Config_Var, nullptr, true);
RecLookupRecord(CONFIG_VAR_MATCH.data(), &Load_Config_Var, nullptr, true);
RecLookupRecord(CONFIG_VAR_QUEUE_SIZE.data(), &Load_Config_Var, nullptr, true);
Expand All @@ -180,7 +197,7 @@ OutboundConnTrack::obtain(TxnConfig const &txn_cnf, std::string_view fqdn, IpEnd
if (loc != _imp._table.end()) {
zret._g = loc;
} else {
zret._g = new Group(key, fqdn);
zret._g = new Group(key, fqdn, txn_cnf.min);
Copy link
Member

@SolidWallOfCode SolidWallOfCode Aug 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does min need to be set here, but not max? Because max is not overridable?

Copy link
Contributor Author

@sudheerv sudheerv Aug 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really - max is just as overridable as min would be after this PR. The reason for not updating max into Group is that, the configured max value is only needed when making a new Origin connection (to check if the current count is past the max), and we always have a valid TXN when making a new origin connection.

However, that's not true for min - min value is needed when closing an existing idle (keep alive) connection, which generally happens because of a VC level event (an error, or a keep alive timeout etc) and it isn't a guaranteed to have a valid TXN object (and therefore it's config) to compare against :(

If it makes sense to keep it consistent and remove confusion, I'm okay to add max also to the Group, but, it'd be redundant.

_imp._table.insert(zret._g);
}
return zret;
Expand Down
20 changes: 12 additions & 8 deletions proxy/http/HttpConnectionCount.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class OutboundConnTrack
/// Per transaction configuration values.
struct TxnConfig {
int max{0}; ///< Maximum concurrent connections.
int min{0}; ///< Minimum keepalive connections.
MatchType match{MATCH_IP}; ///< Match type.
};

Expand All @@ -85,6 +86,7 @@ class OutboundConnTrack
// Unfortunately these are not used in RecordsConfig.cc so that must be made consistent by hand.
// Note: These need to be @c constexpr or there are static initialization ordering risks.
static constexpr std::string_view CONFIG_VAR_MAX{"proxy.config.http.per_server.connection.max"_sv};
static constexpr std::string_view CONFIG_VAR_MIN{"proxy.config.http.per_server.connection.min"_sv};
static constexpr std::string_view CONFIG_VAR_MATCH{"proxy.config.http.per_server.connection.match"_sv};
static constexpr std::string_view CONFIG_VAR_QUEUE_SIZE{"proxy.config.http.per_server.connection.queue_size"_sv};
static constexpr std::string_view CONFIG_VAR_QUEUE_DELAY{"proxy.config.http.per_server.connection.queue_delay"_sv};
Expand All @@ -109,11 +111,12 @@ class OutboundConnTrack
MatchType const &_match_type; ///< Type of matching.
};

IpEndpoint _addr; ///< Remote IP address.
CryptoHash _hash; ///< Hash of the FQDN.
MatchType _match_type; ///< Type of matching.
std::string _fqdn; ///< Expanded FQDN, set if matching on FQDN.
Key _key; ///< Pre-assembled key which references the following members.
IpEndpoint _addr; ///< Remote IP address.
CryptoHash _hash; ///< Hash of the FQDN.
MatchType _match_type; ///< Type of matching.
std::string _fqdn; ///< Expanded FQDN, set if matching on FQDN.
int min_keep_alive_conns; /// < Min keep alive conns on this server group
Key _key; ///< Pre-assembled key which references the following members.

// Counting data.
std::atomic<int> _count{0}; ///< Number of outbound connections.
Expand All @@ -132,7 +135,7 @@ class OutboundConnTrack
* @param key A populated @c Key structure - values are copied to the @c Group.
* @param fqdn The full FQDN.
*/
Group(Key const &key, std::string_view fqdn);
Group(Key const &key, std::string_view fqdn, int min_keep_alive);
/// Key equality checker.
static bool equal(Key const &lhs, Key const &rhs);
/// Hashing function.
Expand Down Expand Up @@ -247,6 +250,7 @@ class OutboundConnTrack
static void Warning_Bad_Match_Type(std::string_view tag);

// Converters for overridable values for use in the TS API.
static const MgmtConverter MIN_CONV;
static const MgmtConverter MAX_CONV;
static const MgmtConverter MATCH_CONV;

Expand Down Expand Up @@ -286,8 +290,8 @@ OutboundConnTrack::instance()
return _imp;
}

inline OutboundConnTrack::Group::Group(Key const &key, std::string_view fqdn)
: _hash(key._hash), _match_type(key._match_type), _key{_addr, _hash, _match_type}
inline OutboundConnTrack::Group::Group(Key const &key, std::string_view fqdn, int min_keep_alive)
: _hash(key._hash), _match_type(key._match_type), min_keep_alive_conns(min_keep_alive), _key{_addr, _hash, _match_type}
{
// store the host name if relevant.
if (MATCH_HOST == _match_type || MATCH_BOTH == _match_type) {
Expand Down
2 changes: 1 addition & 1 deletion proxy/http/HttpSM.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4894,7 +4894,7 @@ HttpSM::do_http_server_open(bool raw)
}

// See if the outbound connection tracker data is needed. If so, get it here for consistency.
if (t_state.txn_conf->outbound_conntrack.max > 0 || t_state.http_config_param->origin_min_keep_alive_connections > 0) {
if (t_state.txn_conf->outbound_conntrack.max > 0 || t_state.txn_conf->outbound_conntrack.min > 0) {
t_state.outbound_conn_track_state = OutboundConnTrack::obtain(
t_state.txn_conf->outbound_conntrack, std::string_view{t_state.current.server->name}, t_state.current.server->dst_addr);
}
Expand Down
3 changes: 2 additions & 1 deletion proxy/http/HttpSessionManager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ ServerSessionPool::eventHandler(int event, void *data)
// origin, then reset the timeouts on our end and do not close the connection
if ((event == VC_EVENT_INACTIVITY_TIMEOUT || event == VC_EVENT_ACTIVE_TIMEOUT) && s->state == HSS_KA_SHARED &&
s->conn_track_group) {
bool connection_count_below_min = s->conn_track_group->_count <= http_config_params->origin_min_keep_alive_connections;
Debug("http_ss", "s->conn_track_group->min_keep_alive_conns : %d", s->conn_track_group->min_keep_alive_conns);
bool connection_count_below_min = s->conn_track_group->_count <= s->conn_track_group->min_keep_alive_conns;

if (connection_count_below_min) {
Debug("http_ss",
Expand Down
5 changes: 5 additions & 0 deletions src/traffic_server/InkAPI.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8535,6 +8535,10 @@ _conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overr
ret = &overridableHttpConfig->outbound_conntrack.max;
conv = &OutboundConnTrack::MAX_CONV;
break;
case TS_CONFIG_HTTP_SERVER_MIN_KEEP_ALIVE_CONNS:
ret = &overridableHttpConfig->outbound_conntrack.min;
conv = &OutboundConnTrack::MIN_CONV;
break;
case TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MATCH:
ret = &overridableHttpConfig->outbound_conntrack.match;
conv = &OutboundConnTrack::MATCH_CONV;
Expand Down Expand Up @@ -8828,6 +8832,7 @@ static const std::unordered_map<std::string_view, std::tuple<const TSOverridable
{"proxy.config.http.default_buffer_water_mark", {TS_CONFIG_HTTP_DEFAULT_BUFFER_WATER_MARK, TS_RECORDDATATYPE_INT}},
{"proxy.config.http.cache.heuristic_lm_factor", {TS_CONFIG_HTTP_CACHE_HEURISTIC_LM_FACTOR, TS_RECORDDATATYPE_FLOAT}},
{OutboundConnTrack::CONFIG_VAR_MAX, {TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MAX, TS_RECORDDATATYPE_INT}},
{OutboundConnTrack::CONFIG_VAR_MIN, {TS_CONFIG_HTTP_SERVER_MIN_KEEP_ALIVE_CONNS, TS_RECORDDATATYPE_INT}},
{"proxy.config.http.anonymize_remove_client_ip", {TS_CONFIG_HTTP_ANONYMIZE_REMOVE_CLIENT_IP, TS_RECORDDATATYPE_INT}},
{"proxy.config.http.cache.open_read_retry_time", {TS_CONFIG_HTTP_CACHE_OPEN_READ_RETRY_TIME, TS_RECORDDATATYPE_INT}},
{"proxy.config.http.down_server.abort_threshold", {TS_CONFIG_HTTP_DOWN_SERVER_ABORT_THRESHOLD, TS_RECORDDATATYPE_INT}},
Expand Down
1 change: 1 addition & 0 deletions src/traffic_server/InkAPITest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8641,6 +8641,7 @@ std::array<std::string_view, TS_CONFIG_LAST_ENTRY> SDK_Overridable_Configs = {
"proxy.config.http.allow_multi_range",
"proxy.config.http.request_buffer_enabled",
"proxy.config.http.allow_half_open",
OutboundConnTrack::CONFIG_VAR_MIN,
OutboundConnTrack::CONFIG_VAR_MAX,
OutboundConnTrack::CONFIG_VAR_MATCH,
"proxy.config.ssl.client.verify.server",
Expand Down