rob05c commented on a change in pull request #7874:
URL: https://github.com/apache/trafficserver/pull/7874#discussion_r669198172
##########
File path: proxy/http/HttpSM.cc
##########
@@ -7446,65 +7372,48 @@ HttpSM::set_next_state()
}
case HttpTransact::SM_ACTION_DNS_LOOKUP: {
- sockaddr const *addr;
-
- if (t_state.api_server_addr_set) {
- /* If the API has set the server address before the OS DNS lookup
- * then we can skip the lookup
- */
- ip_text_buffer ipb;
- SMDebug("dns", "[HttpTransact::HandleRequest] Skipping DNS lookup for
API supplied target %s.",
- ats_ip_ntop(&t_state.server_info.dst_addr, ipb, sizeof(ipb)));
- // this seems wasteful as we will just copy it right back
- ats_ip_copy(t_state.host_db_info.ip(), &t_state.server_info.dst_addr);
- t_state.dns_info.lookup_success = true;
- call_transact_and_set_next_state(nullptr);
- break;
- } else if (0 == ats_ip_pton(t_state.dns_info.lookup_name,
t_state.host_db_info.ip()) &&
- ats_is_ip_loopback(t_state.host_db_info.ip())) {
- // If it's 127.0.0.1 or ::1 don't bother with hostdb
- SMDebug("dns", "[HttpTransact::HandleRequest] Skipping DNS lookup for %s
because it's loopback",
- t_state.dns_info.lookup_name);
- t_state.dns_info.lookup_success = true;
- call_transact_and_set_next_state(nullptr);
- break;
- } else if (t_state.http_config_param->use_client_target_addr == 2 &&
!t_state.url_remap_success &&
- t_state.parent_result.result != PARENT_SPECIFIED &&
t_state.client_info.is_transparent &&
- t_state.dns_info.os_addr_style ==
HttpTransact::DNSLookupInfo::OS_Addr::OS_ADDR_TRY_DEFAULT &&
- ats_is_ip(addr = ua_txn->get_netvc()->get_local_addr())) {
- /* If the connection is client side transparent and the URL
- * was not remapped/directed to parent proxy, we can use the
- * client destination IP address instead of doing a DNS
- * lookup. This is controlled by the 'use_client_target_addr'
- * configuration parameter.
+ if (sockaddr const *addr;
t_state.http_config_param->use_client_target_addr == 2 && // no
CTA verification
+ !t_state.url_remap_success &&
// wasn't remapped
+ t_state.parent_result.result != PARENT_SPECIFIED
&& // no parent.
+ t_state.client_info.is_transparent &&
// inbound transparent
+ t_state.dns_info.os_addr_style ==
ResolveInfo::OS_Addr::TRY_DEFAULT && // haven't tried anything yet.
+ ats_is_ip(addr =
ua_txn->get_netvc()->get_local_addr())) // valid inbound remote
address
+ {
+ /* If the connection is client side transparent and the URL was not
remapped/directed to
+ * parent proxy, we can use the client destination IP address instead of
doing a DNS lookup.
+ * This is controlled by the 'use_client_target_addr' configuration
parameter.
*/
if (is_debug_tag_set("dns")) {
ip_text_buffer ipb;
SMDebug("dns", "[HttpTransact::HandleRequest] Skipping DNS lookup for
client supplied target %s.",
ats_ip_ntop(addr, ipb, sizeof(ipb)));
}
- ats_ip_copy(t_state.host_db_info.ip(), addr);
- t_state.host_db_info.app.http_data.http_version =
t_state.hdr_info.client_request.version_get();
- t_state.dns_info.lookup_success = true;
- // cache this result so we don't have to unreliably duplicate the
- // logic later if the connect fails.
- t_state.dns_info.os_addr_style =
HttpTransact::DNSLookupInfo::OS_Addr::OS_ADDR_TRY_CLIENT;
+ t_state.dns_info.set_upstream_address(addr);
+
+ // Make a note the CTA is being used - don't do this case again.
+ t_state.dns_info.os_addr_style = ResolveInfo::OS_Addr::TRY_CLIENT;
+
+ if (t_state.hdr_info.client_request.version_get() == HTTPVersion(0, 9)) {
+ t_state.dns_info.http_version = HTTP_0_9;
+ } else if (t_state.hdr_info.client_request.version_get() ==
HTTPVersion(1, 0)) {
+ t_state.dns_info.http_version = HTTP_1_0;
+ } else {
+ t_state.dns_info.http_version = HTTP_1_1;
+ }
+
call_transact_and_set_next_state(nullptr);
break;
- } else if (t_state.parent_result.result == PARENT_UNDEFINED &&
t_state.dns_info.lookup_success) {
- // Already set, and we don't have a parent proxy to lookup
- ink_assert(ats_is_ip(t_state.host_db_info.ip()));
- SMDebug("dns", "[HttpTransact::HandleRequest] Skipping DNS lookup,
provided by plugin");
+ } else if (t_state.dns_info.looking_up == ResolveInfo::ORIGIN_SERVER &&
t_state.http_config_param->no_dns_forward_to_parent &&
+ t_state.parent_result.result != PARENT_UNDEFINED) {
+ t_state.dns_info.resolved_p = true; // seems dangerous - where's the IP
address?
Review comment:
Can you explain "dangerous?" Is this an actual bug? Or only if anything
ever gets dns_info's address without checking existence?
##########
File path: iocore/hostdb/I_HostDBProcessor.h
##########
@@ -84,350 +89,537 @@ makeHostHash(const char *string)
// Types
//
-/** Host information metadata used by various parts of HostDB.
- * It is stored as generic data in the cache.
- *
- * As a @c union only one of the members is valid, Which one depends on
context data in the
- * @c HostDBInfo. This data is written literally to disk therefore if any
change is made,
- * the @c object_version for the cache must be updated by modifying @c
HostDBInfo::version.
- *
- * @see HostDBInfo::version
- */
-union HostDBApplicationInfo {
- /// Generic storage. This is verified to be the size of the union.
- struct application_data_allotment {
- unsigned int application1;
- unsigned int application2;
- } allotment;
-
- //////////////////////////////////////////////////////////
- // http server attributes in the host database //
- // //
- // http_version - one of HTTPVersion //
- // last_failure - UNIX time for the last time //
- // we tried the server & failed //
- // fail_count - Number of times we tried and //
- // and failed to contact the host //
- //////////////////////////////////////////////////////////
- struct http_server_attr {
- uint32_t last_failure;
- HTTPVersion http_version;
- uint8_t fail_count;
- http_server_attr() : http_version() {}
- } http_data;
-
- struct application_data_rr {
- unsigned int offset;
- } rr;
- HostDBApplicationInfo() : http_data() {}
-};
-
-struct HostDBRoundRobin;
+class HostDBRecord;
+/// Information for an SRV record.
struct SRVInfo {
- unsigned int srv_offset : 16;
+ unsigned int srv_offset : 16; ///< Memory offset from @c HostDBInfo to name.
unsigned int srv_weight : 16;
unsigned int srv_priority : 16;
unsigned int srv_port : 16;
unsigned int key;
};
-struct HostDBInfo : public RefCountObj {
- /** Internal IP address data.
- This is at least large enough to hold an IPv6 address.
- */
+/// Type of data stored.
+enum class HostDBType : uint8_t {
+ UNSPEC, ///< No valid data.
+ ADDR, ///< IP address.
+ SRV, ///< SRV record.
+ HOST ///< Hostname (reverse DNS)
+};
+char const *name_of(HostDBType t);
- static HostDBInfo *
- alloc(int size = 0)
- {
- size += sizeof(HostDBInfo);
- int iobuffer_index = iobuffer_size_to_index(size, hostdb_max_iobuf_index);
- ink_release_assert(iobuffer_index >= 0);
- void *ptr = ioBufAllocator[iobuffer_index].alloc_void();
- memset(ptr, 0, size);
- HostDBInfo *ret = new (ptr) HostDBInfo();
- ret->_iobuffer_index = iobuffer_index;
- return ret;
- }
+/** Information about a single target.
+ */
+struct HostDBInfo {
+ using self_type = HostDBInfo; ///< Self reference type.
- void
- free() override
- {
- ink_release_assert(from_alloc());
- Debug("hostdb", "freeing %d bytes at [%p]", (1 << (7 + _iobuffer_index)),
this);
- ioBufAllocator[_iobuffer_index].free_void((void *)(this));
- }
+ /// Default constructor.
+ HostDBInfo() = default;
- /// Effectively the @c object_version for cache data.
- /// This is used to indicate incompatible changes in the binary layout of
HostDB records.
- /// It must be updated if any such change is made, even if it is
functionally equivalent.
- static ts::VersionNumber
- version()
- {
- /// - 1.0 Initial version.
- /// - 1.1 tweak HostDBApplicationInfo::http_data.
- return ts::VersionNumber(1, 1);
- }
+ HostDBInfo &operator=(HostDBInfo const &that);
- static HostDBInfo *
- unmarshall(char *buf, unsigned int size)
- {
- if (size < sizeof(HostDBInfo)) {
- return nullptr;
- }
- HostDBInfo *ret = HostDBInfo::alloc(size - sizeof(HostDBInfo));
- int buf_index = ret->_iobuffer_index;
- memcpy((void *)ret, buf, size);
- // Reset the refcount back to 0, this is a bit ugly-- but I'm not sure we
want to expose a method
- // to mess with the refcount, since this is a fairly unique use case
- ret = new (ret) HostDBInfo();
- ret->_iobuffer_index = buf_index;
- return ret;
- }
+ /// Absolute time of when this target failed.
+ /// A value of zero (@c TS_TIME_ZERO ) indicates no failure.
+ ts_time last_fail_time() const;
- // return expiry time (in seconds since epoch)
- ink_time_t
- expiry_time() const
- {
- return ip_timestamp + ip_timeout_interval +
hostdb_serve_stale_but_revalidate;
- }
+ /// Target is alive - no known failure.
+ bool is_alive();
- sockaddr *
- ip()
- {
- return &data.ip.sa;
- }
+ /// Target has failed and is still in the blocked time window.
+ bool is_dead(ts_time now, ts_seconds fail_window);
- sockaddr const *
- ip() const
- {
- return &data.ip.sa;
- }
+ /** Select this target.
+ *
+ * @param now Current time.
+ * @param fail_window Failure window.
+ * @return Status of the selection.
+ *
+ * If a zombie is selected the failure time is updated to make it look dead
to other threads in a thread safe
+ * manner. The caller should check @c last_fail_time to see if a zombie was
selected.
+ */
+ bool select(ts_time now, ts_seconds fail_window);
- char *hostname() const;
- char *perm_hostname() const;
- char *srvname(HostDBRoundRobin *rr) const;
+ /// Check if this info is valid.
+ bool is_valid() const;
- /// Check if this entry is an element of a round robin entry.
- /// If @c true then this entry is part of and was obtained from a round
robin root. This is useful if the
- /// address doesn't work - a retry can probably get a new address by doing
another lookup and resolving to
- /// a different element of the round robin.
- bool
- is_rr_elt() const
- {
- return 0 != round_robin_elt;
- }
+ /// Mark this info as invalid.
+ void invalidate();
- HostDBRoundRobin *rr();
+ /** Mark the entry as down.
+ *
+ * @param now Time of the failure.
+ * @return @c true if @a this was marked down, @c false if not.
+ *
+ * This can return @c false if the entry is already marked down, in which
case the failure time is not updated.
+ */
+ bool mark_down(ts_time now);
- unsigned int
- ip_interval() const
- {
- return (hostdb_current_interval - ip_timestamp) & 0x7FFFFFFF;
- }
+ /** Mark the target as up / alive.
+ *
+ * @return Previous alive state of the target.
+ */
+ bool mark_up();
- int
- ip_time_remaining() const
- {
- return static_cast<int>(ip_timeout_interval) -
static_cast<int>(this->ip_interval());
- }
+ char const *srvname() const;
- bool
- is_ip_stale() const
- {
- return ip_timeout_interval >= 2 * hostdb_ip_stale_interval &&
ip_interval() >= hostdb_ip_stale_interval;
- }
+ /** Migrate data after a DNS update.
+ *
+ * @param that Source item.
+ *
+ * This moves only specific state information, it is not a generic copy.
+ */
+ void migrate_from(self_type const &that);
- bool
- is_ip_timeout() const
- {
- return ip_interval() >= ip_timeout_interval;
- }
+ /// A target is either an IP address or an SRV record.
+ /// The type should be indicated by @c flags.f.is_srv;
+ union {
+ IpAddr ip; ///< IP address / port data.
+ SRVInfo srv; ///< SRV record.
+ } data{IpAddr{}};
+
+ /// Data that migrates after updated DNS records are processed.
+ /// @see migrate_from
+ /// @{
+ /// Last time a failure was recorded.
+ std::atomic<ts_time> last_failure{TS_TIME_ZERO};
+ /// Count of connection failures
+ std::atomic<uint8_t> fail_count{0};
+ /// Expected HTTP version of the target based on earlier transactions.
+ HTTPVersion http_version = HTTP_INVALID;
+ /// @}
+
+ self_type &assign(IpAddr const &addr);
+
+protected:
+ self_type &assign(sa_family_t af, void const *addr);
+ self_type &assign(SRV const *srv, char const *name);
+
+ HostDBType type = HostDBType::UNSPEC; ///< Invalid data.
+
+ friend HostDBContinuation;
+};
- bool
- is_ip_fail_timeout() const
- {
- return ip_interval() >= hostdb_ip_fail_timeout_interval;
+inline HostDBInfo &
+HostDBInfo::operator=(HostDBInfo const &that)
+{
+ if (this != &that) {
+ memcpy(static_cast<void *>(this), static_cast<const void *>(&that),
sizeof(*this));
}
+ return *this;
+}
- void
- refresh_ip()
- {
- ip_timestamp = hostdb_current_interval;
- }
+inline ts_time
+HostDBInfo::last_fail_time() const
+{
+ return last_failure;
+}
- bool
- serve_stale_but_revalidate() const
- {
- // the option is disabled
- if (hostdb_serve_stale_but_revalidate <= 0) {
- return false;
- }
+inline bool
+HostDBInfo::is_alive()
+{
+ return this->last_fail_time() == TS_TIME_ZERO;
+}
- // ip_timeout_interval == DNS TTL
- // hostdb_serve_stale_but_revalidate == number of seconds
- // ip_interval() is the number of seconds between now() and when the entry
was inserted
- if ((ip_timeout_interval + hostdb_serve_stale_but_revalidate) >
ip_interval()) {
- Debug("hostdb", "serving stale entry %d | %d | %d as requested by
config", ip_timeout_interval,
- hostdb_serve_stale_but_revalidate, ip_interval());
- return true;
- }
+inline bool
+HostDBInfo::is_dead(ts_time now, ts_seconds fail_window)
+{
+ auto last_fail = this->last_fail_time();
+ return (last_fail != TS_TIME_ZERO) && (last_fail + fail_window < now);
+}
+
+inline bool
+HostDBInfo::mark_up()
+{
+ auto t = last_failure.exchange(TS_TIME_ZERO);
+ return t != TS_TIME_ZERO;
+}
+
+inline bool
+HostDBInfo::mark_down(ts_time now)
+{
+ auto t0{TS_TIME_ZERO};
+ return last_failure.compare_exchange_strong(t0, now);
+}
- // otherwise, the entry is too old
- return false;
+inline bool
+HostDBInfo::select(ts_time now, ts_seconds fail_window)
+{
+ auto t0 = this->last_fail_time();
+ if (t0 == TS_TIME_ZERO) {
+ return true; // it's alive and so is valid for selection.
}
+ // Success means this is a zombie and this thread updated the failure time.
+ return (t0 + fail_window < now) && last_failure.compare_exchange_strong(t0,
now);
Review comment:
So this CAS will fail if another thread updated underneath us? What
should the caller do if it gets a failure?
##########
File path: proxy/http/HttpSM.cc
##########
@@ -7446,65 +7372,48 @@ HttpSM::set_next_state()
}
case HttpTransact::SM_ACTION_DNS_LOOKUP: {
- sockaddr const *addr;
-
- if (t_state.api_server_addr_set) {
- /* If the API has set the server address before the OS DNS lookup
- * then we can skip the lookup
- */
- ip_text_buffer ipb;
- SMDebug("dns", "[HttpTransact::HandleRequest] Skipping DNS lookup for
API supplied target %s.",
- ats_ip_ntop(&t_state.server_info.dst_addr, ipb, sizeof(ipb)));
- // this seems wasteful as we will just copy it right back
- ats_ip_copy(t_state.host_db_info.ip(), &t_state.server_info.dst_addr);
- t_state.dns_info.lookup_success = true;
- call_transact_and_set_next_state(nullptr);
- break;
- } else if (0 == ats_ip_pton(t_state.dns_info.lookup_name,
t_state.host_db_info.ip()) &&
- ats_is_ip_loopback(t_state.host_db_info.ip())) {
- // If it's 127.0.0.1 or ::1 don't bother with hostdb
- SMDebug("dns", "[HttpTransact::HandleRequest] Skipping DNS lookup for %s
because it's loopback",
- t_state.dns_info.lookup_name);
- t_state.dns_info.lookup_success = true;
- call_transact_and_set_next_state(nullptr);
- break;
- } else if (t_state.http_config_param->use_client_target_addr == 2 &&
!t_state.url_remap_success &&
- t_state.parent_result.result != PARENT_SPECIFIED &&
t_state.client_info.is_transparent &&
- t_state.dns_info.os_addr_style ==
HttpTransact::DNSLookupInfo::OS_Addr::OS_ADDR_TRY_DEFAULT &&
- ats_is_ip(addr = ua_txn->get_netvc()->get_local_addr())) {
- /* If the connection is client side transparent and the URL
- * was not remapped/directed to parent proxy, we can use the
- * client destination IP address instead of doing a DNS
- * lookup. This is controlled by the 'use_client_target_addr'
- * configuration parameter.
+ if (sockaddr const *addr;
t_state.http_config_param->use_client_target_addr == 2 && // no
CTA verification
+ !t_state.url_remap_success &&
// wasn't remapped
+ t_state.parent_result.result != PARENT_SPECIFIED
&& // no parent.
+ t_state.client_info.is_transparent &&
// inbound transparent
+ t_state.dns_info.os_addr_style ==
ResolveInfo::OS_Addr::TRY_DEFAULT && // haven't tried anything yet.
+ ats_is_ip(addr =
ua_txn->get_netvc()->get_local_addr())) // valid inbound remote
address
+ {
+ /* If the connection is client side transparent and the URL was not
remapped/directed to
+ * parent proxy, we can use the client destination IP address instead of
doing a DNS lookup.
+ * This is controlled by the 'use_client_target_addr' configuration
parameter.
*/
if (is_debug_tag_set("dns")) {
ip_text_buffer ipb;
SMDebug("dns", "[HttpTransact::HandleRequest] Skipping DNS lookup for
client supplied target %s.",
ats_ip_ntop(addr, ipb, sizeof(ipb)));
}
- ats_ip_copy(t_state.host_db_info.ip(), addr);
- t_state.host_db_info.app.http_data.http_version =
t_state.hdr_info.client_request.version_get();
- t_state.dns_info.lookup_success = true;
- // cache this result so we don't have to unreliably duplicate the
- // logic later if the connect fails.
- t_state.dns_info.os_addr_style =
HttpTransact::DNSLookupInfo::OS_Addr::OS_ADDR_TRY_CLIENT;
+ t_state.dns_info.set_upstream_address(addr);
+
+ // Make a note the CTA is being used - don't do this case again.
+ t_state.dns_info.os_addr_style = ResolveInfo::OS_Addr::TRY_CLIENT;
+
+ if (t_state.hdr_info.client_request.version_get() == HTTPVersion(0, 9)) {
+ t_state.dns_info.http_version = HTTP_0_9;
+ } else if (t_state.hdr_info.client_request.version_get() ==
HTTPVersion(1, 0)) {
+ t_state.dns_info.http_version = HTTP_1_0;
+ } else {
+ t_state.dns_info.http_version = HTTP_1_1;
Review comment:
Can this ever be H2? I don't really understand this, we're setting the
http_version of the DNS record to the version the client used? Shouldn't it be
the server?
##########
File path: iocore/hostdb/I_HostDBProcessor.h
##########
@@ -84,350 +89,537 @@ makeHostHash(const char *string)
// Types
//
-/** Host information metadata used by various parts of HostDB.
- * It is stored as generic data in the cache.
- *
- * As a @c union only one of the members is valid, Which one depends on
context data in the
- * @c HostDBInfo. This data is written literally to disk therefore if any
change is made,
- * the @c object_version for the cache must be updated by modifying @c
HostDBInfo::version.
- *
- * @see HostDBInfo::version
- */
-union HostDBApplicationInfo {
- /// Generic storage. This is verified to be the size of the union.
- struct application_data_allotment {
- unsigned int application1;
- unsigned int application2;
- } allotment;
-
- //////////////////////////////////////////////////////////
- // http server attributes in the host database //
- // //
- // http_version - one of HTTPVersion //
- // last_failure - UNIX time for the last time //
- // we tried the server & failed //
- // fail_count - Number of times we tried and //
- // and failed to contact the host //
- //////////////////////////////////////////////////////////
- struct http_server_attr {
- uint32_t last_failure;
- HTTPVersion http_version;
- uint8_t fail_count;
- http_server_attr() : http_version() {}
- } http_data;
-
- struct application_data_rr {
- unsigned int offset;
- } rr;
- HostDBApplicationInfo() : http_data() {}
-};
-
-struct HostDBRoundRobin;
+class HostDBRecord;
+/// Information for an SRV record.
struct SRVInfo {
- unsigned int srv_offset : 16;
+ unsigned int srv_offset : 16; ///< Memory offset from @c HostDBInfo to name.
unsigned int srv_weight : 16;
unsigned int srv_priority : 16;
unsigned int srv_port : 16;
unsigned int key;
};
-struct HostDBInfo : public RefCountObj {
- /** Internal IP address data.
- This is at least large enough to hold an IPv6 address.
- */
+/// Type of data stored.
+enum class HostDBType : uint8_t {
+ UNSPEC, ///< No valid data.
+ ADDR, ///< IP address.
+ SRV, ///< SRV record.
+ HOST ///< Hostname (reverse DNS)
+};
+char const *name_of(HostDBType t);
- static HostDBInfo *
- alloc(int size = 0)
- {
- size += sizeof(HostDBInfo);
- int iobuffer_index = iobuffer_size_to_index(size, hostdb_max_iobuf_index);
- ink_release_assert(iobuffer_index >= 0);
- void *ptr = ioBufAllocator[iobuffer_index].alloc_void();
- memset(ptr, 0, size);
- HostDBInfo *ret = new (ptr) HostDBInfo();
- ret->_iobuffer_index = iobuffer_index;
- return ret;
- }
+/** Information about a single target.
+ */
+struct HostDBInfo {
+ using self_type = HostDBInfo; ///< Self reference type.
- void
- free() override
- {
- ink_release_assert(from_alloc());
- Debug("hostdb", "freeing %d bytes at [%p]", (1 << (7 + _iobuffer_index)),
this);
- ioBufAllocator[_iobuffer_index].free_void((void *)(this));
- }
+ /// Default constructor.
+ HostDBInfo() = default;
- /// Effectively the @c object_version for cache data.
- /// This is used to indicate incompatible changes in the binary layout of
HostDB records.
- /// It must be updated if any such change is made, even if it is
functionally equivalent.
- static ts::VersionNumber
- version()
- {
- /// - 1.0 Initial version.
- /// - 1.1 tweak HostDBApplicationInfo::http_data.
- return ts::VersionNumber(1, 1);
- }
+ HostDBInfo &operator=(HostDBInfo const &that);
- static HostDBInfo *
- unmarshall(char *buf, unsigned int size)
- {
- if (size < sizeof(HostDBInfo)) {
- return nullptr;
- }
- HostDBInfo *ret = HostDBInfo::alloc(size - sizeof(HostDBInfo));
- int buf_index = ret->_iobuffer_index;
- memcpy((void *)ret, buf, size);
- // Reset the refcount back to 0, this is a bit ugly-- but I'm not sure we
want to expose a method
- // to mess with the refcount, since this is a fairly unique use case
- ret = new (ret) HostDBInfo();
- ret->_iobuffer_index = buf_index;
- return ret;
- }
+ /// Absolute time of when this target failed.
+ /// A value of zero (@c TS_TIME_ZERO ) indicates no failure.
+ ts_time last_fail_time() const;
- // return expiry time (in seconds since epoch)
- ink_time_t
- expiry_time() const
- {
- return ip_timestamp + ip_timeout_interval +
hostdb_serve_stale_but_revalidate;
- }
+ /// Target is alive - no known failure.
+ bool is_alive();
- sockaddr *
- ip()
- {
- return &data.ip.sa;
- }
+ /// Target has failed and is still in the blocked time window.
+ bool is_dead(ts_time now, ts_seconds fail_window);
- sockaddr const *
- ip() const
- {
- return &data.ip.sa;
- }
+ /** Select this target.
+ *
+ * @param now Current time.
+ * @param fail_window Failure window.
+ * @return Status of the selection.
+ *
+ * If a zombie is selected the failure time is updated to make it look dead
to other threads in a thread safe
+ * manner. The caller should check @c last_fail_time to see if a zombie was
selected.
+ */
+ bool select(ts_time now, ts_seconds fail_window);
- char *hostname() const;
- char *perm_hostname() const;
- char *srvname(HostDBRoundRobin *rr) const;
+ /// Check if this info is valid.
+ bool is_valid() const;
- /// Check if this entry is an element of a round robin entry.
- /// If @c true then this entry is part of and was obtained from a round
robin root. This is useful if the
- /// address doesn't work - a retry can probably get a new address by doing
another lookup and resolving to
- /// a different element of the round robin.
- bool
- is_rr_elt() const
- {
- return 0 != round_robin_elt;
- }
+ /// Mark this info as invalid.
+ void invalidate();
- HostDBRoundRobin *rr();
+ /** Mark the entry as down.
+ *
+ * @param now Time of the failure.
+ * @return @c true if @a this was marked down, @c false if not.
+ *
+ * This can return @c false if the entry is already marked down, in which
case the failure time is not updated.
+ */
+ bool mark_down(ts_time now);
- unsigned int
- ip_interval() const
- {
- return (hostdb_current_interval - ip_timestamp) & 0x7FFFFFFF;
- }
+ /** Mark the target as up / alive.
+ *
+ * @return Previous alive state of the target.
+ */
+ bool mark_up();
- int
- ip_time_remaining() const
- {
- return static_cast<int>(ip_timeout_interval) -
static_cast<int>(this->ip_interval());
- }
+ char const *srvname() const;
- bool
- is_ip_stale() const
- {
- return ip_timeout_interval >= 2 * hostdb_ip_stale_interval &&
ip_interval() >= hostdb_ip_stale_interval;
- }
+ /** Migrate data after a DNS update.
+ *
+ * @param that Source item.
+ *
+ * This moves only specific state information, it is not a generic copy.
+ */
+ void migrate_from(self_type const &that);
- bool
- is_ip_timeout() const
- {
- return ip_interval() >= ip_timeout_interval;
- }
+ /// A target is either an IP address or an SRV record.
+ /// The type should be indicated by @c flags.f.is_srv;
+ union {
+ IpAddr ip; ///< IP address / port data.
+ SRVInfo srv; ///< SRV record.
+ } data{IpAddr{}};
+
+ /// Data that migrates after updated DNS records are processed.
+ /// @see migrate_from
+ /// @{
+ /// Last time a failure was recorded.
+ std::atomic<ts_time> last_failure{TS_TIME_ZERO};
+ /// Count of connection failures
+ std::atomic<uint8_t> fail_count{0};
+ /// Expected HTTP version of the target based on earlier transactions.
+ HTTPVersion http_version = HTTP_INVALID;
+ /// @}
+
+ self_type &assign(IpAddr const &addr);
+
+protected:
+ self_type &assign(sa_family_t af, void const *addr);
+ self_type &assign(SRV const *srv, char const *name);
+
+ HostDBType type = HostDBType::UNSPEC; ///< Invalid data.
+
+ friend HostDBContinuation;
+};
- bool
- is_ip_fail_timeout() const
- {
- return ip_interval() >= hostdb_ip_fail_timeout_interval;
+inline HostDBInfo &
+HostDBInfo::operator=(HostDBInfo const &that)
+{
+ if (this != &that) {
Review comment:
Is it normal to assign a HostDBInfo to itself?
##########
File path: proxy/http/HttpTransact.cc
##########
@@ -1685,13 +1686,13 @@ HttpTransact::setup_plugin_request_intercept(State *s)
s->scheme = s->next_hop_scheme = URL_WKSIDX_HTTP;
// Set up a "fake" server server entry
- update_current_info(&s->current, &s->server_info,
HttpTransact::ORIGIN_SERVER, 0);
+ update_current_info(&s->current, &s->server_info,
ResolveInfo::ORIGIN_SERVER, 0);
// Also "fake" the info we'd normally get from
// hostDB
- s->server_info.http_version = HTTP_1_0;
- s->server_info.keep_alive = HTTP_NO_KEEPALIVE;
- s->host_db_info.app.http_data.http_version = HTTP_1_0;
+ s->server_info.http_version = HTTP_1_0;
+ s->server_info.keep_alive = HTTP_NO_KEEPALIVE;
+ s->server_info.http_version = HTTP_1_0;
Review comment:
Looks like this can just be removed, it's duplicated 2 lines up
--
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]
