Copilot commented on code in PR #13102:
URL: https://github.com/apache/trafficserver/pull/13102#discussion_r3150531201


##########
src/proxy/http/HttpTransact.cc:
##########
@@ -3970,63 +4007,92 @@ HttpTransact::handle_response_from_server(State *s)
       }
     }
 
-    if (is_server_negative_cached(s)) {
-      max_connect_retries = 
s->txn_conf->connect_attempts_max_retries_down_server - 1;
-    } else {
-      // server not yet negative cached - use default number of retries
-      max_connect_retries = s->txn_conf->connect_attempts_max_retries;
-    }
-
+    unsigned max_connect_retries = s->txn_conf->connect_attempts_max_retries;
     TxnDbg(dbg_ctl_http_trans, "max_connect_retries: %d 
s->current.retry_attempts: %d", max_connect_retries,
            s->current.retry_attempts.get());
 
-    if (is_request_retryable(s) && s->current.retry_attempts.get() < 
max_connect_retries &&
-        !HttpTransact::is_response_valid(s, &s->hdr_info.server_response)) {
-      // If this is a round robin DNS entry & we're tried configured
-      //    number of times, we should try another node
-      if (ResolveInfo::OS_Addr::TRY_CLIENT == s->dns_info.os_addr_style) {
-        // attempt was based on client supplied server address. Try again 
using HostDB.
-        // Allow DNS attempt
-        s->dns_info.resolved_p = false;
-        // See if we can get data from HostDB for this.
-        s->dns_info.os_addr_style = ResolveInfo::OS_Addr::TRY_HOSTDB;
-        // Force host resolution to have the same family as the client.
-        // Because this is a transparent connection, we can't switch address
-        // families - that is locked in by the client source address.
-        ats_force_order_by_family(s->current.server->dst_addr.family(), 
s->my_txn_conf().host_res_data.order);
-        return CallOSDNSLookup(s);
-      } else if (ResolveInfo::OS_Addr::USE_API == s->dns_info.os_addr_style && 
!s->api_server_addr_set_retried) {
-        // Plugin set the server address via TSHttpTxnServerAddrSet(). Clear 
resolution
-        // state to allow the OS_DNS hook to be called again, giving the 
plugin a chance
-        // to set a different server address for retry (issue #12611).
-        // Only retry once to avoid infinite loops if the plugin keeps setting 
failing addresses.
-        s->api_server_addr_set_retried = true;
-        s->dns_info.resolved_p         = false;
-        s->dns_info.os_addr_style      = ResolveInfo::OS_Addr::TRY_DEFAULT;
-        // Clear the server request so it can be rebuilt for the new 
destination
-        s->hdr_info.server_request.destroy();
-        TxnDbg(dbg_ctl_http_trans, "Retrying with plugin-set address, 
returning to OS_DNS hook");
-        return CallOSDNSLookup(s);
-      } else {
-        if ((s->txn_conf->connect_attempts_rr_retries > 0) &&
-            ((s->current.retry_attempts.get() + 1) % 
s->txn_conf->connect_attempts_rr_retries == 0)) {
-          s->dns_info.select_next_rr();
-        }
-        retry_server_connection_not_open(s, s->current.state, 
max_connect_retries);
-        TxnDbg(dbg_ctl_http_trans, "Error. Retrying...");
-        s->next_action = how_to_open_connection(s);
-      }
-    } else {
+    // Bail out if the request is not retryable, the global retry cap is 
reached, or we already have a usable response.
+    if (!is_request_retryable(s) || s->current.retry_attempts.get() >= 
max_connect_retries ||
+        HttpTransact::is_response_valid(s, &s->hdr_info.server_response)) {
+      TxnDbg(dbg_ctl_http_trans, "Error. No more retries. %d/%d", 
s->current.retry_attempts.get(), max_connect_retries);

Review Comment:
   The retry cap here is initialized from connect_attempts_max_retries 
unconditionally, but the PR description says retry limits should be state-aware 
(UP/SUSPECT/DOWN). Using the UP config for the early bailout check can allow 
extra retries (or plugin/client-address re-resolution retries) beyond the 
intended per-host cap when the active HostDBInfo is SUSPECT/DOWN. Consider 
initializing max_connect_retries using 
origin_server_connect_attempts_max_retries(s) (with a deliberate fallback for 
non-HostDB targets, if needed).



##########
src/proxy/http/HttpConfig.cc:
##########
@@ -1348,6 +1348,10 @@ HttpConfig::reconfigure()
             "will never redispatch to another server",
             m_master.oride.connect_attempts_rr_retries, 
params->oride.connect_attempts_max_retries);
   }
+  if (m_master.oride.connect_attempts_rr_retries > 0 && 
params->oride.connect_attempts_max_retries_down_server == 0) {
+    Warning("connect_attempts_max_retries_down_server=0 with round-robin 
enabled skips probing recovering (SUSPECT) origins; "
+            "set connect_attempts_max_retries_down_server >= 1 is 
recommended");

Review Comment:
   This Warning message is misleading: 
connect_attempts_max_retries_down_server=0 doesn’t inherently “skip probing” 
SUSPECT origins; it mainly reduces the retry budget (potentially to just a 
single probe attempt). Also the phrasing is grammatically off (“set … is 
recommended”) and it would be clearer to reference the full record name 
(proxy.config.http.connect_attempts_max_retries_down_server). Please update the 
message (and/or the logic) so it accurately reflects the actual behavior.
   ```suggestion
       Warning("proxy.config.http.connect_attempts_max_retries_down_server=0 
with round-robin enabled leaves no retry budget "
               "for down or recovering (SUSPECT) origins beyond the initial 
attempt; setting "
               "proxy.config.http.connect_attempts_max_retries_down_server >= 1 
is recommended");
   ```



##########
tests/gold_tests/dns/replay/connect_attempts_single_max_retries.replay.yaml:
##########
@@ -0,0 +1,134 @@
+#  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.
+
+meta:
+  version: "1.0"
+
+# Configuration section for autest integration
+autest:
+  description: 'Verify connect attempts behavior - no round-robin (single 
host)'
+
+  dns:
+    name: 'dns-single-retries'
+    records: {"backend.example.com": ["0.0.0.1"]}
+
+  server:
+    name: 'server-single-retries'
+
+  client:
+    name: 'client-single-retries'
+    process_config:
+      poll_timeout: 10000 # ms
+
+  ats:
+    name: 'ts-single-retries'
+    process_config:
+      enable_cache: false
+
+    records_config:
+      proxy.config.diags.debug.enabled: 1
+      proxy.config.diags.debug.tags: 'http|hostdb|dns'
+      proxy.config.http.connect_attempts_rr_retries: 99 # make sure this 
doesn't affect this case
+      proxy.config.http.connect_attempts_max_retries: 2
+      proxy.config.http.connect_attempts_max_retries_down_server: 1
+      proxy.config.http.connect_attempts_timeout: 1
+      proxy.config.http.down_server.cache_time: 3
+
+    remap_config:
+      - from: "http://example.com/";
+        to: "http://backend.example.com:{SERVER_HTTP_PORT}/";
+
+    log_validation:
+      error_log:
+        gold_file: "gold/connect_attempts_single_max_retries_error_log.gold"
+
+sessions:
+- transactions:
+  # First request: ATS attempts 3 times (1 initial + 2 reties) to connect to 
0.0.0.1.
+  # The server is marked as down and ATS returns 502.
+  - client-request:
+      method: GET
+      url: /path/
+      version: '1.1'
+      headers:
+        fields:
+        - [Host, example.com]
+        - [uuid, 1]
+
+    # should not hit
+    server-response:
+      status: 200
+      reason: OK
+
+    proxy-response:
+      status: 502
+
+  # Second request: within down_server.cache_time=3s so the server is still 
marked down.
+  # ATS skips the connection attempt entirely and returns 500.
+  - client-request:
+      method: GET
+      url: /path/
+      version: '1.1'
+      headers:
+        fields:
+        - [Host, example.com]
+        - [uuid, 2]
+
+    # should not hit
+    server-response:
+      status: 200
+      reason: OK
+
+    proxy-response:
+      status: 500
+
+  # Third request: delayed 5s so the down_server.cache_time=3s has expired.
+  # ATS attempts 2 times (1 initial + 1 reties) to connect to 0.0.0.1.
+  # The server is marked as down again and ATS returns 502.
+  - client-request:
+      method: GET
+      url: /path/
+      version: '1.1'
+      headers:
+        fields:
+        - [Host, example.com]
+        - [uuid, 3]
+      delay: 5s
+
+    # should not hit
+    server-response:
+      status: 200
+      reason: OK
+
+    proxy-response:
+      status: 502
+
+  # Forth request: within down_server.cache_time=3s so the server is still 
marked down.

Review Comment:
   Typo in comment: “Forth request” → “Fourth request”.
   ```suggestion
     # Fourth request: within down_server.cache_time=3s so the server is still 
marked down.
   ```



##########
src/proxy/http/HttpTransact.cc:
##########
@@ -562,6 +562,41 @@ HttpTransact::is_server_negative_cached(State *s)
   }
 }
 
