Skip to content

Adds two overridable config variables to control parent mark downs. #8595

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
jrushford merged 1 commit into from
Jan 18, 2022
Merged

Adds two overridable config variables to control parent mark downs. #8595

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
26 changes: 26 additions & 0 deletions 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 @@ -1347,6 +1347,32 @@ Parent Proxy Configuration
``2`` Mark the host down. This is the default.
===== ======================================================================

.. ts:cv:: CONFIG proxy.config.http.parent_proxy.enable_parent_timeout_markdowns INT 0
:reloadable:
:overridable:

Enables (``1``) or disables (``0``) parent proxy mark downs due to inactivity
timeouts. By default parent proxies are not marked down due to inactivity
timeouts, the transaction will retry using another parent instead. The
default for this configuration keeps this behavior and is disabled (``0``).
This setting is overridable using one of the two plugins ``header_rewrite``
or ``conf_remap`` to enable inactivity timeout markdowns and should be done
Copy link
Contributor

@ywkaras ywkaras Jan 14, 2022

Choose a reason for hiding this comment

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

Why do you feel like we should mention these particular plugins as the ones overriding the configs? Any plugin that uses these TS API calls can override them:

trafficserver/include/ts/ts.h

Line 2459 in 4e6dc02

tsapi TSReturnCode TSHttpTxnConfigIntSet(TSHttpTxn txnp, TSOverridableConfigKey conf, TSMgmtInt value);
. The lua plugin could also override them.

so rather than enabling this globally. This setting should not be used in
conjunction with ``proxy.config.http.parent_proxy.disable_parent_markdowns``

.. ts:cv:: CONFIG proxy.config.http.parent_proxy.disable_parent_markdowns INT 0
:reloadable:
:overridable:

Enables (``1``) or disables (``0``) parent proxy markdowns. This is useful
if parent entries in a parent.config line are VIP's and one doesn't wish
to mark down a VIP which may have several origin or parent proxies behind
the load balancer. This setting is overridable using one of the
``header_rewrite`` or the ``conf_remap`` plugins to override the default
setting and this method should be used rather than disabling markdowns
globally. This setting should not be used in conjunction with
``proxy.config.http.parent_proxy.enable_parent_timeout_markdowns``

HTTP Connection Timeouts
========================

Expand Down
2 changes: 2 additions & 0 deletions include/ts/apidefs.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,8 @@ typedef enum {
TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK,
TS_CONFIG_NET_SOCK_NOTSENT_LOWAT,
TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE,
TS_CONFIG_HTTP_ENABLE_PARENT_TIMEOUT_MARKDOWNS,
TS_CONFIG_HTTP_DISABLE_PARENT_MARKDOWNS,
TS_CONFIG_LAST_ENTRY
} TSOverridableConfigKey;

Expand Down
4 changes: 4 additions & 0 deletions mgmt/RecordsConfig.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,10 @@ static const RecordElement RecordsConfig[] =
,
{RECT_CONFIG, "proxy.config.http.parent_proxy.self_detect", RECD_INT, "2", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
,
{RECT_CONFIG, "proxy.config.http.parent_proxy.enable_parent_timeout_markdowns", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
,
{RECT_CONFIG, "proxy.config.http.parent_proxy.disable_parent_markdowns", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
,
{RECT_CONFIG, "proxy.config.http.forward.proxy_auth_to_parent", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL}
,

Expand Down
4 changes: 4 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 @@ -143,6 +143,8 @@ typedef enum {
TS_LUA_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK = TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK,
TS_LUA_CONFIG_NET_SOCK_NOTSENT_LOWAT = TS_CONFIG_NET_SOCK_NOTSENT_LOWAT,
TS_LUA_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE = TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE,
TS_LUA_CONFIG_ENABLE_PARENT_TIMEOUT_MARKDOWNS = TS_CONFIG_HTTP_ENABLE_PARENT_TIMEOUT_MARKDOWNS,
TS_LUA_CONFIG_DISABLE_PARENT_MARKDOWNS = TS_CONFIG_HTTP_DISABLE_PARENT_MARKDOWNS,
TS_LUA_CONFIG_LAST_ENTRY = TS_CONFIG_LAST_ENTRY,
} TSLuaOverridableConfigKey;

Expand Down Expand Up @@ -278,6 +280,8 @@ ts_lua_var_item ts_lua_http_config_vars[] = {
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_NET_SOCK_NOTSENT_LOWAT),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_ENABLE_PARENT_TIMEOUT_MARKDOWNS),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_DISABLE_PARENT_MARKDOWNS),
TS_LUA_MAKE_VAR_ITEM(TS_LUA_CONFIG_LAST_ENTRY),
};

