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/10/11 12:56:43 UTC

[GitHub] [trafficcontrol] traeak opened a new pull request, #7120: For parent_retry docs explain parent_retry deprecation

traeak opened a new pull request, #7120:
URL: https://github.com/apache/trafficcontrol/pull/7120

   <!--
   Thank you for contributing! Please be sure to read our contribution guidelines: https://github.com/apache/trafficcontrol/blob/master/CONTRIBUTING.md
   If this closes or relates to an existing issue, please reference it using one of the following:
   
   Closes: #ISSUE
   Related: #ISSUE
   
   If this PR fixes a security vulnerability, DO NOT submit! Instead, contact
   the Apache Traffic Control Security Team at security@trafficcontrol.apache.org and follow the
   guidelines at https://apache.org/security regarding vulnerability disclosure.
   -->
   
   The t3c logic currently ignores the `parent_retry` ds parameter and instead examines the use of parameters
   - max_simple_retries
   - simple_server_retry_responses
   - max_unavailable_server_retries
   - unavailable_server_retry_responses
   
   in order to infer what value to use for parent_retry in parent.config 
   
   Setting either of the above pairs to '0', '""' will disable, otherwise default is always "both".
   
   <!-- **^ Add meaningful description above** --><hr/>
   
   ## Which Traffic Control components are affected by this PR?
   <!-- Please delete all components from this list that are NOT affected by this PR.
   Feel free to add the name of a tool or script that is affected but not on the list.
   -->
   - Documentation
   
   ## What is the best way to verify this PR?
   <!-- Please include here ALL the steps necessary to test your PR.
   If your PR has tests (and most should), provide the steps needed to run the tests.
   If not, please provide step-by-step instructions to test the PR manually and explain why your PR does not need tests. -->
   
   
   ## If this is a bugfix, which Traffic Control versions contained the bug?
   <!-- Delete this section if the PR is not a bugfix, or if the bug is only in the master branch.
   Examples:
   - 5.1.2
   - 5.1.3 (RC1)
    -->
   
   
   ## PR submission checklist
   - [ ] This PR has tests -- documentation only
   - [x] This PR has documentation
   - [x] This PR has a CHANGELOG.md entry
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://apache.org/security) for details)
   
   <!--
   Licensed to the Apache Software Foundation (ASF) under one
   or more contributor license agreements.  See the NOTICE file
   distributed with this work for additional information
   regarding copyright ownership.  The ASF licenses this file
   to you under the Apache License, Version 2.0 (the
   "License"); you may not use this file except in compliance
   with the License.  You may obtain a copy of the License at
   
       http://www.apache.org/licenses/LICENSE-2.0
   
   Unless required by applicable law or agreed to in writing,
   software distributed under the License is distributed on an
   "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
   KIND, either express or implied.  See the License for the
   specific language governing permissions and limitations
   under the License.
   -->
   


-- 
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


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

Posted by GitBox <gi...@apache.org>.
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


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

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #7120:
URL: https://github.com/apache/trafficcontrol/pull/7120#discussion_r1047683940


##########
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:
   Oh, `last.parent_retry` isn't even in this file. That deprecation notice should be wherever the Parameter is documented. But it wouldn't surprise me if it wasn't at all outside of the quick-how-to, so if that's the case, just disregard that part of the comment.



-- 
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


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

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #7120:
URL: https://github.com/apache/trafficcontrol/pull/7120#discussion_r1048983044


##########
docs/source/overview/delivery_services.rst:
##########
@@ -1071,7 +1071,7 @@ Each :term:`Parameter` directly corresponds to a field in a line of the :abbr:`A
 .. _round_robin: https://docs.trafficserver.apache.org/en/9.1.x/admin-guide/files/parent.config.en.html#parent-config-format-round-robin
 .. _max_simple_retries: https://docs.trafficserver.apache.org/en/9.1.x/admin-guide/files/parent.config.en.html#parent-config-format-max-simple-retries
 .. _max_unavailable_server_retries: https://docs.trafficserver.apache.org/en/9.1.x/admin-guide/files/parent.config.en.html#parent-config-format-max-unavailable-server-retries
-.. _parent_retry: https://docs.trafficserver.apache.org/en/9.1.x/admin-guide/files/parent.config.en.html#parent-config-format-parent-retry
+.. _parent_retry: https://docs.trafficserver.apache.org/en/9.1.x/admin-guide/files/parent.config.en.html#parent-config-format-parent-retry (deprecated)

Review Comment:
   This breaks the link anchor, it now takes you to the top of the page (because there's no such anchor) instead of to the description of `parent_retry`



-- 
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


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

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #7120:
URL: https://github.com/apache/trafficcontrol/pull/7120#discussion_r1050904350


##########
CHANGELOG.md:
##########
@@ -29,6 +29,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#7037](https://github.com/apache/trafficcontrol/pull/7037) *Traffic Router* Uses Traffic Ops API 4.0 by default
 - [#7191](https://github.com/apache/trafficcontrol/issues/7191) *tc-health-client* Uses Traffic Ops API 4.0. Also added reload option to systemd service file
 - [#4654](https://github.com/apache/trafficcontrol/pull/4654) *Traffic Ops, Traffic Portal* Switched Delivery Service active state to a three-value system, adding a state that will be used to prevent cache servers from deploying DS configuration.
+- Switched Delivery Service active state to a three-value system, adding a state that will be used to prevent cache servers from deploying DS configuration.

Review Comment:
   This line appears to duplicate the one preceding it.



-- 
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


[GitHub] [trafficcontrol] ocket8888 merged pull request #7120: For delivery service parameter docs explain parent_retry deprecation

Posted by GitBox <gi...@apache.org>.
ocket8888 merged PR #7120:
URL: https://github.com/apache/trafficcontrol/pull/7120


-- 
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