+/**
+  ATS has two configuration options controlling how many times it retries a 
connection attempt against origin servers.
+
+  - proxy.config.http.connect_attempts_max_retries
+  - proxy.config.http.connect_attempts_max_retries_down_server
+
+  The choice is based on the state of the active HostDBInfo.
+
+  - HostDBInfo::State::UP: use proxy.config.http.connect_attempts_max_retries
+  - HostDBInfo::State::DOWN: no retry
+  - HostDBInfo::State::SUSPECT: use 
proxy.config.http.connect_attempts_max_retries_down_server
+
+*/
+uint8_t
+HttpTransact::origin_server_connect_attempts_max_retries(State *s)
+{
+  HostDBInfo *active = s->dns_info.active;
+  if (active == nullptr) {
+    return 0;
+  }
+
+  switch (active->state(ts_clock::now(), s->txn_conf->down_server_timeout)) {
+  case HostDBInfo::State::UP:
+    return s->txn_conf->connect_attempts_max_retries;
+  case HostDBInfo::State::DOWN:
+    return 0;
+  case HostDBInfo::State::SUSPECT:
+    return s->txn_conf->connect_attempts_max_retries_down_server;
+  default:

Review Comment:
   origin_server_connect_attempts_max_retries() returns uint8_t, but the 
underlying configs (txn_conf->connect_attempts_max_retries* are MgmtInt). 
Implicitly converting can truncate/wrap values >255, changing retry behavior 
silently. Consider clamping to [0, UINT8_MAX] (and optionally emitting a 
Warning when clamped) or changing the helper to return a wider type and only 
clamp at the HostDBInfo API boundary.



##########
src/proxy/http/HttpSM.cc:
##########
@@ -5972,34 +5968,35 @@ HttpSM::do_transform_open()
 void
 HttpSM::mark_host_failure(ResolveInfo *info, ts_time time_down)
 {
-  char addrbuf[INET6_ADDRPORTSTRLEN];
+  ink_assert(time_down != TS_TIME_ZERO);
 
-  if (info->active) {
-    if (time_down != TS_TIME_ZERO) {
-      ats_ip_nptop(&t_state.current.server->dst_addr.sa, addrbuf, 
sizeof(addrbuf));
-      // Increment the fail_count
-      if (auto [down, fail_count] = 
info->active->increment_fail_count(time_down, 
t_state.txn_conf->connect_attempts_rr_retries,
-                                                                       
t_state.txn_conf->down_server_timeout);
-          down) {
-        char            *url_str = 
t_state.hdr_info.client_request.url_string_get_ref(nullptr);
-        std::string_view host_name{t_state.unmapped_url.host_get()};
-        swoc::bwprint(error_bw_buffer, "CONNECT : {::s} connecting to {} for 
host='{}' url='{}' fail_count='{}' marking down",
-                      
swoc::bwf::Errno(t_state.current.server->connect_result), 
t_state.current.server->dst_addr, host_name,
-                      swoc::bwf::FirstOf(url_str, "<none>"), fail_count);
-        Log::error("%s", error_bw_buffer.c_str());
-        SMDbg(dbg_ctl_http, "hostdb update marking IP: %s as down", addrbuf);
-        ATS_PROBE2(hostdb_mark_ip_as_down, sm_id, addrbuf);
-      } else {
-        ATS_PROBE3(hostdb_inc_ip_failcount, sm_id, addrbuf, fail_count);
-        SMDbg(dbg_ctl_http, "hostdb increment IP failcount %s to %d", addrbuf, 
fail_count);
-      }
-    } else { // Clear the failure
-      info->active->mark_up();
-    }
+  if (info->active == nullptr) {
+    return;
+  }
+
+  char addrbuf[INET6_ADDRPORTSTRLEN];
+  ats_ip_nptop(&t_state.current.server->dst_addr.sa, addrbuf, sizeof(addrbuf));
+
+  uint8_t    max_connect_retries = 
HttpTransact::origin_server_connect_attempts_max_retries(&t_state);
+  ts_seconds fail_window         = t_state.txn_conf->down_server_timeout;
+
+  // Mark the host DOWN only after every attempt has failed. 
`max_connect_retries` counts only "retries", so the total attempt
+  // budget is `max_connect_retries + 1` (the initial connect plus each retry).
+  auto [down, fail_count] = info->active->increment_fail_count(time_down, 
max_connect_retries + 1, fail_window);
+

Review Comment:
   max_connect_retries is a uint8_t and is used as `max_connect_retries + 1` 
for the attempt budget. If max_connect_retries is 255 (or has already wrapped 
due to narrowing), `+ 1` overflows back to 0, which would make the 
down-threshold computation incorrect. Consider computing the attempt budget in 
a wider integer type and saturating/clamping to UINT8_MAX before passing it to 
increment_fail_count().



##########
src/proxy/http/HttpTransact.cc:
##########
@@ -3970,63 +4007,92 @@ HttpTransact::handle_response_from_server(State *s)
       }
     }
 
-    if (is_server_negative_cached(s)) {
-      max_connect_retries = 
s->txn_conf->connect_attempts_max_retries_down_server - 1;
-    } else {
-      // server not yet negative cached - use default number of retries
-      max_connect_retries = s->txn_conf->connect_attempts_max_retries;
-    }
-
+    unsigned max_connect_retries = s->txn_conf->connect_attempts_max_retries;
     TxnDbg(dbg_ctl_http_trans, "max_connect_retries: %d 
s->current.retry_attempts: %d", max_connect_retries,
            s->current.retry_attempts.get());
 
-    if (is_request_retryable(s) && s->current.retry_attempts.get() < 
max_connect_retries &&
-        !HttpTransact::is_response_valid(s, &s->hdr_info.server_response)) {
-      // If this is a round robin DNS entry & we're tried configured
-      //    number of times, we should try another node
-      if (ResolveInfo::OS_Addr::TRY_CLIENT == s->dns_info.os_addr_style) {
-        // attempt was based on client supplied server address. Try again 
using HostDB.
-        // Allow DNS attempt
-        s->dns_info.resolved_p = false;
-        // See if we can get data from HostDB for this.
-        s->dns_info.os_addr_style = ResolveInfo::OS_Addr::TRY_HOSTDB;
-        // Force host resolution to have the same family as the client.
-        // Because this is a transparent connection, we can't switch address
-        // families - that is locked in by the client source address.
-        ats_force_order_by_family(s->current.server->dst_addr.family(), 
s->my_txn_conf().host_res_data.order);
-        return CallOSDNSLookup(s);
-      } else if (ResolveInfo::OS_Addr::USE_API == s->dns_info.os_addr_style && 
!s->api_server_addr_set_retried) {
-        // Plugin set the server address via TSHttpTxnServerAddrSet(). Clear 
resolution
-        // state to allow the OS_DNS hook to be called again, giving the 
plugin a chance
-        // to set a different server address for retry (issue #12611).
-        // Only retry once to avoid infinite loops if the plugin keeps setting 
failing addresses.
-        s->api_server_addr_set_retried = true;
-        s->dns_info.resolved_p         = false;
-        s->dns_info.os_addr_style      = ResolveInfo::OS_Addr::TRY_DEFAULT;
-        // Clear the server request so it can be rebuilt for the new 
destination
-        s->hdr_info.server_request.destroy();
-        TxnDbg(dbg_ctl_http_trans, "Retrying with plugin-set address, 
returning to OS_DNS hook");
-        return CallOSDNSLookup(s);
-      } else {
-        if ((s->txn_conf->connect_attempts_rr_retries > 0) &&
-            ((s->current.retry_attempts.get() + 1) % 
s->txn_conf->connect_attempts_rr_retries == 0)) {
-          s->dns_info.select_next_rr();
-        }
-        retry_server_connection_not_open(s, s->current.state, 
max_connect_retries);
-        TxnDbg(dbg_ctl_http_trans, "Error. Retrying...");
-        s->next_action = how_to_open_connection(s);
-      }
-    } else {
+    // Bail out if the request is not retryable, the global retry cap is 
reached, or we already have a usable response.
+    if (!is_request_retryable(s) || s->current.retry_attempts.get() >= 
max_connect_retries ||
+        HttpTransact::is_response_valid(s, &s->hdr_info.server_response)) {
+      TxnDbg(dbg_ctl_http_trans, "Error. No more retries. %d/%d", 
s->current.retry_attempts.get(), max_connect_retries);
+      SET_VIA_STRING(VIA_DETAIL_SERVER_CONNECT, VIA_DETAIL_SERVER_FAILURE);
       error_log_connection_failure(s, s->current.state);
-      TxnDbg(dbg_ctl_http_trans, "Error. No more retries.");
+      s->state_machine->do_hostdb_update_if_necessary();
+      handle_server_connection_not_open(s);
+      break;
+    }
+
+    // Attempt was based on a client-supplied address. Re-resolve via HostDB.
+    if (ResolveInfo::OS_Addr::TRY_CLIENT == s->dns_info.os_addr_style) {
+      // Allow DNS attempt
+      s->dns_info.resolved_p = false;
+      // See if we can get data from HostDB for this.
+      s->dns_info.os_addr_style = ResolveInfo::OS_Addr::TRY_HOSTDB;
+      // Force host resolution to have the same family as the client.
+      // Because this is a transparent connection, we can't switch address
+      // families - that is locked in by the client source address.
+      ats_force_order_by_family(s->current.server->dst_addr.family(), 
s->my_txn_conf().host_res_data.order);
+      return CallOSDNSLookup(s);
+    }
+
+    // Plugin set the server address via TSHttpTxnServerAddrSet(). Clear 
resolution
+    // state to allow the OS_DNS hook to be called again, giving the plugin a 
chance
+    // to set a different server address for retry (issue #12611).
+    // Only retry once to avoid infinite loops if the plugin keeps setting 
failing addresses.
+    if (ResolveInfo::OS_Addr::USE_API == s->dns_info.os_addr_style && 
!s->api_server_addr_set_retried) {
+      s->api_server_addr_set_retried = true;
+      s->dns_info.resolved_p         = false;
+      s->dns_info.os_addr_style      = ResolveInfo::OS_Addr::TRY_DEFAULT;
+      // Clear the server request so it can be rebuilt for the new destination
+      s->hdr_info.server_request.destroy();
+      TxnDbg(dbg_ctl_http_trans, "Retrying with plugin-set address, returning 
to OS_DNS hook");
+      return CallOSDNSLookup(s);
+    }
+
+    // Record the failure on the current active target.
+    error_log_connection_failure(s, s->current.state);
+    s->state_machine->do_hostdb_update_if_necessary();
+
+    // Decide between switching to the next round-robin member or staying on 
the same target.
+    if ((s->txn_conf->connect_attempts_rr_retries > 0) &&
+        ((s->current.retry_attempts.get() + 1) % 
s->txn_conf->connect_attempts_rr_retries == 0)) {
+      if (!s->dns_info.select_next_rr(ts_clock::now(), 
s->txn_conf->down_server_timeout)) {
+        TxnDbg(dbg_ctl_http_trans, "No round-robin targets available. Giving 
up");
+        SET_VIA_STRING(VIA_DETAIL_SERVER_CONNECT, VIA_DETAIL_SERVER_FAILURE);
+        handle_server_connection_not_open(s);
+        break;
+      }
+
+      // select_next_rr() only updates dns_info.active; change the dst_addr 
too.
+      s->dns_info.addr.assign(s->dns_info.active->data.ip);
+      s->server_info.dst_addr.assign(s->dns_info.active->data.ip, 
s->server_info.dst_addr.network_order_port());
+      if (dbg_ctl_http_trans.on()) {
+        ip_port_text_buffer addrbuf;
+        TxnDbg(dbg_ctl_http_trans, "switched to next round-robin upstream 
addr=%s",
+               ats_ip_nptop(&s->server_info.dst_addr.sa, addrbuf, 
sizeof(addrbuf)));
+      }

Review Comment:
   When connect_attempts_rr_retries triggers a round-robin switch, this treats 
select_next_rr()==false as “no targets available” and gives up. But 
select_next_rr() can also return false simply because there is no *alternate* 
non-DOWN target (e.g., other RR members are DOWN) even though retrying the 
current target is still allowed. This can prematurely stop retries. Consider 
continuing with the current active target when select_next_rr() returns false, 
and only giving up when the overall retry budget is exhausted / there are truly 
no viable targets.
   ```suggestion
         if (s->dns_info.select_next_rr(ts_clock::now(), 
s->txn_conf->down_server_timeout)) {
           // select_next_rr() only updates dns_info.active; change the 
dst_addr too.
           s->dns_info.addr.assign(s->dns_info.active->data.ip);
           s->server_info.dst_addr.assign(s->dns_info.active->data.ip, 
s->server_info.dst_addr.network_order_port());
           if (dbg_ctl_http_trans.on()) {
             ip_port_text_buffer addrbuf;
             TxnDbg(dbg_ctl_http_trans, "switched to next round-robin upstream 
addr=%s",
                    ats_ip_nptop(&s->server_info.dst_addr.sa, addrbuf, 
sizeof(addrbuf)));
           }
         } else {
           TxnDbg(dbg_ctl_http_trans, "No alternate round-robin target 
available, retrying current upstream");
         }
   ```



##########
tests/gold_tests/dns/replay/connect_attempts_single_max_retries.replay.yaml:
##########
@@ -0,0 +1,134 @@
+#  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.
+
+meta:
+  version: "1.0"
+
+# Configuration section for autest integration
+autest:
+  description: 'Verify connect attempts behavior - no round-robin (single 
host)'
+
+  dns:
+    name: 'dns-single-retries'
+    records: {"backend.example.com": ["0.0.0.1"]}
+
+  server:
+    name: 'server-single-retries'
+
+  client:
+    name: 'client-single-retries'
+    process_config:
+      poll_timeout: 10000 # ms
+
+  ats:
+    name: 'ts-single-retries'
+    process_config:
+      enable_cache: false
+
+    records_config:
+      proxy.config.diags.debug.enabled: 1
+      proxy.config.diags.debug.tags: 'http|hostdb|dns'
+      proxy.config.http.connect_attempts_rr_retries: 99 # make sure this 
doesn't affect this case
+      proxy.config.http.connect_attempts_max_retries: 2
+      proxy.config.http.connect_attempts_max_retries_down_server: 1
+      proxy.config.http.connect_attempts_timeout: 1
+      proxy.config.http.down_server.cache_time: 3
+
+    remap_config:
+      - from: "http://example.com/";
+        to: "http://backend.example.com:{SERVER_HTTP_PORT}/";
+
+    log_validation:
+      error_log:
+        gold_file: "gold/connect_attempts_single_max_retries_error_log.gold"
+
+sessions:
+- transactions:
+  # First request: ATS attempts 3 times (1 initial + 2 reties) to connect to 
0.0.0.1.
+  # The server is marked as down and ATS returns 502.

Review Comment:
   Typo in comment: “reties” → “retries”.



##########
tests/gold_tests/dns/replay/connect_attempts_single_max_retries.replay.yaml:
##########
@@ -0,0 +1,134 @@
+#  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.
+
+meta:
+  version: "1.0"
+
+# Configuration section for autest integration
+autest:
+  description: 'Verify connect attempts behavior - no round-robin (single 
host)'
+
+  dns:
+    name: 'dns-single-retries'
+    records: {"backend.example.com": ["0.0.0.1"]}
+
+  server:
+    name: 'server-single-retries'
+
+  client:
+    name: 'client-single-retries'
+    process_config:
+      poll_timeout: 10000 # ms
+
+  ats:
+    name: 'ts-single-retries'
+    process_config:
+      enable_cache: false
+
+    records_config:
+      proxy.config.diags.debug.enabled: 1
+      proxy.config.diags.debug.tags: 'http|hostdb|dns'
+      proxy.config.http.connect_attempts_rr_retries: 99 # make sure this 
doesn't affect this case
+      proxy.config.http.connect_attempts_max_retries: 2
+      proxy.config.http.connect_attempts_max_retries_down_server: 1
+      proxy.config.http.connect_attempts_timeout: 1
+      proxy.config.http.down_server.cache_time: 3
+
+    remap_config:
+      - from: "http://example.com/";
+        to: "http://backend.example.com:{SERVER_HTTP_PORT}/";
+
+    log_validation:
+      error_log:
+        gold_file: "gold/connect_attempts_single_max_retries_error_log.gold"
+
+sessions:
+- transactions:
+  # First request: ATS attempts 3 times (1 initial + 2 reties) to connect to 
0.0.0.1.
+  # The server is marked as down and ATS returns 502.
+  - client-request:
+      method: GET
+      url: /path/
+      version: '1.1'
+      headers:
+        fields:
+        - [Host, example.com]
+        - [uuid, 1]
+
+    # should not hit
+    server-response:
+      status: 200
+      reason: OK
+
+    proxy-response:
+      status: 502
+
+  # Second request: within down_server.cache_time=3s so the server is still 
marked down.
+  # ATS skips the connection attempt entirely and returns 500.
+  - client-request:
+      method: GET
+      url: /path/
+      version: '1.1'
+      headers:
+        fields:
+        - [Host, example.com]
+        - [uuid, 2]
+
+    # should not hit
+    server-response:
+      status: 200
+      reason: OK
+
+    proxy-response:
+      status: 500
+
+  # Third request: delayed 5s so the down_server.cache_time=3s has expired.
+  # ATS attempts 2 times (1 initial + 1 reties) to connect to 0.0.0.1.
+  # The server is marked as down again and ATS returns 502.

Review Comment:
   Typo in comment: “reties” → “retries”.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to