Use vespalib::steady_time for getMilliSecTime to ensure no wraps around and safer code.

3 files changed, 46 insertions, 60 deletions

diff --git a/storage/src/tests/common/metricstest.cpp b/storage/src/tests/common/metricstest.cpp
index 78fa32e24e5..76c6a9cf991 100644
--- a/storage/src/tests/common/metricstest.cpp
+++ b/storage/src/tests/common/metricstest.cpp
@@ -53,7 +53,6 @@ namespace {
         framework::Clock& _clock;
         explicit MetricClock(framework::Clock& c) : _clock(c) {}
         [[nodiscard]] time_t getTime() const override { return vespalib::count_s(_clock.getMonotonicTime().time_since_epoch()); }
-        [[nodiscard]] time_t getTimeInMilliSecs() const override { return vespalib::count_ms(_clock.getMonotonicTime().time_since_epoch()); }
     };
 }
 
@@ -85,10 +84,7 @@ void MetricsTest::SetUp() {
         _metricManager->registerMetric(guard, *_topSet);
     }
 
-    _metricsConsumer = std::make_unique<StatusMetricConsumer>(
-            _node->getComponentRegister(),
-            *_metricManager,
-            "status");
+    _metricsConsumer = std::make_unique<StatusMetricConsumer>(_node->getComponentRegister(), *_metricManager, "status");
 
     _filestorMetrics = std::make_shared<FileStorMetrics>();
     _filestorMetrics->initDiskMetrics(1, 1);
diff --git a/storage/src/tests/storageserver/statereportertest.cpp b/storage/src/tests/storageserver/statereportertest.cpp
index d6b528e5a25..2570b5232eb 100644
--- a/storage/src/tests/storageserver/statereportertest.cpp
+++ b/storage/src/tests/storageserver/statereportertest.cpp
@@ -54,7 +54,6 @@ struct MetricClock : public metrics::MetricManager::Timer
     framework::Clock& _clock;
     explicit MetricClock(framework::Clock& c) : _clock(c) {}
     [[nodiscard]] time_t getTime() const override { return vespalib::count_s(_clock.getMonotonicTime().time_since_epoch()); }
-    [[nodiscard]] time_t getTimeInMilliSecs() const override { return vespalib::count_ms(_clock.getMonotonicTime().time_since_epoch()); }
 };
 
 }
@@ -85,11 +84,8 @@ void StateReporterTest::SetUp() {
         _metricManager->registerMetric(guard, *_topSet);
     }
 
-    _stateReporter = std::make_unique<StateReporter>(
-            _node->getComponentRegister(),
-            *_metricManager,
-            _generationFetcher,
-            "status");
+    _stateReporter = std::make_unique<StateReporter>(_node->getComponentRegister(), *_metricManager,
+                                                     _generationFetcher, "status");
 
     _filestorMetrics = std::make_shared<FileStorMetrics>();
     _filestorMetrics->initDiskMetrics(1, 1);
@@ -125,20 +121,14 @@ vespalib::Slime slime; \
 #define ASSERT_GENERATION(jsonData, component, generation) \
 { \
     PARSE_JSON(jsonData); \
-    ASSERT_EQ( \
-            generation, \
-            slime.get()["config"][component]["generation"].asDouble()); \
+    ASSERT_EQ(generation, slime.get()["config"][component]["generation"].asDouble()); \
 }
 
 #define ASSERT_NODE_STATUS(jsonData, code, message) \
 { \
     PARSE_JSON(jsonData); \
-    ASSERT_EQ( \
-            vespalib::string(code), \
-            slime.get()["status"]["code"].asString().make_string()); \
-    ASSERT_EQ( \
-            vespalib::string(message), \
-            slime.get()["status"]["message"].asString().make_string()); \
+    ASSERT_EQ(vespalib::string(code), slime.get()["status"]["code"].asString().make_string()); \
+    ASSERT_EQ(vespalib::string(message), slime.get()["status"]["message"].asString().make_string()); \
 }
 
 #define ASSERT_METRIC_GET_PUT(jsonData, expGetCount, expPutCount) \
