You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/12/13 20:09:02 UTC

[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7120: For delivery service parameter docs explain parent_retry deprecation

ocket8888 commented on code in PR #7120:
URL: https://github.com/apache/trafficcontrol/pull/7120#discussion_r1047675448


##########
docs/source/overview/delivery_services.rst:
##########
@@ -1087,27 +1087,31 @@ Each :term:`Parameter` directly corresponds to a field in a line of the :abbr:`A
 	| algorithm                               | `round_robin`_                                             | Sets the algorithm used to determine from which :term:`origin server` content will  |
 	|                                         |                                                            | be requested.                                                                       |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
-	| max_simple_retries                      | `max_simple_retries`_                                      | Sets a strict limit on the number of "simple retries" allowed before giving up      |
-	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
-	| max_unavailable_server_retries          | `max_unavailable_server_retries`_                          | Sets a strict limit on the number of times the :term:`cache server` will attempt to |
-	|                                         |                                                            | request content from an :term:`origin server` that has previously been considered   |
-	|                                         |                                                            | "unavailable".                                                                      |
-	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| parent_retry                            | `parent_retry`_                                            | Sets whether the :term:`cache servers` will use "simple retries",                   |
-	|                                         |                                                            | "unavailable server retries", or both.                                              |
+	|                                         |                                                            | "unavailable server retries", or both.  Deprecated (see below).                     |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| simple_retry_response_codes             | **UNKNOWN**                                                | **UNKNOWN** - supposedly defines HTTP response codes from an :term:`origin server`  |
 	|                                         |                                                            | that necessitate a "simple retry".                                                  |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
+	| max_simple_retries                      | `max_simple_retries`_                                      | Sets a strict limit on the number of "simple retries" allowed before giving up      |
+	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| unavailable_server_retry_response_codes | `unavailable_server_retry_responses`_                      | Defines HTTP response codes from an :term:`origin server` that indicate it is       |
 	|                                         |                                                            | currently "unavailable".                                                            |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
+	| max_unavailable_server_retries          | `max_unavailable_server_retries`_                          | Sets a strict limit on the number of times the :term:`cache server` will attempt to |
+	|                                         |                                                            | request content from an :term:`origin server` that has previously been considered   |
+	|                                         |                                                            | "unavailable".                                                                      |
+	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 
 The above :term:`Parameters` are supported for ``first``, ``inner`` and ``last`` tiers by specifying prefixes ``first.``, ``inner.`` and ``last.``, applicable to both topology and non topology. This allows fine tuning of marking parents "down" and retry behavior inside a CDN.
 
 .. deprecated:: The ``mso.`` prefix is deprecated.  ``last.`` prefix should be preferred although no prefix can also be used.
 
-.. warning:: The `simple_retry_response_codes`` :term:`Parameter` has no apparent, possible use according to the :abbr:`ATS (Apache Traffic Server)` `parent.config documentation <https://docs.trafficserver.apache.org/en/9.1.x/admin-guide/files/parent.config.en.html>`_. Whether or not it has any effect - let alone the *intended* effect - is not known, and its use is therefore strongly discouraged.
+.. deprecated:: The parent_retry parameter is now inferred from the `simple retry` and `unavailable server retry` parameters.  To disable `simple retries` associate parameter ``max_simple_retries`` of ``0`` and ``max_simple_retry_responses`` of ``""``.  Similarly "unavailable server retries" may also be disabled.
+
+.. impl-detail:: With Apache Traffic Server 8.1.x the `simple_retry_response_codes` setting is not available.

Review Comment:
   "_simple_retry_response_codes_" is probably meant to be "`simple_retry_response_codes`".



##########
docs/source/overview/delivery_services.rst:
##########
@@ -1087,27 +1087,31 @@ Each :term:`Parameter` directly corresponds to a field in a line of the :abbr:`A
 	| algorithm                               | `round_robin`_                                             | Sets the algorithm used to determine from which :term:`origin server` content will  |
 	|                                         |                                                            | be requested.                                                                       |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
-	| max_simple_retries                      | `max_simple_retries`_                                      | Sets a strict limit on the number of "simple retries" allowed before giving up      |
-	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
-	| max_unavailable_server_retries          | `max_unavailable_server_retries`_                          | Sets a strict limit on the number of times the :term:`cache server` will attempt to |
-	|                                         |                                                            | request content from an :term:`origin server` that has previously been considered   |
-	|                                         |                                                            | "unavailable".                                                                      |
-	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| parent_retry                            | `parent_retry`_                                            | Sets whether the :term:`cache servers` will use "simple retries",                   |
-	|                                         |                                                            | "unavailable server retries", or both.                                              |
+	|                                         |                                                            | "unavailable server retries", or both.  Deprecated (see below).                     |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| simple_retry_response_codes             | **UNKNOWN**                                                | **UNKNOWN** - supposedly defines HTTP response codes from an :term:`origin server`  |
 	|                                         |                                                            | that necessitate a "simple retry".                                                  |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
+	| max_simple_retries                      | `max_simple_retries`_                                      | Sets a strict limit on the number of "simple retries" allowed before giving up      |
+	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| unavailable_server_retry_response_codes | `unavailable_server_retry_responses`_                      | Defines HTTP response codes from an :term:`origin server` that indicate it is       |
 	|                                         |                                                            | currently "unavailable".                                                            |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
+	| max_unavailable_server_retries          | `max_unavailable_server_retries`_                          | Sets a strict limit on the number of times the :term:`cache server` will attempt to |
+	|                                         |                                                            | request content from an :term:`origin server` that has previously been considered   |
+	|                                         |                                                            | "unavailable".                                                                      |
+	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 
 The above :term:`Parameters` are supported for ``first``, ``inner`` and ``last`` tiers by specifying prefixes ``first.``, ``inner.`` and ``last.``, applicable to both topology and non topology. This allows fine tuning of marking parents "down" and retry behavior inside a CDN.
 
 .. deprecated:: The ``mso.`` prefix is deprecated.  ``last.`` prefix should be preferred although no prefix can also be used.
 
-.. warning:: The `simple_retry_response_codes`` :term:`Parameter` has no apparent, possible use according to the :abbr:`ATS (Apache Traffic Server)` `parent.config documentation <https://docs.trafficserver.apache.org/en/9.1.x/admin-guide/files/parent.config.en.html>`_. Whether or not it has any effect - let alone the *intended* effect - is not known, and its use is therefore strongly discouraged.
+.. deprecated:: The parent_retry parameter is now inferred from the `simple retry` and `unavailable server retry` parameters.  To disable `simple retries` associate parameter ``max_simple_retries`` of ``0`` and ``max_simple_retry_responses`` of ``""``.  Similarly "unavailable server retries" may also be disabled.

Review Comment:
   This lists `parent_retry` as deprecated, but should also mention `last.parent_retry`, unless you'd rather give that its own deprecation notice.
   
   `parent_retry` should probably be monospace.
   
   "_simple retry_" is probably meant to be "`simple_retry`" - using a single grave accent (or "backtick": <kbd>`</kbd>) to surround a body of text emphasizes it by default, not monospace as in Markdown. You need two. Also missing the underscore.
   
    "_unavailable server retry_ parameters" should probably be "`unavailable_server_retries`, `unavailable_server_retry_response_codes`, and `max_unavailable_server_retries` Parameters" (note the title-casing on "Parameters").
    
    Extra space between "." and "To", not a big deal but just looks weird IMO.
    
    "To disable _simple retries_ associate parameter `max_simple_retries` of `0` and `max_simple_retry_responses` of `""`.  Similarly 'unavailable server retries' may also be disabled." should probably be
   
   ```rst
   To disable "simple retries" for a :term:`Profile`, set the Value of its ``max_simple_retries`` :term:`Parameter` to ``0``, and the Value of its ``max_simple_retry_responses`` :term:`Parameter` to an empty string. "Unavailable server retries" may disabled in much the same way, using the analogous :term:`Parameters`.
   ```



##########
docs/source/overview/delivery_services.rst:
##########
@@ -1087,27 +1087,31 @@ Each :term:`Parameter` directly corresponds to a field in a line of the :abbr:`A
 	| algorithm                               | `round_robin`_                                             | Sets the algorithm used to determine from which :term:`origin server` content will  |
 	|                                         |                                                            | be requested.                                                                       |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
-	| max_simple_retries                      | `max_simple_retries`_                                      | Sets a strict limit on the number of "simple retries" allowed before giving up      |
-	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
-	| max_unavailable_server_retries          | `max_unavailable_server_retries`_                          | Sets a strict limit on the number of times the :term:`cache server` will attempt to |
-	|                                         |                                                            | request content from an :term:`origin server` that has previously been considered   |
-	|                                         |                                                            | "unavailable".                                                                      |
-	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| parent_retry                            | `parent_retry`_                                            | Sets whether the :term:`cache servers` will use "simple retries",                   |
-	|                                         |                                                            | "unavailable server retries", or both.                                              |
+	|                                         |                                                            | "unavailable server retries", or both.  Deprecated (see below).                     |

Review Comment:
   Instead of "see below", the docs guidelines encourage using actual links, to make the flow of a document less fragile:
   
   > _"When referencing information in another section on the same page, please do not refer to the current placement of the referenced content relative to the referencing content. For example, instead of 'as discussed below', use 'as discussed in [Terms](https://traffic-control-cdn.readthedocs.io/en/latest/development/documentation_guidelines.html#terms)'."_
   
   This is because tables, figures, asides and some others are all "floating environments", and the output format is not strictly required to replicate the exact placement of those floating environments with respect to surrounding text. For example, in the PDF version of the docs, this table would likely be pushed to the top of the next available page, which could cause the text to which it refers to actually precede it. In that case, "above" and "below" make no sense anyway, because the entire section isn't on a single page.
   
   You could use a footnote, or simply mark it as "deprecated" and let the admonition speak for itself on the matter. Or you could try to link it, but I suspect you'll quickly decide that's too annoying to bother with - because it kinda is.



##########
docs/source/overview/delivery_services.rst:
##########
@@ -1087,27 +1087,31 @@ Each :term:`Parameter` directly corresponds to a field in a line of the :abbr:`A
 	| algorithm                               | `round_robin`_                                             | Sets the algorithm used to determine from which :term:`origin server` content will  |
 	|                                         |                                                            | be requested.                                                                       |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
-	| max_simple_retries                      | `max_simple_retries`_                                      | Sets a strict limit on the number of "simple retries" allowed before giving up      |
-	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
-	| max_unavailable_server_retries          | `max_unavailable_server_retries`_                          | Sets a strict limit on the number of times the :term:`cache server` will attempt to |
-	|                                         |                                                            | request content from an :term:`origin server` that has previously been considered   |
-	|                                         |                                                            | "unavailable".                                                                      |
-	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| parent_retry                            | `parent_retry`_                                            | Sets whether the :term:`cache servers` will use "simple retries",                   |
-	|                                         |                                                            | "unavailable server retries", or both.                                              |
+	|                                         |                                                            | "unavailable server retries", or both.  Deprecated (see below).                     |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| simple_retry_response_codes             | **UNKNOWN**                                                | **UNKNOWN** - supposedly defines HTTP response codes from an :term:`origin server`  |
 	|                                         |                                                            | that necessitate a "simple retry".                                                  |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
+	| max_simple_retries                      | `max_simple_retries`_                                      | Sets a strict limit on the number of "simple retries" allowed before giving up      |
+	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| unavailable_server_retry_response_codes | `unavailable_server_retry_responses`_                      | Defines HTTP response codes from an :term:`origin server` that indicate it is       |
 	|                                         |                                                            | currently "unavailable".                                                            |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
+	| max_unavailable_server_retries          | `max_unavailable_server_retries`_                          | Sets a strict limit on the number of times the :term:`cache server` will attempt to |
+	|                                         |                                                            | request content from an :term:`origin server` that has previously been considered   |
+	|                                         |                                                            | "unavailable".                                                                      |
+	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 
 The above :term:`Parameters` are supported for ``first``, ``inner`` and ``last`` tiers by specifying prefixes ``first.``, ``inner.`` and ``last.``, applicable to both topology and non topology. This allows fine tuning of marking parents "down" and retry behavior inside a CDN.
 
 .. deprecated:: The ``mso.`` prefix is deprecated.  ``last.`` prefix should be preferred although no prefix can also be used.
 
-.. warning:: The `simple_retry_response_codes`` :term:`Parameter` has no apparent, possible use according to the :abbr:`ATS (Apache Traffic Server)` `parent.config documentation <https://docs.trafficserver.apache.org/en/9.1.x/admin-guide/files/parent.config.en.html>`_. Whether or not it has any effect - let alone the *intended* effect - is not known, and its use is therefore strongly discouraged.
+.. deprecated:: The parent_retry parameter is now inferred from the `simple retry` and `unavailable server retry` parameters.  To disable `simple retries` associate parameter ``max_simple_retries`` of ``0`` and ``max_simple_retry_responses`` of ``""``.  Similarly "unavailable server retries" may also be disabled.
+
+.. impl-detail:: With Apache Traffic Server 8.1.x the `simple_retry_response_codes` setting is not available.
+.. impl-detail:: With Apache Traffic Server 9.1.x `unavailable_server_retry_response_codes` are limited to 5xx responses and `simple_retry_response_codes` are limited to 4xx.

Review Comment:
   "_unavailable_server_retry_response_codes_" is probably meant to be "`unavailable_server_retry_response_codes`".
   
   "_simple_retry_response_codes_" is probably meant to be "`simple_retry_response_codes`".



##########
docs/source/overview/delivery_services.rst:
##########
@@ -1087,27 +1087,31 @@ Each :term:`Parameter` directly corresponds to a field in a line of the :abbr:`A
 	| algorithm                               | `round_robin`_                                             | Sets the algorithm used to determine from which :term:`origin server` content will  |
 	|                                         |                                                            | be requested.                                                                       |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
-	| max_simple_retries                      | `max_simple_retries`_                                      | Sets a strict limit on the number of "simple retries" allowed before giving up      |
-	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
-	| max_unavailable_server_retries          | `max_unavailable_server_retries`_                          | Sets a strict limit on the number of times the :term:`cache server` will attempt to |
-	|                                         |                                                            | request content from an :term:`origin server` that has previously been considered   |
-	|                                         |                                                            | "unavailable".                                                                      |
-	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| parent_retry                            | `parent_retry`_                                            | Sets whether the :term:`cache servers` will use "simple retries",                   |
-	|                                         |                                                            | "unavailable server retries", or both.                                              |
+	|                                         |                                                            | "unavailable server retries", or both.  Deprecated (see below).                     |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| simple_retry_response_codes             | **UNKNOWN**                                                | **UNKNOWN** - supposedly defines HTTP response codes from an :term:`origin server`  |
 	|                                         |                                                            | that necessitate a "simple retry".                                                  |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
+	| max_simple_retries                      | `max_simple_retries`_                                      | Sets a strict limit on the number of "simple retries" allowed before giving up      |
+	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 	| unavailable_server_retry_response_codes | `unavailable_server_retry_responses`_                      | Defines HTTP response codes from an :term:`origin server` that indicate it is       |
 	|                                         |                                                            | currently "unavailable".                                                            |
 	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
+	| max_unavailable_server_retries          | `max_unavailable_server_retries`_                          | Sets a strict limit on the number of times the :term:`cache server` will attempt to |
+	|                                         |                                                            | request content from an :term:`origin server` that has previously been considered   |
+	|                                         |                                                            | "unavailable".                                                                      |
+	+-----------------------------------------+------------------------------------------------------------+-------------------------------------------------------------------------------------+
 
 The above :term:`Parameters` are supported for ``first``, ``inner`` and ``last`` tiers by specifying prefixes ``first.``, ``inner.`` and ``last.``, applicable to both topology and non topology. This allows fine tuning of marking parents "down" and retry behavior inside a CDN.
 
 .. deprecated:: The ``mso.`` prefix is deprecated.  ``last.`` prefix should be preferred although no prefix can also be used.
 
-.. warning:: The `simple_retry_response_codes`` :term:`Parameter` has no apparent, possible use according to the :abbr:`ATS (Apache Traffic Server)` `parent.config documentation <https://docs.trafficserver.apache.org/en/9.1.x/admin-guide/files/parent.config.en.html>`_. Whether or not it has any effect - let alone the *intended* effect - is not known, and its use is therefore strongly discouraged.
+.. deprecated:: The parent_retry parameter is now inferred from the `simple retry` and `unavailable server retry` parameters.  To disable `simple retries` associate parameter ``max_simple_retries`` of ``0`` and ``max_simple_retry_responses`` of ``""``.  Similarly "unavailable server retries" may also be disabled.
+
+.. impl-detail:: With Apache Traffic Server 8.1.x the `simple_retry_response_codes` setting is not available.
+.. impl-detail:: With Apache Traffic Server 9.1.x `unavailable_server_retry_response_codes` are limited to 5xx responses and `simple_retry_response_codes` are limited to 4xx.
+.. impl-detail:: Apache Traffic Server 9.2.x allows more flexibility with 4xx and 5xx codes available for use with `simple_retry_response_codes`.

Review Comment:
   "_simple_retry_response_codes_" is probably meant to be "`simple_retry_response_codes`".



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org