masaori335 commented on code in PR #13102:
URL: https://github.com/apache/trafficserver/pull/13102#discussion_r3165828144
##########
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:
seems resonable.
--
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]