@@ -148,16 +138,11 @@ vespalib::Slime slime; \
     double putCount = -1; \
     size_t metricCount = slime.get()["metrics"]["values"].children(); \
     for (size_t j=0; j<metricCount; j++) { \
-        const vespalib::string name = slime.get()["metrics"]["values"][j]["name"] \
-                                 .asString().make_string(); \
-        if (name.compare("vds.filestor.allthreads.get.count") == 0) \
-        { \
-            getCount = slime.get()["metrics"]["values"][j]["values"]["count"] \
-                       .asDouble(); \
-        } else if (name.compare("vds.filestor.allthreads.put.count") == 0) \
-        { \
-            putCount = slime.get()["metrics"]["values"][j]["values"]["count"] \
-                       .asDouble(); \
+        const vespalib::string name = slime.get()["metrics"]["values"][j]["name"].asString().make_string(); \
+        if (name.compare("vds.filestor.allthreads.get.count") == 0) { \
+            getCount = slime.get()["metrics"]["values"][j]["values"]["count"].asDouble(); \
+        } else if (name.compare("vds.filestor.allthreads.put.count") == 0) { \
+            putCount = slime.get()["metrics"]["values"][j]["values"]["count"].asDouble(); \
         } \
     } \
     ASSERT_EQ(expGetCount, getCount); \
@@ -226,8 +211,7 @@ TEST_F(StateReporterTest, report_metrics) {
     for (uint32_t i = 0; i < 6; ++i) {
         _clock->addSecondsToTime(60);
         _metricManager->timeChangedNotification();
-        while (int64_t(_metricManager->getLastProcessedTime()) < vespalib::count_s(_clock->getMonotonicTime().time_since_epoch()))
-        {
+        while (int64_t(_metricManager->getLastProcessedTime()) < vespalib::count_s(_clock->getMonotonicTime().time_since_epoch())) {
             std::this_thread::sleep_for(1ms);
         }
     }
diff --git a/storage/src/vespa/storage/storageserver/storagenode.cpp b/storage/src/vespa/storage/storageserver/storagenode.cpp
index a09abb25f7a..9f8456afc37 100644
--- a/storage/src/vespa/storage/storageserver/storagenode.cpp
+++ b/storage/src/vespa/storage/storageserver/storagenode.cpp
@@ -33,34 +33,36 @@ namespace storage {
 
 namespace {
 
-    using vespalib::getLastErrorString;
+using vespalib::getLastErrorString;
 
-    void writePidFile(const vespalib::string& pidfile)
-    {
-        ssize_t rv = -1;
-        vespalib::string mypid = vespalib::make_string("%d\n", getpid());
-        size_t lastSlash = pidfile.rfind('/');
-        if (lastSlash != vespalib::string::npos) {
-            std::filesystem::create_directories(std::filesystem::path(pidfile.substr(0, lastSlash)));
-        }
-        int fd = open(pidfile.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0644);
-        if (fd != -1) {
-            rv = write(fd, mypid.c_str(), mypid.size());
-            close(fd);
-        }
-        if (rv < 1) {
-            LOG(warning, "Failed to write pidfile '%s': %s",
-                pidfile.c_str(), getLastErrorString().c_str());
-        }
+void
+writePidFile(const vespalib::string& pidfile)
+{
+    ssize_t rv = -1;
+    vespalib::string mypid = vespalib::make_string("%d\n", getpid());
+    size_t lastSlash = pidfile.rfind('/');
+    if (lastSlash != vespalib::string::npos) {
+        std::filesystem::create_directories(std::filesystem::path(pidfile.substr(0, lastSlash)));
+    }
+    int fd = open(pidfile.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0644);
+    if (fd != -1) {
+        rv = write(fd, mypid.c_str(), mypid.size());
+        close(fd);
     }
+    if (rv < 1) {
+        LOG(warning, "Failed to write pidfile '%s': %s",
+            pidfile.c_str(), getLastErrorString().c_str());
+    }
+}
 
-    void removePidFile(const vespalib::string& pidfile)
-    {
-        if (unlink(pidfile.c_str()) != 0) {
-            LOG(warning, "Failed to delete pidfile '%s': %s",
-                pidfile.c_str(), getLastErrorString().c_str());
-        }
+void
+removePidFile(const vespalib::string& pidfile)
+{
+    if (unlink(pidfile.c_str()) != 0) {
+        LOG(warning, "Failed to delete pidfile '%s': %s",
+            pidfile.c_str(), getLastErrorString().c_str());
     }
+}
 
 } // End of anonymous namespace
 
@@ -429,7 +431,8 @@ StorageNode::shutdown()
     LOG(debug, "Done shutting down node");
 }
 
-void StorageNode::configure(std::unique_ptr<StorServerConfig> config) {
+void
+StorageNode::configure(std::unique_ptr<StorServerConfig> config) {
     log_config_received(*config);
     // When we get config, we try to grab the config lock to ensure noone
     // else is doing configuration work, and then we write the new config
@@ -445,7 +448,8 @@ void StorageNode::configure(std::unique_ptr<StorServerConfig> config) {
     }
 }
 
-void StorageNode::configure(std::unique_ptr<UpgradingConfig> config) {
+void
+StorageNode::configure(std::unique_ptr<UpgradingConfig> config) {
     log_config_received(*config);
     {
         std::lock_guard configLockGuard(_configLock);
@@ -457,7 +461,8 @@ void StorageNode::configure(std::unique_ptr<UpgradingConfig> config) {
     }
 }
 
-void StorageNode::configure(std::unique_ptr<StorDistributionConfig> config) {
+void
+StorageNode::configure(std::unique_ptr<StorDistributionConfig> config) {
     log_config_received(*config);
     {
         std::lock_guard configLockGuard(_configLock);
@@ -486,7 +491,8 @@ StorageNode::configure(std::unique_ptr<document::config::DocumenttypesConfig> co
     }
 }
 
-void StorageNode::configure(std::unique_ptr<BucketspacesConfig> config) {
+void
+StorageNode::configure(std::unique_ptr<BucketspacesConfig> config) {
     log_config_received(*config);
     {
         std::lock_guard configLockGuard(_configLock);