This is an automated email from the ASF dual-hosted git repository.
masaori335 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 992584c6f0 Clarify HostDBInfo state (#13092)
992584c6f0 is described below
commit 992584c6f076eaedc1d4864a97e3f91a44db1d19
Author: Masaori Koshiba <[email protected]>
AuthorDate: Wed Apr 22 10:28:24 2026 +0900
Clarify HostDBInfo state (#13092)
* Clarify HostDBInfo state
* Address comments from Copilot
* Cleanup comments
---
.../core-architecture/hostdb.en.rst | 14 +-
include/iocore/hostdb/HostDBProcessor.h | 237 +++++++--------------
src/iocore/hostdb/CMakeLists.txt | 1 +
src/iocore/hostdb/HostDB.cc | 8 +-
src/iocore/hostdb/HostDBInfo.cc | 151 +++++++++++++
src/iocore/hostdb/unit_tests/CMakeLists.txt | 22 ++
src/iocore/hostdb/unit_tests/test_HostDBInfo.cc | 214 +++++++++++++++++++
src/mgmt/rpc/handlers/hostdb/HostDB.cc | 4 +-
src/proxy/http/HttpSM.cc | 7 +-
9 files changed, 483 insertions(+), 175 deletions(-)
diff --git a/doc/developer-guide/core-architecture/hostdb.en.rst
b/doc/developer-guide/core-architecture/hostdb.en.rst
index 18a0e06e61..0666c8bfb2 100644
--- a/doc/developer-guide/core-architecture/hostdb.en.rst
+++ b/doc/developer-guide/core-architecture/hostdb.en.rst
@@ -50,12 +50,10 @@ a flag, where a value of ``TS_TIME_ZERO`` indicates a live
target and any other
down info.
If an info is marked down (has a non-zero last failure time) there is a "fail
window" during which
-no connections are permitted. After this time the info is considered to be a
"zombie". If all infos
+no connections are permitted. After this time the info is considered to be a
"suspect". If all infos
for a record are down then a specific error message is generated (body factory
tag
-"connect#all_down"). Otherwise if the selected info is a zombie, a request is
permitted but the
-zombie is immediately marked down again, preventing any additional requests
until either the fail
-window has passed or the single connection succeeds. A successful connection
clears the last file
-time and the info becomes alive.
+"connect#all_down"). Otherwise if the selected info is a suspect, connections
are permitted and the
+info will transition back to up on success or down on failure.
Runtime Structure
=================
@@ -152,8 +150,8 @@ Future
There is still some work to be done in future PRs.
-* The fail window and the zombie window should be separate values. It is
quite reasonable to want
- to configure a very short fail window (possibly 0) with a moderately long
zombie window so that
+* The fail window and the suspect window should be separate values. It is
quite reasonable to want
+ to configure a very short fail window (possibly 0) with a moderately long
suspect window so that
probing connections can immediately start going upstream at a low rate.
* Failing an upstream should be more loosely connected to transactions.
Currently there is a one
@@ -189,7 +187,7 @@ This version has several major architectural changes from
the previous version.
* State information has been promoted to atomics and updates are immediate
rather than scheduled.
This also means the data in the state machine is a reference to a shared
object, not a local copy.
- The promotion was necessary to coordinate zombie connections to upstreams
marked down across transactions.
+ The promotion was necessary to coordinate suspect connections to upstreams
marked down across transactions.
* The "resolve key" is now a separate data object from the HTTP request. This
is a subtle but
major change. The effect is requests can be routed to different upstreams
without changing
diff --git a/include/iocore/hostdb/HostDBProcessor.h
b/include/iocore/hostdb/HostDBProcessor.h
index fecb1abef7..c369aace4f 100644
--- a/include/iocore/hostdb/HostDBProcessor.h
+++ b/include/iocore/hostdb/HostDBProcessor.h
@@ -123,10 +123,53 @@ enum class HostDBType : uint8_t {
};
/** Information about a single target.
+ *
+ * Each instance tracks the health state of one upstream address. The state is
derived from @c _last_failure and the caller-supplied
+ * @a fail_window:
+ *
+ * | State | Description
|
+ *
|---------|-----------------------------------------------------------------------------------|
+ * | Up | No known failure; eligible for normal selection.
|
+ * | Down | Blocked; no connections permitted until @c _last_failure + @a
fail_window elapses |
+ * | Suspect | Fail window has elapsed; connections are permitted.
|
+ * | | On success transitions to Up (@c mark_up); on failure returns
to Down. |
+ *
+ * State transition diagram:
+ *
+ * @startuml
+ * hide empty description
+ *
+ * [*] --> Up
+ * Up --> Down : connect failure\n(mark_down)
+ * Down --> Suspect : fail_window elapses
+ * Suspect --> Up : connect success\n(mark_up)
+ * Suspect --> Down : connect failure\n(mark_down)
+ * @enduml
+ *
+ * State transition and `fail_window` time chart:
+ *
+ * @code
+ * |<-- fail_window -->|
+ * -+----------+--------------------+--------------------+----------+---->
time
+ * | Up | Down | Suspect | Up |
+ * -+----------+--------------------+--------------------+----------+---->
+ * ^ ^ ^
+ * \ \ \
+ * (_last_failure) (_last_failure + fail_window) (connect success)
+ * @endcode
*/
-struct HostDBInfo {
+class HostDBInfo
+{
+public:
using self_type = HostDBInfo; ///< Self reference type.
+ /// Health state of this target.
+ enum class State {
+ UP,
+ DOWN,
+ SUSPECT,
+ };
+
/// Default constructor.
HostDBInfo() = default;
@@ -134,50 +177,23 @@ struct HostDBInfo {
/// Absolute time of when this target failed.
/// A value of zero (@c TS_TIME_ZERO ) indicates no failure.
- ts_time last_fail_time() const;
-
- /// Target is alive - no known failure.
- bool is_alive();
-
- /// Target has failed and is still in the blocked time window.
- bool is_down(ts_time now, ts_seconds fail_window);
-
- /** 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 appear
down 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) const;
+ ts_time last_fail_time() const;
+ uint8_t fail_count() const;
+ char const *srvname() const;
- /** 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);
+ /// Return the current health state of this target.
+ State state(ts_time now, ts_seconds fail_window) const;
- std::pair<bool, uint8_t> increment_fail_count(ts_time now, uint8_t
max_retries);
+ // Sugars of checking state
+ bool is_up() const;
+ bool is_down(ts_time now, ts_seconds fail_window) const;
+ bool is_suspect(ts_time now, ts_seconds fail_window) const;
- /** Mark the target as up / alive.
- *
- * @return Previous alive state of the target.
- */
- bool mark_up();
-
- char const *srvname() const;
+ // State controllers
+ bool mark_up();
+ bool mark_down(ts_time now, ts_seconds fail_window);
+ std::pair<bool, uint8_t> increment_fail_count(ts_time now, uint8_t
max_retries, ts_seconds fail_window);
- /** 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);
/// A target is either an IP address or an SRV record.
@@ -187,16 +203,8 @@ struct HostDBInfo {
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);
@@ -207,96 +215,11 @@ protected:
HostDBType type = HostDBType::UNSPEC; ///< Invalid data.
friend HostDBContinuation;
-};
-inline HostDBInfo &
-HostDBInfo::operator=(HostDBInfo const &that)
-{
- if (this != &that) {
- memcpy(static_cast<void *>(this), static_cast<const void *>(&that),
sizeof(*this));
- }
- return *this;
-}
-
-inline ts_time
-HostDBInfo::last_fail_time() const
-{
- return last_failure;
-}
-
-inline bool
-HostDBInfo::is_alive()
-{
- return this->last_fail_time() == TS_TIME_ZERO;
-}
-
-/**
- Check if this HostDBInfo is currently marked DOWN (true) or UP (false).
Returns true while within the `fail_window` period after
- `last_failure`. Once `fail_window` expires, the host is treated as UP and
this function returns false.
-
- |<-- fail_window -->|
- ----------------+-------------------+-----------------> time
- UP | DOWN | UP
- (is_down=false) | (is_down=true) | (is_down=false)
- | |
- ^ ^
- \ \
- last_failure last_failure + fail_window
- */
-inline bool
-HostDBInfo::is_down(ts_time now, ts_seconds fail_window)
-{
- auto last_fail = this->last_fail_time();
- return (last_fail != TS_TIME_ZERO) && (now <= last_fail + fail_window);
-}
-
-inline bool
-HostDBInfo::mark_up()
-{
- auto t = last_failure.exchange(TS_TIME_ZERO);
- bool was_down = t != TS_TIME_ZERO;
- if (was_down) {
- fail_count.store(0);
- }
- return was_down;
-}
-
-inline bool
-HostDBInfo::mark_down(ts_time now)
-{
- auto t0{TS_TIME_ZERO};
- return last_failure.compare_exchange_strong(t0, now);
-}
-
-inline std::pair<bool, uint8_t>
-HostDBInfo::increment_fail_count(ts_time now, uint8_t max_retries)
-{
- auto fcount = ++fail_count;
- bool marked_down = false;
- if (fcount >= max_retries) {
- marked_down = mark_down(now);
- }
- return std::make_pair(marked_down, fcount);
-}
-
-inline bool
-HostDBInfo::select(ts_time now, ts_seconds fail_window) const
-{
- auto t0 = this->last_fail_time();
- if (t0 == TS_TIME_ZERO) {
- return true; // it's alive and so is valid for selection.
- }
- // Return true and give it a try if enough time is elapsed since the last
failure
- return (t0 + fail_window < now);
-}
-
-inline void
-HostDBInfo::migrate_from(HostDBInfo::self_type const &that)
-{
- this->last_failure = that.last_failure.load();
- this->fail_count = that.fail_count.load();
- this->http_version = that.http_version;
-}
+private:
+ std::atomic<ts_time> _last_failure{TS_TIME_ZERO}; ///< Last time a failure
was recorded
+ std::atomic<uint8_t> _fail_count{0}; ///< Count of connection
failures
+};
// ----
/** Root item for HostDB.
@@ -371,15 +294,12 @@ public:
/** Pick the next round robin and update the record atomically.
*
- * @note This may select a zombie server and reserve it for the caller,
therefore the caller must
- * attempt to connect to the selected target if possible.
- *
- * @param now Current time to use for aliveness calculations.
- * @param fail_window Blackout time for down servers.
- * @return Status of the updated target.
+ * @note This may select a suspect server. The caller must attempt to
connect to the selected
+ * target if possible.
*
- * If the return value is @c HostDBInfo::Status::DOWN this means all targets
are down and there is
- * no valid upstream.
+ * @param[in] now Current time to use for HostDBInfo state calculations.
+ * @param[in] fail_window Blackout time for down servers.
+ * @return The selected target, or @c nullptr if all targets are down.
*
* @note Concurrency - this is not done under lock and depends on the caller
for correct use.
* For strict round robin, it is a feature that every call will get a
distinct index. For
@@ -434,9 +354,9 @@ public:
* This accounts for the round robin setting. The default is to use "client
affinity" in
* which case @a hash_addr is as a hash seed to select the target.
*
- * This may select a zombie target, which can be detected by checking the
target's last
- * failure time. If it is not @c TS_TIME_ZERO the target is a zombie. Other
transactions will
- * be blocked from selecting that target until @a fail_window time has
passed.
+ * This may select a suspect target (fail window elapsed, connections
permitted again), which can
+ * be detected by checking the target's last failure time. If it is not @c
TS_TIME_ZERO the target
+ * is a suspect. Multiple threads may concurrently select the same suspect
target.
*
* In cases other than strict round robin, a base target is selected. If
valid, that is returned,
* but if not then the targets in this record are searched until a valid one
is found. The result
@@ -588,7 +508,7 @@ struct ResolveInfo {
/// Keep a reference to the base HostDB object, so it doesn't get GC'd.
Ptr<HostDBRecord> record;
- HostDBInfo *active = nullptr; ///< Active host record.
+ HostDBInfo *active = nullptr; ///< Active HostDBInfo
/// Working address. The meaning / source of the value depends on other
elements.
/// This is the "resolved" address if @a resolved_p is @c true.
@@ -646,19 +566,20 @@ struct ResolveInfo {
*/
bool resolve_immediate();
- /** Mark the active target as down.
+ /** Mark the active target as DOWN.
*
- * @param now Time of failure.
+ * @param[in] now Time of failure.
+ * @param[in] fail_window The fail window duration
(proxy.config.http.down_server.cache_time).
* @return @c true if the server was marked as down, @c false if not.
*
*/
- bool mark_active_server_down(ts_time now);
+ bool mark_active_server_down(ts_time now, ts_seconds fail_window);
- /** Mark the active target as alive.
+ /** Mark the active target as UP.
*
* @return @c true if the target changed state.
*/
- bool mark_active_server_alive();
+ bool mark_active_server_up();
/// Select / resolve to the next RR entry for the record.
bool select_next_rr();
@@ -863,15 +784,15 @@ ResolveInfo::set_active(sockaddr const *s)
}
inline bool
-ResolveInfo::mark_active_server_alive()
+ResolveInfo::mark_active_server_up()
{
return active->mark_up();
}
inline bool
-ResolveInfo::mark_active_server_down(ts_time now)
+ResolveInfo::mark_active_server_down(ts_time now, ts_seconds fail_window)
{
- return active != nullptr && active->mark_down(now);
+ return active != nullptr && active->mark_down(now, fail_window);
}
inline bool
diff --git a/src/iocore/hostdb/CMakeLists.txt b/src/iocore/hostdb/CMakeLists.txt
index 4e099f8da7..07360c8f2d 100644
--- a/src/iocore/hostdb/CMakeLists.txt
+++ b/src/iocore/hostdb/CMakeLists.txt
@@ -45,4 +45,5 @@ if(BUILD_TESTING)
)
add_catch2_test(NAME test_hostdb_RefCountCache COMMAND
$<TARGET_FILE:test_RefCountCache>)
+ add_subdirectory(unit_tests)
endif()
diff --git a/src/iocore/hostdb/HostDB.cc b/src/iocore/hostdb/HostDB.cc
index 12a8532be6..ec7473e292 100644
--- a/src/iocore/hostdb/HostDB.cc
+++ b/src/iocore/hostdb/HostDB.cc
@@ -1317,14 +1317,14 @@ HostDBRecord::select_best_http(ts_time now, ts_seconds
fail_window, sockaddr con
// Starting at the current target, search for a valid one.
for (unsigned short i = 0; i < rr_count; i++) {
auto target = &info[this->rr_idx(i)];
- if (target->select(now, fail_window)) {
+ if (!target->is_down(now, fail_window)) {
best_alive = target;
break;
}
}
}
} else {
- if (info[0].select(now, fail_window)) {
+ if (!info[0].is_down(now, fail_window)) {
best_alive = &info[0];
}
}
@@ -1647,7 +1647,7 @@ HostDBRecord::select_next_rr(ts_time now, ts_seconds
fail_window)
auto rr_info = this->rr_info();
for (unsigned idx = 0, limit = rr_info.count(); idx < limit; ++idx) {
auto &target = rr_info[this->next_rr()];
- if (target.select(now, fail_window)) {
+ if (!target.is_down(now, fail_window)) {
return ⌖
}
}
@@ -1711,7 +1711,7 @@ ResolveInfo::select_next_rr()
if (active) {
if (auto rr_info{this->record->rr_info()}; rr_info.count() > 1) {
unsigned limit = active - rr_info.data(), idx = (limit + 1) %
rr_info.count();
- while ((idx = (idx + 1) % rr_info.count()) != limit &&
!rr_info[idx].is_alive()) {}
+ while ((idx = (idx + 1) % rr_info.count()) != limit &&
!rr_info[idx].is_up()) {}
active = &rr_info[idx];
return idx != limit; // if the active record was actually changed.
}
diff --git a/src/iocore/hostdb/HostDBInfo.cc b/src/iocore/hostdb/HostDBInfo.cc
index db76345a16..1bb4126359 100644
--- a/src/iocore/hostdb/HostDBInfo.cc
+++ b/src/iocore/hostdb/HostDBInfo.cc
@@ -27,6 +27,8 @@
namespace
{
+DbgCtl dbg_ctl_hostdb_info{"hostdb_info"};
+
/** Assign raw storage to an @c IpAddr
*
* @param ip Destination.
@@ -95,3 +97,152 @@ HostDBInfo::srvname() const
{
return data.srv.srv_offset ? reinterpret_cast<char const *>(this) +
data.srv.srv_offset : nullptr;
}
+
+HostDBInfo &
+HostDBInfo::operator=(HostDBInfo const &that)
+{
+ if (this != &that) {
+ memcpy(static_cast<void *>(this), static_cast<const void *>(&that),
sizeof(*this));
+ }
+ return *this;
+}
+
+ts_time
+HostDBInfo::last_fail_time() const
+{
+ return _last_failure;
+}
+
+uint8_t
+HostDBInfo::fail_count() const
+{
+ return _fail_count;
+}
+
+HostDBInfo::State
+HostDBInfo::state(ts_time now, ts_seconds fail_window) const
+{
+ auto last_fail = this->last_fail_time();
+ if (last_fail == TS_TIME_ZERO) {
+ return State::UP;
+ }
+
+ if (now <= last_fail + fail_window) {
+ return State::DOWN;
+ } else {
+ return State::SUSPECT;
+ }
+}
+
+bool
+HostDBInfo::is_up() const
+{
+ return this->last_fail_time() == TS_TIME_ZERO;
+}
+
+bool
+HostDBInfo::is_down(ts_time now, ts_seconds fail_window) const
+{
+ return this->state(now, fail_window) == State::DOWN;
+}
+
+bool
+HostDBInfo::is_suspect(ts_time now, ts_seconds fail_window) const
+{
+ return this->state(now, fail_window) == State::SUSPECT;
+}
+
+/** Mark the target as UP
+ *
+ * @return @c true if the target was previously DOWN or SUSPECT (i.e., a state
change occurred).
+ */
+bool
+HostDBInfo::mark_up()
+{
+ auto t = _last_failure.exchange(TS_TIME_ZERO);
+ _fail_count.store(0);
+
+ return t != TS_TIME_ZERO;
+}
+
+/** Mark the entry as DOWN.
+ *
+ * @param[in] now Time of the failure.
+ * @param[in] fail_window The fail window duration
(proxy.config.http.down_server.cache_time).
+ * @return @c true if @a this was marked down, @c false if not.
+ *
+ * Handles two transitions:
+ * - UP → DOWN: @c _last_failure is @c TS_TIME_ZERO; set via CAS.
+ * - SUSPECT → DOWN: @c fail_window has elapsed since the last failure; @c
_last_failure is
+ * refreshed via CAS to restart the fail window.
+ *
+ * On a successful transition @c _fail_count is reset to zero so that the next
SUSPECT window
+ * accumulates failures from a clean baseline.
+ *
+ * Returns @c false if the server is already DOWN (within the active fail
window), so the
+ * fail window is not refreshed by concurrent failures.
+ */
+bool
+HostDBInfo::mark_down(ts_time now, ts_seconds fail_window)
+{
+ // UP -> DOWN
+ auto t0{TS_TIME_ZERO};
+ if (_last_failure.compare_exchange_strong(t0, now)) {
+ // Reset so the next SUSPECT window starts with a fresh failure count.
+ _fail_count.store(0);
+ return true;
+ }
+
+ // After the failed CAS, t0 holds the current _last_failure value.
+ // SUSPECT -> DOWN: the fail window has elapsed; refresh _last_failure to
restart it.
+ if (t0 + fail_window < now) {
+ if (_last_failure.compare_exchange_strong(t0, now)) {
+ // Reset so the next SUSPECT window starts with a fresh failure count.
+ _fail_count.store(0);
+ return true;
+ }
+ }
+
+ // Already DOWN; don't refresh the fail window.
+ return false;
+}
+
+/** Increment the connection failure counter and conditionally mark the target
DOWN.
+ *
+ * @param[in] now Current time, used as the failure timestamp if the
target is marked DOWN.
+ * @param[in] max_retries Number of failures that triggers a transition to
DOWN.
+ * @param[in] fail_window The fail window duration
(proxy.config.http.down_server.cache_time).
+ * @return A pair { @c marked_down, @c fail_count } where @c marked_down is @c
true if this call
+ * caused the target to transition to DOWN (i.e. @c fail_count just
reached @a max_retries
+ * and the @c mark_down CAS succeeded), and @c fail_count is the
updated counter value.
+ *
+ * @note @c marked_down can be @c false even when @c fail_count >= @a
max_retries if another
+ * thread concurrently marked the target DOWN first (the CAS on @c
_last_failure will fail).
+ */
+std::pair<bool, uint8_t>
+HostDBInfo::increment_fail_count(ts_time now, uint8_t max_retries, ts_seconds
fail_window)
+{
+ auto fcount = ++_fail_count;
+ bool marked_down = false;
+
+ Dbg(dbg_ctl_hostdb_info, "fail_count=%d max_retries=%d", fcount,
max_retries);
+
+ if (fcount >= max_retries) {
+ marked_down = mark_down(now, fail_window);
+ }
+ return std::make_pair(marked_down, fcount);
+}
+
+/** Migrate data after a DNS update.
+ *
+ * @param[in] that Source item.
+ *
+ * This moves only specific state information, it is not a generic copy.
+ */
+void
+HostDBInfo::migrate_from(HostDBInfo::self_type const &that)
+{
+ this->_last_failure = that._last_failure.load();
+ this->_fail_count = that._fail_count.load();
+ this->http_version = that.http_version;
+}
diff --git a/src/iocore/hostdb/unit_tests/CMakeLists.txt
b/src/iocore/hostdb/unit_tests/CMakeLists.txt
new file mode 100644
index 0000000000..0354641423
--- /dev/null
+++ b/src/iocore/hostdb/unit_tests/CMakeLists.txt
@@ -0,0 +1,22 @@
+######################
+#
+# 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.
+#
+######################
+
+add_executable(test_hostdb test_HostDBInfo.cc
"${CMAKE_CURRENT_SOURCE_DIR}/../HostDBInfo.cc")
+
+target_link_libraries(test_hostdb PRIVATE Catch2::Catch2WithMain ts::tscore
ts::tsutil ts::inkevent configmanager)
+
+add_catch2_test(NAME test_hostdb COMMAND $<TARGET_FILE:test_hostdb>)
diff --git a/src/iocore/hostdb/unit_tests/test_HostDBInfo.cc
b/src/iocore/hostdb/unit_tests/test_HostDBInfo.cc
new file mode 100644
index 0000000000..431ef1fa1f
--- /dev/null
+++ b/src/iocore/hostdb/unit_tests/test_HostDBInfo.cc
@@ -0,0 +1,214 @@
+/** @file
+
+ Unit tests for HostDBInfo state transitions (UP / DOWN / SUSPECT).
+
+ @section license License
+
+ 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.
+ */
+
+#include <catch2/catch_test_macros.hpp>
+
+#include "iocore/hostdb/HostDBProcessor.h"
+#include "tscore/ink_time.h"
+
+// ---------------------------------------------------------------------------
+// Helpers
+// ---------------------------------------------------------------------------
+
+// fail_window used throughout the tests
+static const ts_seconds FAIL_WINDOW{300};
+
+static const ts_time T0 = ts_clock::now(); ///< A fixed anchor in time;
+
+static const ts_time T1 = T0 + ts_seconds(1); ///< A time within
FAIL_WINDOW from T0
+static const ts_time T2 = T0 + FAIL_WINDOW + ts_seconds(1); ///< A time past
FAIL_WINDOW from T0
+
+static const ts_time T3 = T2 + ts_seconds(1); ///< A time within
FAIL_WINDOW from T2
+static const ts_time T4 = T2 + FAIL_WINDOW + ts_seconds(1); ///< A time past
FAIL_WINDOW from T2
+// ---------------------------------------------------------------------------
+// Tests
+// ---------------------------------------------------------------------------
+
+TEST_CASE("HostDBInfo state", "[hostdb]")
+{
+ SECTION("initial state is UP")
+ {
+ HostDBInfo info;
+ REQUIRE(info.state(T0, FAIL_WINDOW) == HostDBInfo::State::UP);
+ REQUIRE(info.is_up());
+ REQUIRE(info.last_fail_time() == TS_TIME_ZERO);
+ REQUIRE(info.fail_count() == 0);
+ }
+
+ SECTION("UP -> DOWN via mark_down; fail_count is reset")
+ {
+ HostDBInfo info;
+ REQUIRE(info.mark_down(T0, FAIL_WINDOW) == true);
+ REQUIRE(info.last_fail_time() == T0);
+ REQUIRE(info.fail_count() == 0); // reset so the next SUSPECT window
starts fresh
+ REQUIRE(info.state(T1, FAIL_WINDOW) == HostDBInfo::State::DOWN);
+ }
+
+ SECTION("mark_down on DOWN server returns false and does not refresh fail
window")
+ {
+ HostDBInfo info;
+ info.mark_down(T0, FAIL_WINDOW);
+ REQUIRE(info.mark_down(T1, FAIL_WINDOW) == false);
+ REQUIRE(info.last_fail_time() == T0); // unchanged
+ }
+
+ SECTION("DOWN -> SUSPECT when fail window elapses (time-based, no explicit
call)")
+ {
+ HostDBInfo info;
+ info.mark_down(T0, FAIL_WINDOW);
+ REQUIRE(info.state(T2, FAIL_WINDOW) == HostDBInfo::State::SUSPECT);
+ REQUIRE(info.is_suspect(T2, FAIL_WINDOW));
+ REQUIRE(info.last_fail_time() == T0); // _last_failure is unchanged
+ }
+
+ SECTION("SUSPECT -> DOWN via mark_down; fail_count is reset; fail window
restarts")
+ {
+ HostDBInfo info;
+ info.mark_down(T0, FAIL_WINDOW);
+
+ REQUIRE(info.mark_down(T2, FAIL_WINDOW) == true);
+ REQUIRE(info.last_fail_time() == T2); // refreshed to the new failure time
+ REQUIRE(info.fail_count() == 0); // reset so the next SUSPECT window
starts fresh
+ // server is DOWN again with a fresh window anchored at T2
+ REQUIRE(info.state(T2 + FAIL_WINDOW / 2, FAIL_WINDOW) ==
HostDBInfo::State::DOWN);
+ }
+
+ SECTION("SUSPECT -> UP via mark_up; returns true (was DOWN/SUSPECT)")
+ {
+ HostDBInfo info;
+ info.mark_down(T0, FAIL_WINDOW);
+ REQUIRE(info.mark_up() == true);
+ REQUIRE(info.state(T0, FAIL_WINDOW) == HostDBInfo::State::UP);
+ REQUIRE(info.last_fail_time() == TS_TIME_ZERO);
+ REQUIRE(info.fail_count() == 0);
+ }
+
+ SECTION("mark_up on already-UP server returns false")
+ {
+ HostDBInfo info;
+ REQUIRE(info.mark_up() == false);
+ }
+
+ SECTION("UP -> DOWN via increment_fail_count with max_retries=1")
+ {
+ HostDBInfo info;
+ auto [marked, count] = info.increment_fail_count(T0, 1, FAIL_WINDOW);
+ REQUIRE(marked == true);
+ REQUIRE(count == 1);
+ REQUIRE(info.state(T1, FAIL_WINDOW) == HostDBInfo::State::DOWN);
+ REQUIRE(info.fail_count() == 0); // reset by mark_down
+ }
+
+ SECTION("UP -> DOWN via increment_fail_count with max_retries=3 requires 3
failures")
+ {
+ HostDBInfo info;
+ {
+ auto [marked, count] = info.increment_fail_count(T0, 3, FAIL_WINDOW);
+ REQUIRE(marked == false);
+ REQUIRE(count == 1);
+ }
+ {
+ auto [marked, count] = info.increment_fail_count(T0, 3, FAIL_WINDOW);
+ REQUIRE(marked == false);
+ REQUIRE(count == 2);
+ }
+ {
+ auto [marked, count] = info.increment_fail_count(T0, 3, FAIL_WINDOW);
+ REQUIRE(marked == true);
+ REQUIRE(count == 3);
+ }
+ REQUIRE(info.state(T1, FAIL_WINDOW) == HostDBInfo::State::DOWN);
+ REQUIRE(info.fail_count() == 0); // reset by mark_down
+ }
+
+ SECTION("SUSPECT -> DOWN via increment_fail_count; fail_count starts from
zero each SUSPECT window")
+ {
+ HostDBInfo info;
+ // Drive UP -> DOWN (R=1); fail_count is reset to 0 after mark_down.
+ info.increment_fail_count(T0, 1, FAIL_WINDOW);
+ REQUIRE(info.fail_count() == 0);
+
+ // D=3: need exactly 3 fresh failures in the SUSPECT window.
+ {
+ auto [marked, count] = info.increment_fail_count(T2, 3, FAIL_WINDOW);
+ REQUIRE(marked == false);
+ REQUIRE(count == 1);
+ }
+ {
+ auto [marked, count] = info.increment_fail_count(T2, 3, FAIL_WINDOW);
+ REQUIRE(marked == false);
+ REQUIRE(count == 2);
+ }
+ {
+ auto [marked, count] = info.increment_fail_count(T2, 3, FAIL_WINDOW);
+ REQUIRE(marked == true);
+ REQUIRE(count == 3);
+ }
+ REQUIRE(info.last_fail_time() == T2);
+ REQUIRE(info.fail_count() == 0); // reset by mark_down SUSPECT->DOWN
+ }
+
+ SECTION("full cycle: UP -> DOWN -> SUSPECT -> DOWN -> SUSPECT -> UP")
+ {
+ HostDBInfo info;
+
+ // UP -> DOWN
+ info.increment_fail_count(T0, 1, FAIL_WINDOW);
+ REQUIRE(info.state(T1, FAIL_WINDOW) == HostDBInfo::State::DOWN);
+ REQUIRE(info.fail_count() == 0);
+
+ // DOWN -> SUSPECT
+ REQUIRE(info.state(T2, FAIL_WINDOW) == HostDBInfo::State::SUSPECT);
+
+ // SUSPECT -> DOWN (fail window restarts from T2)
+ info.increment_fail_count(T2, 1, FAIL_WINDOW);
+ REQUIRE(info.last_fail_time() == T2);
+ REQUIRE(info.state(T3, FAIL_WINDOW) == HostDBInfo::State::DOWN);
+ REQUIRE(info.fail_count() == 0);
+
+ // DOWN -> SUSPECT again
+ REQUIRE(info.state(T4, FAIL_WINDOW) == HostDBInfo::State::SUSPECT);
+
+ // SUSPECT -> UP
+ REQUIRE(info.mark_up() == true);
+ REQUIRE(info.state(T4, FAIL_WINDOW) == HostDBInfo::State::UP);
+ REQUIRE(info.fail_count() == 0);
+ }
+
+ SECTION("partial failures in SUSPECT then success resets to UP cleanly")
+ {
+ HostDBInfo info;
+ info.increment_fail_count(T0, 1, FAIL_WINDOW);
+
+ // Two failures below the D=3 threshold
+ info.increment_fail_count(T2, 3, FAIL_WINDOW);
+ info.increment_fail_count(T2, 3, FAIL_WINDOW);
+ REQUIRE(info.state(T2, FAIL_WINDOW) == HostDBInfo::State::SUSPECT);
+
+ // Connection succeeds
+ info.mark_up();
+ REQUIRE(info.state(T2, FAIL_WINDOW) == HostDBInfo::State::UP);
+ REQUIRE(info.last_fail_time() == TS_TIME_ZERO);
+ REQUIRE(info.fail_count() == 0);
+ }
+}
diff --git a/src/mgmt/rpc/handlers/hostdb/HostDB.cc
b/src/mgmt/rpc/handlers/hostdb/HostDB.cc
index d90343bb1f..9759a1cd19 100644
--- a/src/mgmt/rpc/handlers/hostdb/HostDB.cc
+++ b/src/mgmt/rpc/handlers/hostdb/HostDB.cc
@@ -157,8 +157,8 @@ template <> struct convert<HostDBRecord> {
info_node["ip"] = std::string(buf);
}
- info_node["health"]["last_failure"] =
info.last_failure.load().time_since_epoch().count();
- info_node["health"]["fail_count"] =
static_cast<int>(info.fail_count.load());
+ info_node["health"]["last_failure"] =
info.last_fail_time().time_since_epoch().count();
+ info_node["health"]["fail_count"] =
static_cast<int>(info.fail_count());
node["info"].push_back(info_node);
}
diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc
index de97e3a200..c8a6b128dc 100644
--- a/src/proxy/http/HttpSM.cc
+++ b/src/proxy/http/HttpSM.cc
@@ -4735,10 +4735,10 @@ HttpSM::do_hostdb_update_if_necessary()
if (track_connect_fail()) {
this->mark_host_failure(&t_state.dns_info,
ts_clock::from_time_t(t_state.client_request_time));
} else {
- if (t_state.dns_info.mark_active_server_alive()) {
+ if (t_state.dns_info.mark_active_server_up()) {
char addrbuf[INET6_ADDRPORTSTRLEN];
ats_ip_nptop(&t_state.current.server->dst_addr.sa, addrbuf,
sizeof(addrbuf));
- ATS_PROBE2(mark_active_server_alive, sm_id, addrbuf);
+ ATS_PROBE2(mark_active_server_up, sm_id, addrbuf);
if (t_state.dns_info.record->is_srv()) {
SMDbg(dbg_ctl_http, "[%" PRId64 "] hostdb update marking SRV: %s(%s)
as up", sm_id, t_state.dns_info.record->name(),
addrbuf);
@@ -5978,7 +5978,8 @@ HttpSM::mark_host_failure(ResolveInfo *info, ts_time
time_down)
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);
+ 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()};
