Merge pull request #25888 from vespa-engine/balder/gc-millisconds

GC MilliSeconds in favor of c++11 time/duration.

11 files changed, 126 insertions, 222 deletions

diff --git a/storage/src/tests/storageframework/clock/timetest.cpp b/storage/src/tests/storageframework/clock/timetest.cpp
index 9dbcdd409d8..0f18dc9ed3a 100644
--- a/storage/src/tests/storageframework/clock/timetest.cpp
+++ b/storage/src/tests/storageframework/clock/timetest.cpp
@@ -9,11 +9,7 @@ namespace storage::framework::defaultimplementation {
 TEST(TimeTest, testBasics)
 {
 
-    MilliSecTime timeMillis(1000);
-    EXPECT_EQ(uint64_t(1000), timeMillis.getTime());
-
-    MicroSecTime timeMicros = timeMillis.getMicros();
-    EXPECT_EQ(timeMillis, timeMicros.getMillis());
+    MicroSecTime timeMicros(1000*1000);
 
     MicroSecTime timeMicros2 = timeMicros;
     EXPECT_EQ(timeMicros2, timeMicros);
@@ -23,11 +19,6 @@ TEST(TimeTest, testBasics)
     timeMicros2 -= MicroSecTime(30000);
     EXPECT_LT(timeMicros2, timeMicros);
     EXPECT_GT(timeMicros, timeMicros2);
-    timeMicros2 += MicroSecTime(55000);
-
-    MilliSecTime timeMillis2 = timeMicros2.getMillis();
-    EXPECT_GT(timeMillis2, timeMillis);
-    EXPECT_EQ(uint64_t(1050), timeMillis2.getTime());
 }
 
 TEST(TimeTest, testCreatedFromClock)
@@ -35,7 +26,6 @@ TEST(TimeTest, testCreatedFromClock)
     defaultimplementation::FakeClock clock;
     clock.setAbsoluteTimeInSeconds(600);
 
-    EXPECT_EQ(MilliSecTime(600 * 1000), MilliSecTime(clock));
     EXPECT_EQ(MicroSecTime(600 * 1000 * 1000), MicroSecTime(clock));
 }
 
@@ -45,7 +35,6 @@ TEST(TimeTest, canAssignMicrosecondResolutionTimeToFakeClock)
     clock.setAbsoluteTimeInMicroSeconds(1234567); // 1.234567 seconds
 
     // All non-microsec time points must necessarily be truncated.
-    EXPECT_EQ(MilliSecTime(1234), MilliSecTime(clock));
     EXPECT_EQ(MicroSecTime(1234567), MicroSecTime(clock));
 }
 
diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp
index 6d36abc896e..73142a624c4 100644
--- a/storage/src/vespa/storage/visiting/visitor.cpp
+++ b/storage/src/vespa/storage/visiting/visitor.cpp
@@ -175,21 +175,23 @@ Visitor::Visitor(StorageComponent& component)
       _bucketStates(),
       _calledStartingVisitor(false),
       _calledCompletedVisitor(false),
-      _startTime(_component.getClock().getTimeInMicros()),
+      _startTime(_component.getClock().getMonotonicTime()),
       _hasSentReply(false),
       _docBlockSize(1024),
       _memoryUsageLimit(UINT32_MAX),
-      _docBlockTimeout(180 * 1000),
-      _visitorInfoTimeout(60 * 1000),
-      _serialNumber(0),
+      _docBlockTimeout(180s),
+      _visitorInfoTimeout(60s),
       _traceLevel(0),
       _ownNodeIndex(0xffff),
       _visitorCmdId(0),
       _visitorId(0),
       _priority(api::StorageMessage::NORMAL),
       _result(api::ReturnCode::OK),
+      _recentlySentErrorMessages(),
+      _timeToDie(vespalib::steady_time::max()),
+      _hitCounter(),
       _trace(DEFAULT_TRACE_MEMORY_LIMIT),
-      _messageHandler(0),
+      _messageHandler(nullptr),
       _id(),
       _controlDestination(),
       _dataDestination(),
@@ -211,14 +213,12 @@ Visitor::sendMessage(documentapi::DocumentMessage::UP cmd)
 
     cmd->setPriority(_documentPriority);
 
-    framework::MicroSecTime time(_component.getClock().getTimeInMicros());
+    vespalib::steady_time time = _component.getClock().getMonotonicTime();
 