Expand Down
25 changes: 15 additions & 10 deletions proxy/http/HttpConfig.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,9 @@ HttpConfig::startup()
"proxy.config.http.parent_proxy.per_parent_connect_attempts");
HttpEstablishStaticConfigLongLong(c.oride.parent_connect_timeout, "proxy.config.http.parent_proxy.connect_attempts_timeout");
HttpEstablishStaticConfigByte(c.oride.parent_failures_update_hostdb, "proxy.config.http.parent_proxy.mark_down_hostdb");
HttpEstablishStaticConfigByte(c.oride.enable_parent_timeout_markdowns,
"proxy.config.http.parent_proxy.enable_parent_timeout_markdowns");
HttpEstablishStaticConfigByte(c.oride.disable_parent_markdowns, "proxy.config.http.parent_proxy.disable_parent_markdowns");

HttpEstablishStaticConfigLongLong(c.oride.sock_recv_buffer_size_out, "proxy.config.net.sock_recv_buffer_size_out");
HttpEstablishStaticConfigLongLong(c.oride.sock_send_buffer_size_out, "proxy.config.net.sock_send_buffer_size_out");
Expand Down Expand Up @@ -1535,16 +1538,18 @@ HttpConfig::reconfigure()
"will never redispatch to another server",
m_master.oride.connect_attempts_rr_retries, params->oride.connect_attempts_max_retries);
}
params->oride.connect_attempts_rr_retries = m_master.oride.connect_attempts_rr_retries;
params->oride.connect_attempts_timeout = m_master.oride.connect_attempts_timeout;
params->oride.connect_dead_policy = m_master.oride.connect_dead_policy;
params->oride.post_connect_attempts_timeout = m_master.oride.post_connect_attempts_timeout;
params->oride.parent_connect_attempts = m_master.oride.parent_connect_attempts;
params->oride.parent_retry_time = m_master.oride.parent_retry_time;
params->oride.parent_fail_threshold = m_master.oride.parent_fail_threshold;
params->oride.per_parent_connect_attempts = m_master.oride.per_parent_connect_attempts;
params->oride.parent_connect_timeout = m_master.oride.parent_connect_timeout;
params->oride.parent_failures_update_hostdb = m_master.oride.parent_failures_update_hostdb;
params->oride.connect_attempts_rr_retries = m_master.oride.connect_attempts_rr_retries;
params->oride.connect_attempts_timeout = m_master.oride.connect_attempts_timeout;
params->oride.connect_dead_policy = m_master.oride.connect_dead_policy;
params->oride.post_connect_attempts_timeout = m_master.oride.post_connect_attempts_timeout;
params->oride.parent_connect_attempts = m_master.oride.parent_connect_attempts;
params->oride.parent_retry_time = m_master.oride.parent_retry_time;
params->oride.parent_fail_threshold = m_master.oride.parent_fail_threshold;
params->oride.per_parent_connect_attempts = m_master.oride.per_parent_connect_attempts;
params->oride.parent_connect_timeout = m_master.oride.parent_connect_timeout;
params->oride.parent_failures_update_hostdb = m_master.oride.parent_failures_update_hostdb;
params->oride.enable_parent_timeout_markdowns = m_master.oride.enable_parent_timeout_markdowns;
params->oride.disable_parent_markdowns = m_master.oride.disable_parent_markdowns;

