You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2022/01/10 17:38:14 UTC

[GitHub] [trafficserver] jrushford opened a new pull request #8595: Adds two overridable config variables to control parent mark downs.

jrushford opened a new pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595


   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.


-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] zwoop commented on pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#issuecomment-1042436437


   Cherry-picked to v9.2.x


-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784953812



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather
+   than enabling this globally
+
+.. 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 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.

Review comment:
       @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.  
   
   @ywkaras I've pushed an update to the documentation.  Is it now clearer?




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784953812



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather
+   than enabling this globally
+
+.. 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 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.

Review comment:
       @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 though if you think it's necessary.  Just trying to keep it simple.




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] ywkaras commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784958140



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather

Review comment:
       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?




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] bneradt removed a comment on pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
bneradt removed a comment on pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#issuecomment-1010082951


   [approve ci]


-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784951088



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather

Review comment:
       @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?




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] ywkaras commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784954581



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather

Review comment:
       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




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784969452



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather

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




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784953812



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather
+   than enabling this globally
+
+.. 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 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.

Review comment:
       @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 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.  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
   
   @ywkaras I've pushed an update to the documentation.  Is it now clearer?




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784953812



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather
+   than enabling this globally
+
+.. 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 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.

Review comment:
       @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 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
   
   @ywkaras I've pushed an update to the documentation.  Is it now clearer?




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] ywkaras commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r785221551



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -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

Review comment:
       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:  https://github.com/apache/trafficserver/blob/4e6dc02f7ef7ae15ebda6c3bdf54e80f1ec55ba5/include/ts/ts.h#L2459 .  The lua plugin could also override them.




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784953812



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather
+   than enabling this globally
+
+.. 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 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.

Review comment:
       @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
   
   @ywkaras I've pushed an update to the documentation.  Is it now clearer?




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784953812



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather
+   than enabling this globally
+
+.. 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 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.

Review comment:
       @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
   




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784953812



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather
+   than enabling this globally
+
+.. 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 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.

Review comment:
       @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.  




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] ywkaras commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r785223239



##########
File path: proxy/http/HttpTransact.cc
##########
@@ -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;
+  }

Review comment:
       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.




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] ywkaras commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784377037



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather
+   than enabling this globally
+
+.. 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 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.

Review comment:
       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.




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] ywkaras commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784374328



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather

Review comment:
       Did you mean conf_remap, not header_rewrite?




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784953812



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather
+   than enabling this globally
+
+.. 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 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.

Review comment:
       @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.  
   
   @ywkaras I've pushed an update to the documentation.  Is it now clearer?




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784971754



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather

Review comment:
       @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 :)




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r785323795



##########
File path: proxy/http/HttpTransact.cc
##########
@@ -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;
+  }

Review comment:
       Ok, I’ll make the changes




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784953812



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather
+   than enabling this globally
+
+.. 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 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.

Review comment:
       @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 though if you think it's necessary.




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford merged pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford merged pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595


   


-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784953812



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather
+   than enabling this globally
+
+.. 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 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.

Review comment:
       @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
   
   @ywkaras I've pushed an update to the documentation.  Is it now clearer?




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] jrushford commented on a change in pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#discussion_r784980104



##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -1347,6 +1347,25 @@ 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.  The default is disabled (``0``).  This should be used for specific
+   remap entries using ``header_rewrite`` to enable inactivity timeout markdowns rather

Review comment:
       @ywkaras the docs now mention both plugins.




-- 
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: github-unsubscribe@trafficserver.apache.org

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



[GitHub] [trafficserver] bneradt commented on pull request #8595: Adds two overridable config variables to control parent mark downs.

Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #8595:
URL: https://github.com/apache/trafficserver/pull/8595#issuecomment-1010082951


   [approve ci]


-- 
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: github-unsubscribe@trafficserver.apache.org

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