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

jrushford · 2022-01-10T17:38:03Z

This PR adds two overridable config variables to control parent mark downs when using
parent selection or NextHop Strategies. These configs may be overridden by the
header_rewrite plugin on a per remap basis.

proxy.config.http.parent_proxy.enable_parent_timeout_markdowns is
used to enable parent markdowns when an inactivity timeout triggers
on a parent proxy. The default is disabled or '0'.
proxy.config.http.parent_proxy.disable_parent_markdowns is used to
disable all parent proxy markdowns. The default is disabled or '0'.

Currently when using either parent selection or next hop strategies, parents are marked down only
when a connection cannot be established, CONNECTION_ERROR, or when there is a DNS resolution
failure of the selected parent. We have use cases where we need to mark down a parent due to
INACTIVITY_TIMEOUT for a particular remap. Also, we have use cases where the parent endpoint
is a VIP and we wish not to mark down the VIP endpoint at all. These two configuration parameters
will allow us to control that by using the header_rewrite plugin on a per re-map basis.

ywkaras · 2022-01-13T22:31:09Z

doc/admin-guide/files/records.config.en.rst

+
+   Enables (``1``) or disables (``0``) parent proxy mark downs due to inactivity
+   timeouts.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather


Did you mean conf_remap, not header_rewrite?

@ywkaras I mean using the header_rewrite plugin to override the default value of the config so as to enable marking down on timeouts or by disabling markdowns altogether for a particular remap. Should I change header_rewrite to header_rewrite plugin?

So, I thought we normally used this plugin to change config values per remap rule: https://docs.trafficserver.apache.org/admin-guide/plugins/conf_remap.en.html

Oh OK, you can do it with both: https://docs.trafficserver.apache.org/admin-guide/plugins/header_rewrite.en.html#set-config . Is there reason to think these would only ever be overridden by header_rewrite? Maybe it's sufficient that they are marked as overridable?

@ywkaras I've updated the documentation. Please take a look.

@ywkaras thanks for the link. I've never used the conf_remap plugin. We've always used header_rewrite plugin for that purpose. I'll change the docs again :)

@ywkaras the docs now mention both plugins.

ywkaras · 2022-01-13T22:36:24Z

doc/admin-guide/files/records.config.en.rst

+   if a 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
+   this load balancer.  This should be used for specific remap entries using
+   ``header_rewrite`` rather than disabling parent mark downs globally.


Would it be cleaner to just have disable_parent_markdowns:
0 - parent markdowns enabled.
1 - (default) parent markdowns disabled due to timeouts.
2 - all parent markdowns disabled.

@ywkaras We want to do this using overridable configuration parameters. One wouldn't override both enabling timeout markdowns and disabling markdowns at the same time. I can add this to the documentation if it's not clear that this would not work. I prefer it as it is. I'll update the documentation as the config "enables" mark downs on inactivity timeouts, not "disable". By default '0', there are no mark downs on inactivity timeouts. I'll make it clearer in the documentation. I think it's clearer by seeing the name of the individual configuration parameters than using 0, 1, or 2 for a single parameter.

Currently it is hard coded that parents are marked down on connection errors and dns resolution errors. The intent of this PR is to add inactivity timeouts or to disable markdowns entirely in certain situations. So if I change this it would be something like:

parent_markdown mode:
0 - default, parents are marked down when a connection can't be established, connection errors, or dns resolution errors.
1 - adds markdowns on inactivity timeouts to the default mode.
2 - disable markdowns altogether

These configs may be overridden by the header_rewrite plugin on a per remap basis. - proxy.config.http.parent_proxy.enable_parent_timeout_markdowns is used to enable parent markdowns when an inactivity timeout triggers on a parent proxy. The default is disabled or '0'. - proxy.config.http.parent_proxy.disable_parent_markdowns is used to disable all parent proxy markdowns. The default is disabled or '0'.

ywkaras · 2022-01-14T23:10:28Z

doc/admin-guide/files/records.config.en.rst

+   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


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.

ywkaras · 2022-01-14T23:16:26Z

proxy/http/HttpTransact.cc

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


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.

Ok, I’ll make the changes

ywkaras

I gave Yahoo Prod a chance to whine about this but they didn't, so I'll leave it up to John if it's better to keep the two config variables or consolidate into one.

zwoop · 2022-02-17T00:20:04Z

Cherry-picked to v9.2.x

…8595) These configs may be overridden by the header_rewrite plugin on a per remap basis. - proxy.config.http.parent_proxy.enable_parent_timeout_markdowns is used to enable parent markdowns when an inactivity timeout triggers on a parent proxy. The default is disabled or '0'. - proxy.config.http.parent_proxy.disable_parent_markdowns is used to disable all parent proxy markdowns. The default is disabled or '0'. (cherry picked from commit 427c42f)

* asf/9.2.x: Revert "DNS: Fix lack of nameserver failover in low use circumstances. (apache#7843)" (apache#8663) Fix strategies to initialize scheme (apache#8650) DNS: Fix lack of nameserver failover in low use circumstances. (apache#7843) Cleanup strategy debug logs (apache#8656) Making 9.2.x backwards compatible with 9.1.x (apache#8661) Adds two overridable config variables to control parent mark downs. (apache#8595) Fix plugin parent_select missing hostname len (apache#8649) Ports apache#7925 apache#8365 core to parent_select plugin (apache#8590) Ports apache#7897 from core strategies to parent_select plugin. (apache#8580) Adding clangd language server files to .gitignore (apache#8640) Make TsSharedMutex.h compile on MacOS. (apache#8645) In TsSharedMutex.h, make error reporting thread-safe. (apache#8636) Revert "body factory does not respect runroot (apache#8388)" (apache#8654) doc: Convert miscased Traffic Server references to |TS| macro (apache#8543) Add a new --enable-event-tracker configure option (apache#8179) Add parent_select plugin strategy caching (apache#8651) TLS Session Resumption: fix timed out session (apache#8667) Fix to allow running from outside top_srcdir (apache#8673) Send diags output to stderr when running regression tests. (apache#8662) Default proxy.config.http.strict_uri_parsing to "2" (apache#8632)

jrushford added this to the 10.0.0 milestone Jan 10, 2022

jrushford self-assigned this Jan 10, 2022

jrushford requested review from SolidWallOfCode, bryancall and zwoop as code owners January 10, 2022 17:38

bryancall added the Parent Proxy label Jan 11, 2022

bryancall requested a review from ywkaras January 11, 2022 00:04

jrushford force-pushed the parent_selection_markdowns branch from a3ba8ea to 3cc5150 Compare January 11, 2022 14:16

ywkaras reviewed Jan 13, 2022

View reviewed changes

jrushford force-pushed the parent_selection_markdowns branch 4 times, most recently from 77edeb1 to 563e5d7 Compare January 14, 2022 16:42

jrushford force-pushed the parent_selection_markdowns branch from 563e5d7 to 7786e43 Compare January 14, 2022 17:15

ywkaras reviewed Jan 14, 2022

View reviewed changes

ywkaras approved these changes Jan 18, 2022

View reviewed changes

jrushford merged commit 427c42f into apache:master Jan 18, 2022

zwoop modified the milestones: 10.0.0, 9.2.0 Feb 17, 2022

vmamidi added the New Feature label Nov 14, 2022

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

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

Uh oh!

Conversation

jrushford commented Jan 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrushford Jan 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ywkaras left a comment

Choose a reason for hiding this comment

Uh oh!

zwoop commented Feb 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jrushford Jan 14, 2022 •

edited

Loading