Merge pull request #25898 from vespa-engine/balder/use-steady-time-for-node-state

Use steady_time for vdslib::NodeState MERGEOK

5 files changed, 67 insertions, 73 deletions

diff --git a/storage/src/vespa/storage/storageserver/statemanager.cpp b/storage/src/vespa/storage/storageserver/statemanager.cpp
index 81961370ed3..647cba52bfc 100644
--- a/storage/src/vespa/storage/storageserver/statemanager.cpp
+++ b/storage/src/vespa/storage/storageserver/statemanager.cpp
@@ -9,16 +9,14 @@
 #include <vespa/metrics/metricset.h>
 #include <vespa/metrics/metrictimer.h>
 #include <vespa/metrics/valuemetric.h>
-#include <vespa/storageapi/messageapi/storagemessage.h>
 #include <vespa/vdslib/state/cluster_state_bundle.h>
 #include <vespa/vdslib/state/clusterstate.h>
 #include <vespa/vespalib/stllike/asciistream.h>
 #include <vespa/vespalib/util/exceptions.h>
 #include <vespa/vespalib/util/string_escape.h>
 #include <vespa/vespalib/util/stringfmt.h>
-
 #include <fstream>
-#include <unistd.h>
+#include <ranges>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".state.manager");
@@ -71,7 +69,7 @@ StateManager::StateManager(StorageComponentRegister& compReg,
       _requested_almost_immediate_node_state_replies(false)
 {
     _nodeState->setMinUsedBits(58);
-    _nodeState->setStartTimestamp(_component.getClock().getTimeInSeconds().getTime());
+    _nodeState->setStartTimestamp(_component.getClock().getSystemTime());
     _component.registerStatusPage(*this);
     _component.registerMetric(*_metrics);
 }
@@ -135,9 +133,9 @@ StateManager::reportHtmlStatus(std::ostream& out,
             << "<h1>System state history</h1>\n"
             << "<table border=\"1\"><tr>"
             << "<th>Received at time</th><th>State</th></tr>\n";
-        for (auto it = _systemStateHistory.rbegin(); it != _systemStateHistory.rend(); ++it) {
-            out << "<tr><td>" << it->first << "</td><td>"
-                << xml_content_escaped(it->second->getBaselineClusterState()->toString()) << "</td></tr>\n";
+        for (const auto & it : std::ranges::reverse_view(_systemStateHistory)) {
+            out << "<tr><td>" << vespalib::to_string(vespalib::to_utc(it.first)) << "</td><td>"
+                << xml_content_escaped(it.second->getBaselineClusterState()->toString()) << "</td></tr>\n";
         }
         out << "</table>\n";
     }
@@ -146,7 +144,7 @@ StateManager::reportHtmlStatus(std::ostream& out,
 lib::Node
 StateManager::thisNode() const
 {
-    return lib::Node(_component.getNodeType(), _component.getIndex());
+    return { _component.getNodeType(), _component.getIndex() };
 }
 
 lib::NodeState::CSP
@@ -298,7 +296,7 @@ StateManager::enableNextClusterState()
         _reported_host_info_cluster_state_version = _systemState->getVersion();
     } // else: reported version updated upon explicit activation edge
     _nextSystemState.reset();
-    _systemStateHistory.emplace_back(_component.getClock().getTimeInMillis(), _systemState);
+    _systemStateHistory.emplace_back(_component.getClock().getMonotonicTime(), _systemState);
 }
 
 namespace {
@@ -392,8 +390,7 @@ StateManager::onGetNodeState(const api::GetNodeStateCommand::SP& cmd)
 {
     bool sentReply = false;
     if (cmd->getSourceIndex() != 0xffff) {
-        sentReply = sendGetNodeStateReplies(framework::MilliSecTime(0),
-                                            cmd->getSourceIndex());
+        sentReply = sendGetNodeStateReplies(vespalib::steady_time::max(), cmd->getSourceIndex());
     }
     std::shared_ptr<api::GetNodeStateReply> reply;
     {
@@ -404,16 +401,13 @@ StateManager::onGetNodeState(const api::GetNodeStateCommand::SP& cmd)
             && (*cmd->getExpectedState() == *_nodeState || sentReply)
             && is_up_to_date)
         {
-            int64_t msTimeout = vespalib::count_ms(cmd->getTimeout());
+            vespalib::duration timeout = cmd->getTimeout();
             LOG(debug, "Received get node state request with timeout of "
-                       "%" PRId64 " milliseconds. Scheduling to be answered in "
-                       "%" PRId64 " milliseconds unless a node state change "
+                       "%f seconds. Scheduling to be answered in "
+                       "%f seconds unless a node state change "
                        "happens before that time.",
-                msTimeout, msTimeout * 800 / 1000);
-            TimeStateCmdPair pair(
-                    _component.getClock().getTimeInMillis()
-                    + framework::MilliSecTime(msTimeout * 800 / 1000),
-                    cmd);
+                vespalib::to_s(timeout), vespalib::to_s(timeout)*0.8);
+            TimeStateCmdPair pair(_component.getClock().getMonotonicTime() + timeout, cmd);
             _queuedStateRequests.emplace_back(std::move(pair));
         } else {
             LOG(debug, "Answered get node state request right away since it "
@@ -497,13 +491,14 @@ StateManager::tick() {
     bool almost_immediate_replies = _requested_almost_immediate_node_state_replies.load(std::memory_order_relaxed);
     if (almost_immediate_replies) {
         _requested_almost_immediate_node_state_replies.store(false, std::memory_order_relaxed);
+        sendGetNodeStateReplies();
+    } else {
+        sendGetNodeStateReplies(_component.getClock().getMonotonicTime());
     }
-    framework::MilliSecTime time(almost_immediate_replies ? framework::MilliSecTime(0) : _component.getClock().getTimeInMillis());
-    sendGetNodeStateReplies(time);
 }
 
 bool
-StateManager::sendGetNodeStateReplies(framework::MilliSecTime olderThanTime, uint16_t node)
+StateManager::sendGetNodeStateReplies(vespalib::steady_time olderThanTime, uint16_t node)
 {
     std::vector<std::shared_ptr<api::GetNodeStateReply>> replies;
     {
@@ -511,9 +506,8 @@ StateManager::sendGetNodeStateReplies(framework::MilliSecTime olderThanTime, uin
         for (auto it = _queuedStateRequests.begin(); it != _queuedStateRequests.end();) {
             if (node != 0xffff && node != it->second->getSourceIndex()) {
                 ++it;
-            } else if (!olderThanTime.isSet() || it->first < olderThanTime) {
-                LOG(debug, "Sending reply to msg with id %" PRIu64,
-                    it->second->getMsgId());
+            } else if (it->first < olderThanTime) {
+                LOG(debug, "Sending reply to msg with id %" PRIu64, it->second->getMsgId());
 
                 replies.emplace_back(std::make_shared<api::GetNodeStateReply>(*it->second, *_nodeState));
                 auto eraseIt = it++;
diff --git a/storage/src/vespa/storage/storageserver/statemanager.h b/storage/src/vespa/storage/storageserver/statemanager.h
index 74b59875ff8..3605a0b1605 100644
--- a/storage/src/vespa/storage/storageserver/statemanager.h
+++ b/storage/src/vespa/storage/storageserver/statemanager.h
@@ -42,8 +42,8 @@ class StateManager : public NodeStateUpdater,
                      private vespalib::JsonStreamTypes
 {
     using ClusterStateBundle = lib::ClusterStateBundle;
-    using TimeStateCmdPair   = std::pair<framework::MilliSecTime, api::GetNodeStateCommand::SP>;
-    using TimeSysStatePair   = std::pair<framework::MilliSecTime, std::shared_ptr<const ClusterStateBundle>>;
+    using TimeStateCmdPair   = std::pair<vespalib::steady_time, api::GetNodeStateCommand::SP>;
+    using TimeSysStatePair   = std::pair<vespalib::steady_time, std::shared_ptr<const ClusterStateBundle>>;
 
     struct StateManagerMetrics;
 
@@ -109,7 +109,7 @@ private:
 
     void notifyStateListeners();
     bool sendGetNodeStateReplies(
-            framework::MilliSecTime olderThanTime = framework::MilliSecTime(0),
+            vespalib::steady_time olderThanTime = vespalib::steady_time::max(),
             uint16_t index = 0xffff);
     void mark_controller_as_having_observed_explicit_node_state(const std::unique_lock<std::mutex> &, uint16_t controller_index);
 
diff --git a/vdslib/src/tests/state/nodestatetest.cpp b/vdslib/src/tests/state/nodestatetest.cpp
index 000542e77fe..c854adf3915 100644
--- a/vdslib/src/tests/state/nodestatetest.cpp
+++ b/vdslib/src/tests/state/nodestatetest.cpp
@@ -20,7 +20,7 @@ TEST(NodeStateTest, test_parsing)
     {
         NodeState ns = NodeState("t:4");
         EXPECT_EQ(std::string("s:u t:4"), ns.toString());
-        EXPECT_EQ(uint64_t(4), ns.getStartTimestamp());
+        EXPECT_EQ(4s, ns.getStartTimestamp().time_since_epoch());
     }
     {
         NodeState ns = NodeState("s:u c:2.4 b:12");
diff --git a/vdslib/src/vespa/vdslib/state/nodestate.cpp b/vdslib/src/vespa/vdslib/state/nodestate.cpp
index a7c5476456a..0dd7f5abb4c 100644
--- a/vdslib/src/vespa/vdslib/state/nodestate.cpp
+++ b/vdslib/src/vespa/vdslib/state/nodestate.cpp
@@ -8,9 +8,8 @@
 #include <vespa/vespalib/util/exceptions.h>
 #include <vespa/vespalib/stllike/asciistream.h>
 #include <sstream>
-#include <cmath>
-#include <vespa/log/log.h>
 
+#include <vespa/log/log.h>
 LOG_SETUP(".vdslib.nodestate");
 
 namespace storage::lib {
@@ -19,16 +18,16 @@ NodeState::NodeState(const NodeState &) = default;
 NodeState & NodeState::operator = (const NodeState &) = default;
 NodeState::NodeState(NodeState &&) noexcept = default;
 NodeState & NodeState::operator = (NodeState &&) noexcept = default;
-NodeState::~NodeState() { }
+NodeState::~NodeState() = default;
 
 NodeState::NodeState()
-    : _type(0),
-      _state(0),
+    : _type(nullptr),
+      _state(nullptr),
       _description(""),
       _capacity(1.0),
       _initProgress(0.0),
       _minUsedBits(16),
-      _startTimestamp(0)
+      _startTimestamp()
 {
     setState(State::UP);
 }
@@ -36,12 +35,12 @@ NodeState::NodeState()
 NodeState::NodeState(const NodeType& type, const State& state,
                      vespalib::stringref description, double capacity)
     : _type(&type),
-      _state(0),
+      _state(nullptr),
       _description(description),
       _capacity(1.0),
       _initProgress(0.0),
       _minUsedBits(16),
-      _startTimestamp(0)
+      _startTimestamp()
 {
     setState(state);
     if (type == NodeType::STORAGE) {
@@ -56,25 +55,24 @@ NodeState::NodeState(vespalib::stringref serialized, const NodeType* type)
       _capacity(1.0),
       _initProgress(0.0),
       _minUsedBits(16),
-      _startTimestamp(0)
+      _startTimestamp()
 {
 
     vespalib::StringTokenizer st(serialized, " \t\f\r\n");
     st.removeEmptyTokens();
-    for (vespalib::StringTokenizer::Iterator it = st.begin();
-         it != st.end(); ++it)
+    for (auto it : st)
     {
-        std::string::size_type index = it->find(':');
+        std::string::size_type index = it.find(':');
         if (index == std::string::npos) {
             throw vespalib::IllegalArgumentException(
-                    "Token " + *it + " does not contain ':': " + serialized,
+                    "Token " + it + " does not contain ':': " + serialized,
                     VESPA_STRLOC);
         }
-        std::string key = it->substr(0, index);
-        std::string value = it->substr(index + 1);
-        if (key.size() > 0) switch (key[0]) {
+        std::string key = it.substr(0, index);
+        std::string value = it.substr(index + 1);
+        if (!key.empty()) switch (key[0]) {
             case 'b':
-                if (_type != 0 && *type != NodeType::STORAGE) break;
+                if (_type != nullptr && *type != NodeType::STORAGE) break;
                 if (key.size() > 1) break;
                 try{
                     setMinUsedBits(boost::lexical_cast<uint32_t>(value));
@@ -91,7 +89,7 @@ NodeState::NodeState(vespalib::stringref serialized, const NodeType* type)
                 continue;
             case 'c':
                 if (key.size() > 1) break;
-                if (_type != 0 && *type != NodeType::STORAGE) break;
+                if (_type != nullptr && *type != NodeType::STORAGE) break;
                 try{
                     setCapacity(boost::lexical_cast<double>(value));
                 } catch (...) {
@@ -115,7 +113,7 @@ NodeState::NodeState(vespalib::stringref serialized, const NodeType* type)
             case 't':
                 if (key.size() > 1) break;
                 try{
-                    setStartTimestamp(boost::lexical_cast<uint64_t>(value));
+                    setStartTimestamp(vespalib::system_time(std::chrono::seconds(boost::lexical_cast<uint64_t>(value))));
                 } catch (...) {
                     throw vespalib::IllegalArgumentException(
                             "Illegal start timestamp '" + value + "'. Start "
@@ -165,7 +163,7 @@ NodeState::serialize(vespalib::asciistream & out, vespalib::stringref prefix,
     SeparatorPrinter sep;
     // Always give node state if not part of a system state
     // to prevent empty serialization
-    if (*_state != State::UP || prefix.size() == 0) {
+    if (*_state != State::UP || prefix.empty()) {
         out << sep << prefix << "s:";
         out << _state->serialize();
     }
@@ -178,8 +176,8 @@ NodeState::serialize(vespalib::asciistream & out, vespalib::stringref prefix,
     if (*_state == State::INITIALIZING) {
         out << sep << prefix << "i:" << _initProgress;
     }
-    if (_startTimestamp != 0) {
-        out << sep << prefix << "t:" << _startTimestamp;
+    if (_startTimestamp != vespalib::system_time()) {
+        out << sep << prefix << "t:" << vespalib::count_s(_startTimestamp.time_since_epoch());
     }
     if (includeDescription && ! _description.empty()) {
         out << sep << prefix << "m:"
@@ -190,7 +188,7 @@ NodeState::serialize(vespalib::asciistream & out, vespalib::stringref prefix,
 void
 NodeState::setState(const State& state)
 {
-    if (_type != 0) {
+    if (_type != nullptr) {
         // We don't know whether you want to store reported, wanted or
         // current node state, so we must accept any.
         if (!state.validReportedNodeState(*_type)
@@ -225,7 +223,7 @@ NodeState::setCapacity(vespalib::Double capacity)
                 "must be a positive floating point number";
         throw vespalib::IllegalArgumentException(ost.str(), VESPA_STRLOC);
     }
-    if (_type != 0 && *_type != NodeType::STORAGE) {
+    if (_type != nullptr && *_type != NodeType::STORAGE) {
         throw vespalib::IllegalArgumentException(
                 "Capacity only make sense for storage nodes.", VESPA_STRLOC);
     }
@@ -245,7 +243,7 @@ NodeState::setInitProgress(vespalib::Double initProgress)
 }
 
 void
-NodeState::setStartTimestamp(uint64_t startTimestamp)
+NodeState::setStartTimestamp(vespalib::system_time startTimestamp)
 {
     _startTimestamp = startTimestamp;
 }
@@ -270,10 +268,10 @@ NodeState::print(std::ostream& out, bool verbose,
     if (*_state == State::INITIALIZING) {
         out << ", init progress " << _initProgress;
     }
-    if (_startTimestamp != 0) {
-        out << ", start timestamp " << _startTimestamp;
+    if (_startTimestamp != vespalib::system_time()) {
+        out << ", start timestamp " << vespalib::to_string(_startTimestamp);
     }
-    if (_description.size() > 0) {
+    if (!_description.empty()) {
         out << ": " << _description;
     }
 }
@@ -317,7 +315,7 @@ NodeState::similarTo(const NodeState& other) const
 void
 NodeState::verifySupportForNodeType(const NodeType& type) const
 {
-    if (_type != 0 && *_type == type) return;
+    if (_type != nullptr && *_type == type) return;
     if (!_state->validReportedNodeState(type)
         && !_state->validWantedNodeState(type))
     {
@@ -357,8 +355,8 @@ NodeState::getTextualDifference(const NodeState& other) const {
         }
     }
     if (_startTimestamp != other._startTimestamp) {
-        source << ", start timestamp " << _startTimestamp;
-        target << ", start timestamp " << other._startTimestamp;
+        source << ", start timestamp " << vespalib::to_string(_startTimestamp);
+        target << ", start timestamp " << vespalib::to_string(other._startTimestamp);
     }
 
     if (source.str().length() < 2 || target.str().length() < 2) {
diff --git a/vdslib/src/vespa/vdslib/state/nodestate.h b/vdslib/src/vespa/vdslib/state/nodestate.h
index 541395e15cb..4fb035b6dcd 100644
--- a/vdslib/src/vespa/vdslib/state/nodestate.h
+++ b/vdslib/src/vespa/vdslib/state/nodestate.h
@@ -13,6 +13,7 @@
 #include "state.h"
 #include <vespa/document/util/printable.h>
 #include <vespa/vespalib/objects/floatingpointtype.h>
+#include <vespa/vespalib/util/time.h>
 #include <memory>
 
 namespace storage::lib {
@@ -25,7 +26,7 @@ class NodeState : public document::Printable
     vespalib::Double _capacity;
     vespalib::Double _initProgress;
     uint32_t _minUsedBits;
-    uint64_t _startTimestamp;
+    vespalib::system_time _startTimestamp;
 
 public:
     using CSP = std::shared_ptr<const NodeState>;
@@ -43,8 +44,8 @@ public:
               vespalib::stringref description = "",
               double capacity = 1.0);
     /** Set type if you want to verify that content fit with the given type. */
-    NodeState(vespalib::stringref serialized, const NodeType* nodeType = 0);
-    ~NodeState();
+    explicit NodeState(vespalib::stringref serialized, const NodeType* nodeType = nullptr);
+    ~NodeState() override;
 
     /**
      * Setting prefix to something implies using this function to write a
@@ -54,26 +55,27 @@ public:
     void serialize(vespalib::asciistream & out, vespalib::stringref prefix = "",
                    bool includeDescription = true) const;
 
-    const State& getState() const { return *_state; }
-    vespalib::Double getCapacity() const { return _capacity; }
-    uint32_t getMinUsedBits() const { return _minUsedBits; }
-    vespalib::Double getInitProgress() const { return _initProgress; }
-    const vespalib::string& getDescription() const { return _description; }
-    uint64_t getStartTimestamp() const { return _startTimestamp; }
+    [[nodiscard]] const State& getState() const { return *_state; }
+    [[nodiscard]] vespalib::Double getCapacity() const { return _capacity; }
+    [[nodiscard]] uint32_t getMinUsedBits() const { return _minUsedBits; }
+    [[nodiscard]] vespalib::Double getInitProgress() const { return _initProgress; }
+    [[nodiscard]] const vespalib::string& getDescription() const { return _description; }
+    [[nodiscard]] vespalib::system_time getStartTimestamp() const { return _startTimestamp; }
 
     void setState(const State& state);
     void setCapacity(vespalib::Double capacity);
     void setMinUsedBits(uint32_t usedBits);
     void setInitProgress(vespalib::Double initProgress);
-    void setStartTimestamp(uint64_t startTimestamp);
+    void setStartTimestamp(vespalib::system_time startTimestamp);
     void setDescription(vespalib::stringref desc) { _description = desc; }
 
     void print(std::ostream& out, bool verbose,
                const std::string& indent) const override;
     bool operator==(const NodeState& other) const;
-    bool operator!=(const NodeState& other) const
-        { return !(operator==(other)); }
-    bool similarTo(const NodeState& other) const;
+    bool operator!=(const NodeState& other) const {
+        return !(operator==(other));
+    }
+    [[nodiscard]] bool similarTo(const NodeState& other) const;
 
     /**
      * Verify that the contents of this object fits with the given nodetype.