params->oride.sock_recv_buffer_size_out = m_master.oride.sock_recv_buffer_size_out;
params->oride.sock_send_buffer_size_out = m_master.oride.sock_send_buffer_size_out;
Expand Down
12 changes: 7 additions & 5 deletions proxy/http/HttpConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,11 +678,13 @@ struct OverridableHttpConfigParams {
////////////////////////////////////
// parent proxy connect attempts //
///////////////////////////////////
MgmtInt parent_connect_attempts = 4;
MgmtInt parent_retry_time = 300;
MgmtInt parent_fail_threshold = 10;
MgmtInt per_parent_connect_attempts = 2;
MgmtInt parent_connect_timeout = 30;
MgmtInt parent_connect_attempts = 4;
MgmtInt parent_retry_time = 300;
MgmtInt parent_fail_threshold = 10;
MgmtInt per_parent_connect_attempts = 2;
MgmtInt parent_connect_timeout = 30;
MgmtByte enable_parent_timeout_markdowns = 0;
MgmtByte disable_parent_markdowns = 0;

MgmtInt down_server_timeout = 300;
MgmtInt client_abort_threshold = 1000;
Expand Down
16 changes: 14 additions & 2 deletions proxy/http/HttpTransact.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,18 @@ markParentDown(HttpTransact::State *s)
HTTP_INCREMENT_DYN_STAT(http_total_parent_marked_down_count);
url_mapping *mp = s->url_map.getMapping();

TxnDebug("http_trans", "sm_id[%" PRId64 "] enable_parent_timeout_markdowns: %d, disable_parent_markdowns: %d",
s->state_machine->sm_id, s->txn_conf->enable_parent_timeout_markdowns, s->txn_conf->disable_parent_markdowns);

if (s->txn_conf->disable_parent_markdowns == 1) {
TxnDebug("http_trans", "parent markdowns are disabled for this request");
return;
}
Copy link
Contributor

@ywkaras ywkaras Jan 14, 2022

Choose a reason for hiding this comment

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

If disable_parent_markdowns is 1, we return on line 222, so the value of enable_parent_timeout_markdowns is moot. That is what makes me think it would be clearer to combine the two settings into a single one.

Copy link
Contributor Author

@jrushford jrushford Jan 15, 2022

Choose a reason for hiding this comment

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

Ok, I’ll make the changes


if (s->current.state == HttpTransact::INACTIVE_TIMEOUT && s->txn_conf->enable_parent_timeout_markdowns == 0) {
return;
}

if (s->response_action.handled) {
// Do nothing. If a plugin handled the response, let it handle markdown.
} else if (mp && mp->strategy) {
Expand Down Expand Up @@ -3702,7 +3714,7 @@ HttpTransact::handle_response_from_parent(State *s)
// Only mark the parent down if we failed to connect
// to the parent otherwise slow origin servers cause
// us to mark the parent down
if (s->current.state == CONNECTION_ERROR) {
if (s->current.state == CONNECTION_ERROR || s->current.state == INACTIVE_TIMEOUT) {
markParentDown(s);
}
// We are done so look for another parent if any
Expand All @@ -3713,7 +3725,7 @@ HttpTransact::handle_response_from_parent(State *s)
// appropriate
HTTP_INCREMENT_DYN_STAT(http_total_parent_retries_exhausted_stat);
TxnDebug("http_trans", "[handle_response_from_parent] Error. No more retries.");
if (s->current.state == CONNECTION_ERROR) {
if (s->current.state == CONNECTION_ERROR || s->current.state == INACTIVE_TIMEOUT) {
markParentDown(s);
}
s->parent_result.result = PARENT_FAIL;
Expand Down
6 changes: 5 additions & 1 deletion src/shared/overridable_txn_vars.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,8 @@ const std::unordered_map<std::string_view, std::tuple<const TSOverridableConfigK
{"proxy.config.plugin.vc.default_buffer_water_mark", {TS_CONFIG_PLUGIN_VC_DEFAULT_BUFFER_WATER_MARK, TS_RECORDDATATYPE_INT}},
{"proxy.config.net.sock_notsent_lowat", {TS_CONFIG_NET_SOCK_NOTSENT_LOWAT, TS_RECORDDATATYPE_INT}},
{"proxy.config.body_factory.response_suppression_mode",
{TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE, TS_RECORDDATATYPE_INT}}});
{TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE, TS_RECORDDATATYPE_INT}},
{"proxy.config.http.parent_proxy.enable_parent_timeout_markdowns",
{TS_CONFIG_HTTP_ENABLE_PARENT_TIMEOUT_MARKDOWNS, TS_RECORDDATATYPE_INT}},
{"proxy.config.http.parent_proxy.disable_parent_markdowns",
{TS_CONFIG_HTTP_DISABLE_PARENT_MARKDOWNS, TS_RECORDDATATYPE_INT}}});
6 changes: 6 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 @@ -8960,6 +8960,12 @@ _conf_to_memberp(TSOverridableConfigKey conf, OverridableHttpConfigParams *overr
case TS_CONFIG_BODY_FACTORY_RESPONSE_SUPPRESSION_MODE:
ret = _memberp_to_generic(&overridableHttpConfig->response_suppression_mode, conv);
break;
case TS_CONFIG_HTTP_ENABLE_PARENT_TIMEOUT_MARKDOWNS:
ret = _memberp_to_generic(&overridableHttpConfig->enable_parent_timeout_markdowns, conv);
break;
case TS_CONFIG_HTTP_DISABLE_PARENT_MARKDOWNS:
ret = _memberp_to_generic(&overridableHttpConfig->disable_parent_markdowns, conv);
break;

// This helps avoiding compiler warnings, yet detect unhandled enum members.
case TS_CONFIG_NULL:
Expand Down
4 changes: 3 additions & 1 deletion src/traffic_server/InkAPITest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8698,7 +8698,9 @@ std::array<std::string_view, TS_CONFIG_LAST_ENTRY> SDK_Overridable_Configs = {
"proxy.config.plugin.vc.default_buffer_index",
"proxy.config.plugin.vc.default_buffer_water_mark",
"proxy.config.net.sock_notsent_lowat",
"proxy.config.body_factory.response_suppression_mode"}};
"proxy.config.body_factory.response_suppression_mode",
"proxy.config.http.parent_proxy.enable_parent_timeout_markdowns",
"proxy.config.http.parent_proxy.disable_parent_markdowns"}};

extern ClassAllocator<HttpSM> httpSMAllocator;

Expand Down