-    if (time + _docBlockTimeout.getMicros() > _timeToDie) {
-        cmd->setTimeRemaining(std::chrono::milliseconds((_timeToDie > time)
-                              ? (_timeToDie - time).getMillis().getTime()
-                              : 0));
+    if ((time + _docBlockTimeout) > _timeToDie) {
+        cmd->setTimeRemaining((_timeToDie > time) ? _timeToDie - time : vespalib::duration::zero());
     } else {
-        cmd->setTimeRemaining(std::chrono::milliseconds(_docBlockTimeout.getTime()));
+        cmd->setTimeRemaining(_docBlockTimeout);
     }
     cmd->getTrace().setLevel(_traceLevel);
 
@@ -233,36 +233,25 @@ Visitor::sendDocumentApiMessage(VisitorTarget::MessageMeta& msgMeta) {
     if (_messageSession->pending() >= _visitorOptions._maxPending
         && cmd.getType() != documentapi::DocumentProtocol::MESSAGE_VISITORINFO)
     {
-        MBUS_TRACE(cmd.getTrace(), 5, vespalib::make_string(
-                           "Enqueueing message because the visitor already "
-                           "had %d pending messages",
-                           _visitorOptions._maxPending));
-
-        LOG(spam,
-            "Visitor '%s' enqueueing message with id %" PRIu64,
-            _id.c_str(),
-            msgMeta.messageId);
-        _visitorTarget._queuedMessages.insert(std::make_pair(
-                    framework::MicroSecTime(0), msgMeta.messageId));
+        MBUS_TRACE(cmd.getTrace(), 5,
+                   vespalib::make_string("Enqueueing message because the visitor already had %d pending messages",
+                                         _visitorOptions._maxPending));
+
+        LOG(spam, "Visitor '%s' enqueueing message with id %" PRIu64,
+            _id.c_str(), msgMeta.messageId);
+        _visitorTarget._queuedMessages.insert(std::make_pair(vespalib::steady_time::min(), msgMeta.messageId));
     } else {
-        LOG(spam,
-            "Visitor '%s' immediately sending message '%s' with id %" PRIu64,
-            _id.c_str(),
-            cmd.toString().c_str(),
-            msgMeta.messageId);
+        LOG(spam, "Visitor '%s' immediately sending message '%s' with id %" PRIu64,
+            _id.c_str(), cmd.toString().c_str(), msgMeta.messageId);
         cmd.setContext(msgMeta.messageId);
         mbus::Result res(_messageSession->send(std::move(msgMeta.message)));
         if (res.isAccepted()) {
             _visitorTarget._pendingMessages.insert(msgMeta.messageId);
         } else {
-            LOG(warning,
-                "Visitor '%s' failed to send DocumentAPI message: %s",
-                _id.c_str(),
-                res.getError().toString().c_str());
-            api::ReturnCode returnCode(
-                    static_cast<api::ReturnCode::Result>(
-                            res.getError().getCode()),
-                    res.getError().getMessage());
+            LOG(warning, "Visitor '%s' failed to send DocumentAPI message: %s",
+                _id.c_str(), res.getError().toString().c_str());
+            api::ReturnCode returnCode(static_cast<api::ReturnCode::Result>(res.getError().getCode()),
+                                       res.getError().getMessage());
             fail(returnCode, true);
             close();
         }
@@ -278,7 +267,7 @@ Visitor::sendInfoMessage(documentapi::VisitorInfoMessage::UP cmd)
     if (_controlDestination->toString().length()) {
         cmd->setRoute(*_controlDestination);
         cmd->setPriority(_documentPriority);
-        cmd->setTimeRemaining(std::chrono::milliseconds(_visitorInfoTimeout.getTime()));
+        cmd->setTimeRemaining(_visitorInfoTimeout);
         auto& msgMeta = _visitorTarget.insertMessage(std::move(cmd));
         sendDocumentApiMessage(msgMeta);
     }
@@ -440,22 +429,17 @@ Visitor::shouldReportProblemToClient(const api::ReturnCode& code, size_t retryCo
 void
 Visitor::reportProblem(const std::string& problem)
 {
-    framework::MicroSecTime time(_component.getClock().getTimeInMicros());
-    std::map<std::string, framework::MicroSecTime>::iterator it(
-            _recentlySentErrorMessages.find(problem));
+    vespalib::steady_time now = _component.getClock().getMonotonicTime();
+    auto it = _recentlySentErrorMessages.find(problem);
         // Ignore errors already reported last minute
-    if (it != _recentlySentErrorMessages.end() &&
-        it->second + framework::MicroSecTime(60*1000*1000) > time)
-    {
+    if ((it != _recentlySentErrorMessages.end()) && ((it->second + 60s) > now)) {
         return;
     }
+
     LOG(debug, "Visitor '%s' sending VisitorInfo with message \"%s\" to %s",
-        _id.c_str(),
-        problem.c_str(),
-        _controlDestination->toString().c_str());
-    _recentlySentErrorMessages[problem] = time;
-    documentapi::VisitorInfoMessage::UP cmd(
-            new documentapi::VisitorInfoMessage());
+        _id.c_str(), problem.c_str(), _controlDestination->toString().c_str());
+    _recentlySentErrorMessages[problem] = now;
+    auto cmd = std::make_unique<documentapi::VisitorInfoMessage>();
     cmd->setErrorMessage(problem);
     sendInfoMessage(std::move(cmd));
 
@@ -511,36 +495,46 @@ Visitor::start(api::VisitorId id, api::StorageMessage::Id cmdId,
         _buckets.size(),
         _visitorOptions._fromTime.getTime(),
         _visitorOptions._toTime.getTime(),
-        (buckets.size() > 0 ? _buckets[0].toString().c_str() : ""),
+        (!buckets.empty() ? _buckets[0].toString().c_str() : ""),
         _visitorOptions._maxPending,
         (_visitorOptions._visitRemoves ? "true" : "false"),
         _visitorOptions._fieldSet.c_str());
 }
 
+namespace {
+
+vespalib::steady_time
+capped_future(vespalib::steady_time time, vespalib::duration duration) {
+    vespalib::steady_time future = time + duration;
+    return (future < time)
+        ? vespalib::steady_time::max()
+        : future;
+}
+
+}
+
 void
 Visitor::attach(std::shared_ptr<api::CreateVisitorCommand> initiatingCmd,
                 const mbus::Route& controlAddress,
                 const mbus::Route& dataAddress,
-                framework::MilliSecTime timeout)
+                vespalib::duration timeout)
 {
     _priority = initiatingCmd->getPriority();
-    _timeToDie = _component.getClock().getTimeInMicros() + timeout.getMicros();
+    _timeToDie = capped_future(_component.getClock().getMonotonicTime(), timeout);
     if (_initiatingCmd.get()) {
         std::shared_ptr<api::StorageReply> reply(_initiatingCmd->makeReply());
         reply->setResult(api::ReturnCode::ABORTED);
         _messageHandler->send(reply);
     }
-    _initiatingCmd = initiatingCmd;
+    _initiatingCmd = std::move(initiatingCmd);
     _traceLevel = _initiatingCmd->getTrace().getLevel();
     {
         // Set new address
         _controlDestination = std::make_unique<mbus::Route>(controlAddress);
         _dataDestination = std::make_unique<mbus::Route>(dataAddress);
     }
-    LOG(debug, "Visitor '%s' has control destination %s and data "
-               "destination %s.",
-        _id.c_str(), _controlDestination->toString().c_str(),
-        _dataDestination->toString().c_str());
+    LOG(debug, "Visitor '%s' has control destination %s and data destination %s.",
+        _id.c_str(), _controlDestination->toString().c_str(), _dataDestination->toString().c_str());
     if (!_calledStartingVisitor) {
         _calledStartingVisitor = true;
         try{
@@ -598,11 +592,8 @@ Visitor::handleDocumentApiReply(mbus::Reply::UP reply, VisitorThreadMetrics& met
     auto meta = _visitorTarget.releaseMetaForMessageId(messageId);
 
     if (!reply->hasErrors()) {
-        metrics.averageMessageSendTime.addValue(
-                (message->getTimeRemaining() - message->getTimeRemainingNow()).count() / 1000.0);
-        LOG(debug, "Visitor '%s' reply %s for message ID %" PRIu64 " was OK", _id.c_str(),
-            reply->toString().c_str(), messageId);
-
+        metrics.averageMessageSendTime.addValue(vespalib::to_s(message->getTimeRemaining() - message->getTimeRemainingNow()));
+        LOG(debug, "Visitor '%s' reply %s for message ID %" PRIu64 " was OK", _id.c_str(), reply->toString().c_str(), messageId);
         continueVisitor();
         return;
     }
@@ -640,13 +631,10 @@ Visitor::handleDocumentApiReply(mbus::Reply::UP reply, VisitorThreadMetrics& met
 
     // Tag time for later resending. nextSendAttemptTime != 0 indicates
     // that the message is not pending, but should be sent later.
-    framework::MicroSecTime delay(
-            (1 << std::min(12u, meta.retryCount)) * 10000);
+    vespalib::duration delay(std::chrono::milliseconds(1 << std::min(12u, meta.retryCount)) * 10);
 
     _visitorTarget.reinsertMeta(std::move(meta));
-    _visitorTarget._queuedMessages.insert(
-            std::make_pair(_component.getClock().getTimeInMicros() + delay,
-                           messageId));
+    _visitorTarget._queuedMessages.emplace(_component.getClock().getMonotonicTime() + delay, messageId);
     if (shouldReportProblemToClient(returnCode, retryCount)) {
         reportProblem(returnCode);
     }
@@ -655,16 +643,12 @@ Visitor::handleDocumentApiReply(mbus::Reply::UP reply, VisitorThreadMetrics& met
     // Max delay is then 40 seconds. At which time, retrying should not
     // use up that much resources.
     // 20, 40, 80, 160, 320, 640, 1280, 2560, 5120, 10240, 20480, 40960
-    LOG(debug, "Failed to send message from visitor '%s', due to "
-        "%s. Resending in %" PRIu64 " ms",
-        _id.c_str(), returnCode.toString().c_str(),
-        delay.getMillis().getTime());
+    LOG(debug, "Failed to send message from visitor '%s', due to %s. Resending in %" PRIu64 " ms",
+        _id.c_str(), returnCode.toString().c_str(), vespalib::count_ms(delay));
 }
 
 void
-Visitor::onCreateIteratorReply(
-        const std::shared_ptr<CreateIteratorReply>& reply,
-        VisitorThreadMetrics& /*metrics*/)
+Visitor::onCreateIteratorReply(const std::shared_ptr<CreateIteratorReply>& reply, VisitorThreadMetrics&)
 {
     auto it = _bucketStates.rbegin();
 
@@ -787,7 +771,7 @@ Visitor::onGetIterReply(const std::shared_ptr<GetIterReply>& reply, VisitorThrea
 }
 
 void
-Visitor::sendDueQueuedMessages(framework::MicroSecTime timeNow)
+Visitor::sendDueQueuedMessages(vespalib::steady_time timeNow)
 {
     // Assuming few messages in sent queue, so cheap to go through all.
     while (!_visitorTarget._queuedMessages.empty()
@@ -811,46 +795,35 @@ Visitor::continueVisitor()
         transitionTo(STATE_COMPLETED);
         return false;
     }
-    framework::MicroSecTime time(_component.getClock().getTimeInMicros());
-    if (time > _timeToDie) { // If we have timed out, just shut down.
+    vespalib::steady_time now =_component.getClock().getMonotonicTime();
+    if (now > _timeToDie) { // If we have timed out, just shut down.
         if (isRunning()) {
             LOG(debug, "Visitor %s timed out. Closing it.", _id.c_str());
-            fail(api::ReturnCode(api::ReturnCode::ABORTED,
-                                 "Visitor timed out"));
+            fail(api::ReturnCode(api::ReturnCode::ABORTED, "Visitor timed out"));
             close();
         }
         return false;
     }
 
-    sendDueQueuedMessages(time);
+    sendDueQueuedMessages(now);
 
     // No need to do more work if we already have maximum pending towards data handler
-    if (_messageSession->pending() + _visitorTarget._queuedMessages.size()
-        >= _visitorOptions._maxPending)
-    {
-        LOG(spam, "Number of pending messages (%zu pending, %zu queued) "
-            "already >= max pending (%u)",
-            _visitorTarget._pendingMessages.size(),
-            _visitorTarget._queuedMessages.size(),
-            _visitorOptions._maxPending);
+    if (_messageSession->pending() + _visitorTarget._queuedMessages.size() >= _visitorOptions._maxPending) {
+        LOG(spam, "Number of pending messages (%zu pending, %zu queued) already >= max pending (%u)",
+            _visitorTarget._pendingMessages.size(), _visitorTarget._queuedMessages.size(), _visitorOptions._maxPending);
         return false;
     }
 
     if (_visitorTarget.getMemoryUsage() >= _memoryUsageLimit) {
-        LOG(spam,
-            "Visitor already using maximum amount of memory "
-            "(using %u, limit %u)",
-            _visitorTarget.getMemoryUsage(),
-            _memoryUsageLimit);
+        LOG(spam, "Visitor already using maximum amount of memory (using %u, limit %u)",
+            _visitorTarget.getMemoryUsage(), _memoryUsageLimit);
         return false;
     }
 
     // If there are no more buckets to visit and no pending messages
     // to the client, mark visitor as complete.
     if (!getIterators()) {
-        if (_visitorTarget._pendingMessages.empty()
-            && _visitorTarget._queuedMessages.empty())
-        {
+        if (_visitorTarget._pendingMessages.empty() && _visitorTarget._queuedMessages.empty()) {
             if (isRunning()) {
                 LOG(debug, "Visitor '%s' has not been aborted", _id.c_str());
                 if (!_calledCompletedVisitor) {
@@ -970,14 +943,14 @@ Visitor::getStatus(std::ostream& out, bool verbose) const
         out << "<tr><td>Trace level</td><td>"
             << _traceLevel << "</td></tr>\n";
 
-        framework::MicroSecTime time(_component.getClock().getTimeInMicros());
+        vespalib::steady_time time = _component.getClock().getMonotonicTime();
 
         out << "<tr><td>Time left until timeout</td><td>";
         if (time <= _timeToDie) {
-            out << (_timeToDie - time).getMillis().getTime() << " ms";
+            out << vespalib::count_ms(_timeToDie - time) << " ms";
         } else {
             out << "(expired "
-                << (time - _timeToDie).getMillis().getTime()
+                << vespalib::count_ms(time - _timeToDie)
                 << " ms ago)";
         }
         out << "</td></tr>\n";
@@ -985,19 +958,19 @@ Visitor::getStatus(std::ostream& out, bool verbose) const
     out << "</table>\n";
 
     out << "<h4>Buckets to visit</h4>";
-    for (uint32_t i=0; i<_buckets.size(); ++i) {
-        out << _buckets[i] << "\n<br>";
+    for (auto bucket : _buckets) {
+        out << bucket << "\n<br>";
     }
 
     out << "<h4>States of buckets currently being visited</h4>";
-    if (_bucketStates.size() == 0) {
+    if (_bucketStates.empty()) {
         out << "None\n";
     }
-    for (auto* state : _bucketStates) {
+    for (const auto* state : _bucketStates) {
         out << "  " << *state << "<br>\n";
     }
 
-    std::unordered_map<uint64_t, framework::MicroSecTime> idToSendTime;
+    std::unordered_map<uint64_t, vespalib::steady_time> idToSendTime;
     for (auto& sendTimeToId : _visitorTarget._queuedMessages) {
         idToSendTime[sendTimeToId.second] = sendTimeToId.first;
     }
@@ -1021,7 +994,7 @@ Visitor::getStatus(std::ostream& out, bool verbose) const
         auto queued = idToSendTime.find(idAndMeta.first);
         if (queued != idToSendTime.end()) {
             out << "Scheduled for sending at timestamp "
-                << (queued->second.getSeconds());
+                << vespalib::to_s(vespalib::to_utc(queued->second).time_since_epoch());
         }
 
         out << "<br/>\n";
diff --git a/storage/src/vespa/storage/visiting/visitor.h b/storage/src/vespa/storage/visiting/visitor.h
index d152e1b721b..c2da55b2edc 100644
--- a/storage/src/vespa/storage/visiting/visitor.h
+++ b/storage/src/vespa/storage/visiting/visitor.h
@@ -228,12 +228,11 @@ private:
 
         // Maps from time sent to message to send.
         // Value refers to message id (key in _messageMeta).
-        using MessageQueue = std::multimap<framework::MicroSecTime, uint64_t>;
+        using MessageQueue = std::multimap<vespalib::steady_time, uint64_t>;
 
         MessageQueue _queuedMessages;
 
-        MessageMeta& insertMessage(
-                std::unique_ptr<documentapi::DocumentMessage>);
+        MessageMeta& insertMessage(std::unique_ptr<documentapi::DocumentMessage>);
         /**
          * Preconditions:
          *   msgId exists as a key in _messageMeta
@@ -272,15 +271,14 @@ private:
     bool _calledStartingVisitor;
     bool _calledCompletedVisitor;
 
-    framework::MicroSecTime _startTime;
+    vespalib::steady_time _startTime;
 
     bool _hasSentReply;
 
     uint32_t _docBlockSize;
     uint32_t _memoryUsageLimit;
-    framework::MilliSecTime _docBlockTimeout;
-    framework::MilliSecTime _visitorInfoTimeout;
-    uint32_t _serialNumber;
+    vespalib::duration _docBlockTimeout;
+    vespalib::duration _visitorInfoTimeout;
     // Keep trace level independent of _initiatingCmd, since we might want to
     // print out the trace level even after the command's ownership has been
     // released away from us.
@@ -294,17 +292,14 @@ private:
     api::StorageMessage::Priority _priority;
 
     api::ReturnCode _result;
-    std::map<std::string, framework::MicroSecTime> _recentlySentErrorMessages;
-    framework::MicroSecTime _timeToDie; // Visitor will time out to distributor at this time
+    std::map<std::string, vespalib::steady_time> _recentlySentErrorMessages;
+    vespalib::steady_time _timeToDie; // Visitor will time out to distributor at this time
 
     std::unique_ptr<HitCounter> _hitCounter;
 
     static constexpr size_t DEFAULT_TRACE_MEMORY_LIMIT = 65536;
     MemoryBoundedTrace _trace;
 
-    Visitor(const Visitor &);
-    Visitor& operator=(const Visitor &);
-
 protected:
     // These variables should not be altered after visitor starts. This not
     // controlled by locks.
@@ -341,36 +336,35 @@ protected:
     [[nodiscard]] virtual bool remap_docapi_message_error_code(api::ReturnCode& in_out_code);
 public:
     using DocEntryList = std::vector<std::unique_ptr<spi::DocEntry>>;
-    Visitor(StorageComponent& component);
+    Visitor(const Visitor &) = delete;
+    Visitor& operator=(const Visitor &) = delete;
+    explicit Visitor(StorageComponent& component);
     virtual ~Visitor();
 
-    framework::MicroSecTime getStartTime() const { return _startTime; }
-    api::VisitorId getVisitorId() const { return _visitorId; }
-    const std::string& getVisitorName() const { return _id; }
-    const mbus::Route* getControlDestination() const {
+    [[nodiscard]] vespalib::steady_time getStartTime() const { return _startTime; }
+    [[nodiscard]] api::VisitorId getVisitorId() const { return _visitorId; }
+    [[nodiscard]] const std::string& getVisitorName() const { return _id; }
+    [[nodiscard]] const mbus::Route* getControlDestination() const {
         return _controlDestination.get(); // Can't be null if attached
     }
-    const mbus::Route* getDataDestination() const {
+    [[nodiscard]] const mbus::Route* getDataDestination() const {
         return _dataDestination.get(); // Can't be null if attached
     }
 
-    void setMaxPending(unsigned int maxPending)
-        { _visitorOptions._maxPending = maxPending; }
+    void setMaxPending(unsigned int maxPending) { _visitorOptions._maxPending = maxPending; }
 
     void setFieldSet(const std::string& fieldSet) { _visitorOptions._fieldSet = fieldSet; }
     void visitRemoves() { _visitorOptions._visitRemoves = true; }
     void setDocBlockSize(uint32_t size) { _docBlockSize = size; }
-    uint32_t getDocBlockSize() const { return _docBlockSize; }
+    [[nodiscard]] uint32_t getDocBlockSize() const { return _docBlockSize; }
     void setMemoryUsageLimit(uint32_t limit) noexcept {
         _memoryUsageLimit = limit;
     }
-    uint32_t getMemoryUsageLimit() const noexcept {
+    [[nodiscard]] uint32_t getMemoryUsageLimit() const noexcept {
         return _memoryUsageLimit;
     }
-    void setDocBlockTimeout(framework::MilliSecTime timeout)
-        { _docBlockTimeout = timeout; }
-    void setVisitorInfoTimeout(framework::MilliSecTime timeout)
-        { _visitorInfoTimeout = timeout; }
+    void setDocBlockTimeout(vespalib::duration timeout) { _docBlockTimeout = timeout; }
+    void setVisitorInfoTimeout(vespalib::duration timeout) { _visitorInfoTimeout = timeout; }
     void setOwnNodeIndex(uint16_t nodeIndex) { _ownNodeIndex = nodeIndex; }
     void setBucketSpace(document::BucketSpace bucketSpace) { _bucketSpace = bucketSpace; }
 
@@ -462,19 +456,15 @@ public:
     void attach(std::shared_ptr<api::CreateVisitorCommand> initiatingCmd,
                 const mbus::Route& controlAddress,
                 const mbus::Route& dataAddress,
-                framework::MilliSecTime timeout);
+                vespalib::duration timeout);
 
-    void handleDocumentApiReply(mbus::Reply::UP reply,
-                                VisitorThreadMetrics& metrics);
+    void handleDocumentApiReply(mbus::Reply::UP reply, VisitorThreadMetrics& metrics);
 
-    void onGetIterReply(const std::shared_ptr<GetIterReply>& reply,
-                        VisitorThreadMetrics& metrics);
+    void onGetIterReply(const std::shared_ptr<GetIterReply>& reply, VisitorThreadMetrics& metrics);
 
-    void onCreateIteratorReply(
-            const std::shared_ptr<CreateIteratorReply>& reply,
-            VisitorThreadMetrics& metrics);
+    void onCreateIteratorReply(const std::shared_ptr<CreateIteratorReply>& reply, VisitorThreadMetrics& metrics);
 
-    bool failed() const { return _result.failed(); }
+    [[nodiscard]] bool failed() const { return _result.failed(); }
 
     /**
      * This function will check current state and make the visitor move on, if
@@ -545,7 +535,7 @@ private:
      * Ensures number of resulting pending messages from visitor does not
      * violate maximum pending options.
      */
-    void sendDueQueuedMessages(framework::MicroSecTime timeNow);
+    void sendDueQueuedMessages(vespalib::steady_time timeNow);
 
     /**
      * Whether visitor should enable and forward message bus traces for messages
diff --git a/storage/src/vespa/storage/visiting/visitorthread.cpp b/storage/src/vespa/storage/visiting/visitorthread.cpp
index ed26ae59bcc..3e72cea830c 100644
--- a/storage/src/vespa/storage/visiting/visitorthread.cpp
+++ b/storage/src/vespa/storage/visiting/visitorthread.cpp
@@ -89,7 +89,8 @@ VisitorThread::VisitorThread(uint32_t threadIndex,
       _defaultPendingMessages(0),
       _defaultDocBlockSize(0),
       _visitorMemoryUsageLimit(UINT32_MAX),
-      _defaultDocBlockTimeout(180000),
+      _defaultDocBlockTimeout(180s),
+      _defaultVisitorInfoTimeout(60s),
       _timeBetweenTicks(1000),
       _component(componentRegister, getThreadName(threadIndex)),
       _messageSessionFactory(messageSessionFac),
@@ -280,11 +281,11 @@ VisitorThread::tick()
 void
 VisitorThread::close()
 {
-    framework::MicroSecTime closeTime(_component.getClock().getTimeInMicros());
+    vespalib::steady_time closeTime = _component.getClock().getMonotonicTime();
 
     Visitor& v = *_currentlyRunningVisitor->second;
 
-    _metrics.averageVisitorLifeTime.addValue((closeTime - v.getStartTime()).getMillis().getTime());
+    _metrics.averageVisitorLifeTime.addValue(vespalib::count_ms(closeTime - v.getStartTime()));
     v.finalize();
     _messageSender.closed(_currentlyRunningVisitor->first);
     if (v.failed()) {
@@ -508,8 +509,7 @@ VisitorThread::onCreateVisitor(
                            _messageSender,
                            std::move(messageSession),
                            documentPriority);
-            visitor->attach(cmd, *controlAddress, *dataAddress,
-                            framework::MilliSecTime(vespalib::count_ms(cmd->getTimeout())));
+            visitor->attach(cmd, *controlAddress, *dataAddress, cmd->getTimeout());
         } catch (std::exception& e) {
             // We don't handle exceptions from this code, as we've
             // added visitor to internal structs we'll end up calling
@@ -575,8 +575,8 @@ VisitorThread::onInternal(const std::shared_ptr<api::InternalCommand>& cmd)
                             _defaultPendingMessages,
                             _defaultDocBlockSize,
                             _visitorMemoryUsageLimit,
-                            _defaultDocBlockTimeout.getTime(),
-                            _defaultVisitorInfoTimeout.getTime(),
+                            vespalib::count_ms(_defaultDocBlockTimeout),
+                            vespalib::count_ms(_defaultVisitorInfoTimeout),
                             config.disconnectedvisitortimeout,
                             config.ignorenonexistingvisitortimelimit,
                             config.defaultparalleliterators,
@@ -588,14 +588,13 @@ VisitorThread::onInternal(const std::shared_ptr<api::InternalCommand>& cmd)
                    );
             }
             _disconnectedVisitorTimeout = config.disconnectedvisitortimeout;
-            _ignoreNonExistingVisitorTimeLimit
-                    = config.ignorenonexistingvisitortimelimit;
+            _ignoreNonExistingVisitorTimeLimit = config.ignorenonexistingvisitortimelimit;
             _defaultParallelIterators = config.defaultparalleliterators;
             _defaultPendingMessages = config.defaultpendingmessages;
             _defaultDocBlockSize = config.defaultdocblocksize;
             _visitorMemoryUsageLimit = config.visitorMemoryUsageLimit;
-            _defaultDocBlockTimeout.setTime(config.defaultdocblocktimeout);
-            _defaultVisitorInfoTimeout.setTime(config.defaultinfotimeout);
+            _defaultDocBlockTimeout = std::chrono::milliseconds(config.defaultdocblocktimeout);
+            _defaultVisitorInfoTimeout = std::chrono::milliseconds(config.defaultinfotimeout);
             if (_defaultParallelIterators < 1) {
                 LOG(config, "Cannot use value of defaultParallelIterators < 1");
                 _defaultParallelIterators = 1;
@@ -608,9 +607,9 @@ VisitorThread::onInternal(const std::shared_ptr<api::InternalCommand>& cmd)
                 LOG(config, "Refusing to use default block size less than 1k");
                 _defaultDocBlockSize = 1024;
             }
-            if (_defaultDocBlockTimeout.getTime() < 1) {
+            if (_defaultDocBlockTimeout < 1ms) {
                 LOG(config, "Cannot use value of defaultDocBlockTimeout < 1");
-                _defaultDocBlockTimeout.setTime(1);
+                _defaultDocBlockTimeout = 1ms;
             }
             break;
         }
@@ -710,7 +709,7 @@ VisitorThread::getStatus(vespalib::asciistream& out,
             << "<tr><td>Default DocBlock size</td><td>"
             << _defaultDocBlockSize << "</td></tr>\n"
             << "<tr><td>Default DocBlock timeout (ms)</td><td>"
-            << _defaultDocBlockTimeout.getTime() << "</td></tr>\n"
+            << vespalib::count_ms(_defaultDocBlockTimeout) << "</td></tr>\n"
             << "<tr><td>Visitor memory usage limit</td><td>"
             << _visitorMemoryUsageLimit << "</td></tr>\n"
             << "</table>\n";
diff --git a/storage/src/vespa/storage/visiting/visitorthread.h b/storage/src/vespa/storage/visiting/visitorthread.h
index 23a4a81d6a6..729b675df3a 100644
--- a/storage/src/vespa/storage/visiting/visitorthread.h
+++ b/storage/src/vespa/storage/visiting/visitorthread.h
@@ -83,8 +83,8 @@ class VisitorThread : public framework::Runnable,
     uint32_t _defaultPendingMessages;
     uint32_t _defaultDocBlockSize;
     uint32_t _visitorMemoryUsageLimit;
-    framework::MilliSecTime _defaultDocBlockTimeout;
-    framework::MilliSecTime _defaultVisitorInfoTimeout;
+    vespalib::duration _defaultDocBlockTimeout;
+    vespalib::duration _defaultVisitorInfoTimeout;
     std::atomic<uint32_t> _timeBetweenTicks;
     StorageComponent _component;
     std::unique_ptr<framework::Thread> _thread;
diff --git a/storage/src/vespa/storageframework/defaultimplementation/clock/fakeclock.h b/storage/src/vespa/storageframework/defaultimplementation/clock/fakeclock.h
index d234f432f2b..b97c2bd92c1 100644
--- a/storage/src/vespa/storageframework/defaultimplementation/clock/fakeclock.h
+++ b/storage/src/vespa/storageframework/defaultimplementation/clock/fakeclock.h
@@ -61,9 +61,6 @@ public:
     }
 
     framework::MicroSecTime getTimeInMicros() const override ;
-    framework::MilliSecTime getTimeInMillis() const override {
-        return getTimeInMicros().getMillis();
-    }
     framework::SecondTime getTimeInSeconds() const override {
         return getTimeInMicros().getSeconds();
     }
diff --git a/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.cpp b/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.cpp
index df6115aa416..2e2894de9d5 100644
--- a/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.cpp
+++ b/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.cpp
@@ -12,14 +12,6 @@ RealClock::getTimeInMicros() const {
     return MicroSecTime(mytime.tv_sec * 1000000llu + mytime.tv_usec);
 }
 
-MilliSecTime
-RealClock::getTimeInMillis() const {
-    struct timeval mytime;
-    gettimeofday(&mytime, 0);
-    return MilliSecTime(
-            mytime.tv_sec * 1000llu + mytime.tv_usec / 1000);
-}
-
 SecondTime
 RealClock::getTimeInSeconds() const {
     struct timeval mytime;
diff --git a/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.h b/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.h
index de176a3e402..dc4884b4439 100644
--- a/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.h
+++ b/storage/src/vespa/storageframework/defaultimplementation/clock/realclock.h
@@ -15,7 +15,6 @@ namespace storage::framework::defaultimplementation {
 
 struct RealClock : public Clock {
     MicroSecTime getTimeInMicros() const override;
-    MilliSecTime getTimeInMillis() const override;
     SecondTime getTimeInSeconds() const override;
     vespalib::steady_time getMonotonicTime() const override;
     vespalib::system_time getSystemTime() const override;
diff --git a/storage/src/vespa/storageframework/generic/clock/clock.h b/storage/src/vespa/storageframework/generic/clock/clock.h
index e1f8419f069..21d6bc589a6 100644
--- a/storage/src/vespa/storageframework/generic/clock/clock.h
+++ b/storage/src/vespa/storageframework/generic/clock/clock.h
@@ -24,7 +24,6 @@ struct Clock {
     virtual ~Clock() = default;
 
     virtual MicroSecTime getTimeInMicros() const = 0;
-    virtual MilliSecTime getTimeInMillis() const = 0;
     virtual SecondTime getTimeInSeconds() const = 0;
 
     // Time point resolution is intentionally not defined here.
diff --git a/storage/src/vespa/storageframework/generic/clock/time.cpp b/storage/src/vespa/storageframework/generic/clock/time.cpp
index 7bf3fca3835..f5426901e4c 100644
--- a/storage/src/vespa/storageframework/generic/clock/time.cpp
+++ b/storage/src/vespa/storageframework/generic/clock/time.cpp
@@ -7,8 +7,7 @@
 #include <cassert>
 #include <sstream>
 
-namespace storage {
-namespace framework {
+namespace storage::framework {
 
 namespace {
     void detectUnit(uint64_t& val, const char* unit, uint64_t size,
@@ -17,7 +16,7 @@ namespace {
             uint64_t value = val / size;
             vespalib::string unitname = unit;
             if (value != 1) unitname += "s";
-            units.push_back(std::make_pair(value, unitname));
+            units.emplace_back(value, unitname);
             val -= value * size;
         }
     }
@@ -41,7 +40,7 @@ getTimeString(uint64_t microSecondTime, TimeFormat format)
             if (vals.empty()) { ost << "0 seconds"; }
         }
         if (vals.empty()) {
-            return vespalib::string(ost.str().c_str());
+            return ost.str();
         }
         ost << vals[0].first << " " << vals[0].second;
         for (uint32_t i=1; i<vals.size(); ++i) {
@@ -52,7 +51,7 @@ getTimeString(uint64_t microSecondTime, TimeFormat format)
             }
             ost << vals[i].first << " " << vals[i].second;
         }
-        return vespalib::string(ost.str().c_str());
+        return ost.str();
     }
     time_t secondTime = microSecondTime / 1000000;
     struct tm datestruct;
@@ -72,7 +71,7 @@ getTimeString(uint64_t microSecondTime, TimeFormat format)
     } else if (format == DATETIME_WITH_MICROS) {
         ost << '.' << std::setw(6) << micros;
     }
-    return vespalib::string(ost.str().c_str());
+    return ost.str();
 }
 
 uint64_t
@@ -82,11 +81,9 @@ getRawMicroTime(const Clock& clock)
 }
 
 template std::ostream& operator<< <MicroSecTime, 1>(std::ostream&, const Time<MicroSecTime, 1> &);
-template std::ostream& operator<< <MilliSecTime, 1000>(std::ostream&, const Time<MilliSecTime, 1000> &);
 template std::ostream& operator<< <SecondTime, 1000000>(std::ostream&, const Time<SecondTime, 1000000> &);
 
 template vespalib::asciistream& operator<< <MicroSecTime, 1>(vespalib::asciistream &, const Time<MicroSecTime, 1> &);
-template vespalib::asciistream& operator<< <MilliSecTime, 1000>(vespalib::asciistream &, const Time<MilliSecTime, 1000> &);
+template vespalib::asciistream& operator<< <SecondTime, 1000000>(vespalib::asciistream &, const Time<SecondTime, 1000000> &);
 
-} // framework
-} // storage
+}
diff --git a/storage/src/vespa/storageframework/generic/clock/time.h b/storage/src/vespa/storageframework/generic/clock/time.h
index 882ff58fb74..6f7cb490355 100644
--- a/storage/src/vespa/storageframework/generic/clock/time.h
+++ b/storage/src/vespa/storageframework/generic/clock/time.h
@@ -95,7 +95,6 @@ template<typename Type, int MPU>
 vespalib::asciistream& operator<<(vespalib::asciistream& out, const Time<Type, MPU>& t);
 
 struct MicroSecTime;
-struct MilliSecTime;
 
 /**
  * \class storage::framework::SecondTime
@@ -114,24 +113,6 @@ struct SecondTime : public Time<SecondTime, 1000000> {
 };
 
 /**
- * \class storage::framework::MilliSecTime
- * \ingroup clock
- *
- * \brief Wrapper class for a timestamp in milliseconds.
- *
- * To prevent errors where one passes time in one granularity to a function
- * requiring time in another granularity. This little wrapper class exist to
- * make sure that will conflict in types
- */
-struct MilliSecTime : public Time<MilliSecTime, 1000> {
-    explicit MilliSecTime(uint64_t t = 0) : Time<MilliSecTime, 1000>(t) {}
-    explicit MilliSecTime(const Clock& clock)
-        : Time<MilliSecTime, 1000>(getRawMicroTime(clock) / 1000) {}
-
-    [[nodiscard]] MicroSecTime getMicros() const;
-};
-
-/**
  * \class storage::framework::MicroSecTime
  * \ingroup clock
  *
@@ -146,14 +127,9 @@ struct MicroSecTime : public Time<MicroSecTime, 1> {
     explicit MicroSecTime(const Clock& clock)
         : Time<MicroSecTime, 1>(getRawMicroTime(clock)) {}
 
-    [[nodiscard]] MilliSecTime getMillis() const { return MilliSecTime(getTime() / 1000); }
     [[nodiscard]] SecondTime getSeconds() const { return SecondTime(getTime() / 1000000); }
 };
 
-inline MicroSecTime MilliSecTime::getMicros() const {
-    return MicroSecTime(getTime() * 1000);
-}
-
 inline MicroSecTime
 operator + (MicroSecTime a, MicroSecTime b) {
     MicroSecTime result(a);
@@ -161,13 +137,6 @@ operator + (MicroSecTime a, MicroSecTime b) {
     return result;
 }
 
-inline MilliSecTime
-operator + (MilliSecTime a, MilliSecTime b) {
-    MilliSecTime result(a);
-    result += b;
-    return result;
-}
-
 inline MicroSecTime
 operator - (MicroSecTime a, MicroSecTime b) {
     MicroSecTime result(a);