Merge pull request #13903 from vespa-engine/vekterli/misc-cleanup

Miscellaneous content layer code cleanup

6 files changed, 44 insertions, 42 deletions

diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp
index 65ee78f7642..85604299e85 100644
--- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp
+++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp
@@ -130,9 +130,9 @@ FileStorManager::configure(std::unique_ptr<vespa::config::content::StorFilestorC
         _metrics->initDiskMetrics(_disks.size(), _component.getLoadTypes()->getMetricLoadTypes(), numStripes, numThreads);
 
         _filestorHandler = std::make_unique<FileStorHandler>(numThreads, numStripes, *this, *_metrics, _partitions, _compReg);
-        uint32_t numResposeThreads = computeNumResponseThreads(_config->numResponseThreads);
-        if (numResposeThreads > 0) {
-            _sequencedExecutor = vespalib::SequencedTaskExecutor::create(numResposeThreads, 10000, selectSequencer(_config->responseSequencerType));
+        uint32_t numResponseThreads = computeNumResponseThreads(_config->numResponseThreads);
+        if (numResponseThreads > 0) {
+            _sequencedExecutor = vespalib::SequencedTaskExecutor::create(numResponseThreads, 10000, selectSequencer(_config->responseSequencerType));
         }
         for (uint32_t i=0; i<_component.getDiskCount(); ++i) {
             if (_partitions[i].isUp()) {
diff --git a/storage/src/vespa/storage/persistence/persistencethread.cpp b/storage/src/vespa/storage/persistence/persistencethread.cpp
index 76c241d0627..a96e9870d36 100644
--- a/storage/src/vespa/storage/persistence/persistencethread.cpp
+++ b/storage/src/vespa/storage/persistence/persistencethread.cpp
@@ -919,7 +919,7 @@ PersistenceThread::processLockedMessage(FileStorHandler::LockedMessage lock) {
 void
 PersistenceThread::run(framework::ThreadHandle& thread)
 {
-    LOG(debug, "Started persistence thread with pid %d", getpid());
+    LOG(debug, "Started persistence thread");
 
     while (!thread.interrupted() && !_env._fileStorHandler.closed(_env._partition)) {
         thread.registerTick();
@@ -933,7 +933,7 @@ PersistenceThread::run(framework::ThreadHandle& thread)
         vespalib::MonitorGuard flushMonitorGuard(_flushMonitor);
         flushMonitorGuard.broadcast();
     }
-    LOG(debug, "Closing down persistence thread %d", getpid());
+    LOG(debug, "Closing down persistence thread");
     vespalib::MonitorGuard flushMonitorGuard(_flushMonitor);
     _closed = true;
     flushMonitorGuard.broadcast();
diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.cpp b/storage/src/vespa/storage/storageserver/communicationmanager.cpp
index d0d1056d4fd..c0adb01ad47 100644
--- a/storage/src/vespa/storage/storageserver/communicationmanager.cpp
+++ b/storage/src/vespa/storage/storageserver/communicationmanager.cpp
@@ -46,7 +46,7 @@ void
 CommunicationManager::receiveStorageReply(const std::shared_ptr<api::StorageReply>& reply)
 {
     assert(reply);
-    enque_or_process(reply);
+    enqueue_or_process(reply);
 }
 
 namespace {
@@ -101,7 +101,7 @@ CommunicationManager::handleMessage(std::unique_ptr<mbus::Message> msg)
         cmd->setTrace(docMsgPtr->getTrace());
         cmd->setTransportContext(std::make_unique<StorageTransportContext>(std::move(docMsgPtr)));
 
-        enque_or_process(std::move(cmd));
+        enqueue_or_process(std::move(cmd));
     } else if (protocolName == mbusprot::StorageProtocol::NAME) {
         std::unique_ptr<mbusprot::StorageCommand> storMsgPtr(static_cast<mbusprot::StorageCommand*>(msg.release()));
 
@@ -113,7 +113,7 @@ CommunicationManager::handleMessage(std::unique_ptr<mbus::Message> msg)
         cmd->setTrace(storMsgPtr->getTrace());
         cmd->setTransportContext(std::make_unique<StorageTransportContext>(std::move(storMsgPtr)));
 
-        enque_or_process(std::move(cmd));
+        enqueue_or_process(std::move(cmd));
     } else {
         LOGBM(warning, "Received unsupported message type %d for protocol '%s'",
               msg->getType(), msg->getProtocol().c_str());
@@ -443,7 +443,7 @@ CommunicationManager::process(const std::shared_ptr<api::StorageMessage>& msg)
 }
 
 void
-CommunicationManager::enque_or_process(std::shared_ptr<api::StorageMessage> msg)
+CommunicationManager::enqueue_or_process(std::shared_ptr<api::StorageMessage> msg)
 {
     assert(msg);
     if (_skip_thread) {
diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.h b/storage/src/vespa/storage/storageserver/communicationmanager.h
index c2d5ea7039d..23b59f5a42a 100644
--- a/storage/src/vespa/storage/storageserver/communicationmanager.h
+++ b/storage/src/vespa/storage/storageserver/communicationmanager.h
@@ -114,7 +114,7 @@ private:
     bool                  _skip_thread;
 
     void updateMetrics(const MetricLockGuard &) override;
-    void enque_or_process(std::shared_ptr<api::StorageMessage> msg);
+    void enqueue_or_process(std::shared_ptr<api::StorageMessage> msg);
 
     // Test needs access to configure() for live reconfig testing.
     friend struct CommunicationManagerTest;
diff --git a/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp b/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp
index fd16f43050a..2595514e6f4 100644
--- a/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp
+++ b/storageframework/src/vespa/storageframework/defaultimplementation/component/componentregisterimpl.cpp
@@ -19,16 +19,22 @@ ComponentRegisterImpl::ComponentRegisterImpl()
       _shutdownListener(nullptr)
 { }
 
-ComponentRegisterImpl::~ComponentRegisterImpl() { }
+ComponentRegisterImpl::~ComponentRegisterImpl() = default;
 
 void
 ComponentRegisterImpl::registerComponent(ManagedComponent& mc)
 {
     vespalib::LockGuard lock(_componentLock);
     _components.push_back(&mc);
-    if (_clock != 0) mc.setClock(*_clock);
-    if (_threadPool != 0) mc.setThreadPool(*_threadPool);
-    if (_metricManager != 0) mc.setMetricRegistrator(*this);
+    if (_clock) {
+        mc.setClock(*_clock);
+    }
+    if (_threadPool) {
+        mc.setThreadPool(*_threadPool);
+    }
+    if (_metricManager) {
+        mc.setMetricRegistrator(*this);
+    }
     mc.setUpgradeFlag(_upgradeFlag);
 }
 
@@ -36,7 +42,7 @@ void
 ComponentRegisterImpl::requestShutdown(vespalib::stringref reason)
 {
     vespalib::LockGuard lock(_componentLock);
-    if (_shutdownListener != 0) {
+    if (_shutdownListener) {
         _shutdownListener->requestShutdown(reason);
     }
 }
@@ -47,7 +53,7 @@ ComponentRegisterImpl::setMetricManager(metrics::MetricManager& mm)
     std::vector<ManagedComponent*> components;
     {
         vespalib::LockGuard lock(_componentLock);
-        assert(_metricManager == 0);
+        assert(_metricManager == nullptr);
         components = _components;
         _metricManager = &mm;
     }
@@ -55,8 +61,8 @@ ComponentRegisterImpl::setMetricManager(metrics::MetricManager& mm)
         metrics::MetricLockGuard lock(mm.getMetricLock());
         mm.registerMetric(lock, _topMetricSet);
     }
-    for (uint32_t i=0; i<components.size(); ++i) {
-        components[i]->setMetricRegistrator(*this);
+    for (auto* component : _components) {
+        component->setMetricRegistrator(*this);
     }
 }
 
@@ -64,10 +70,10 @@ void
 ComponentRegisterImpl::setClock(Clock& c)
 {
     vespalib::LockGuard lock(_componentLock);
-    assert(_clock == 0);
+    assert(_clock == nullptr);
     _clock = &c;
-    for (uint32_t i=0; i<_components.size(); ++i) {
-        _components[i]->setClock(c);
+    for (auto* component : _components) {
+        component->setClock(c);
     }
 }
 
@@ -75,10 +81,10 @@ void
 ComponentRegisterImpl::setThreadPool(ThreadPool& tp)
 {
     vespalib::LockGuard lock(_componentLock);
-    assert(_threadPool == 0);
+    assert(_threadPool == nullptr);
     _threadPool = &tp;
-    for (uint32_t i=0; i<_components.size(); ++i) {
-        _components[i]->setThreadPool(tp);
+    for (auto* component : _components) {
+        component->setThreadPool(tp);
     }
 }
 
@@ -87,8 +93,8 @@ ComponentRegisterImpl::setUpgradeFlag(UpgradeFlags flag)
 {
     vespalib::LockGuard lock(_componentLock);
     _upgradeFlag = flag;
-    for (uint32_t i=0; i<_components.size(); ++i) {
-        _components[i]->setUpgradeFlag(_upgradeFlag);
+    for (auto* component : _components) {
+        component->setUpgradeFlag(_upgradeFlag);
     }
 }
 
@@ -96,14 +102,14 @@ const StatusReporter*
 ComponentRegisterImpl::getStatusReporter(vespalib::stringref id)
 {
     vespalib::LockGuard lock(_componentLock);
-    for (uint32_t i=0; i<_components.size(); ++i) {
-        if (_components[i]->getStatusReporter() != 0
-            && _components[i]->getStatusReporter()->getId() == id)
+    for (auto* component : _components) {
+        if ((component->getStatusReporter() != nullptr)
+            && (component->getStatusReporter()->getId() == id))
         {
-            return _components[i]->getStatusReporter();
+            return component->getStatusReporter();
         }
     }
-    return 0;
+    return nullptr;
 }
 
 std::vector<const StatusReporter*>
@@ -111,9 +117,9 @@ ComponentRegisterImpl::getStatusReporters()
 {
     std::vector<const StatusReporter*> reporters;
     vespalib::LockGuard lock(_componentLock);
-    for (uint32_t i=0; i<_components.size(); ++i) {
-        if (_components[i]->getStatusReporter() != 0) {
-            reporters.push_back(_components[i]->getStatusReporter());
+    for (auto* component : _components) {
+        if (component->getStatusReporter() != nullptr) {
+            reporters.emplace_back(component->getStatusReporter());
         }
     }
     return reporters;
@@ -147,9 +153,9 @@ ComponentRegisterImpl::registerUpdateHook(vespalib::stringref name,
                                           SecondTime period)
 {
     vespalib::LockGuard lock(_componentLock);
-    metrics::UpdateHook::UP hookPtr(new MetricHookWrapper(name, hook));
+    auto hookPtr = std::make_unique<MetricHookWrapper>(name, hook);
     _metricManager->addMetricUpdateHook(*hookPtr, period.getTime());
-    _hooks.push_back(std::move(hookPtr));
+    _hooks.emplace_back(std::move(hookPtr));
 }
 
 metrics::MetricLockGuard
@@ -162,11 +168,7 @@ void
 ComponentRegisterImpl::registerShutdownListener(ShutdownListener& listener)
 {
     vespalib::LockGuard lock(_componentLock);
-    if (_shutdownListener != 0) {
-        throw vespalib::IllegalStateException(
-                "A shutdown listener is already registered. Add functionality "
-                "for having multiple if we need multiple.", VESPA_STRLOC);
-    }
+    assert(_shutdownListener == nullptr);
     _shutdownListener = &listener;
 }
 
diff --git a/storageframework/src/vespa/storageframework/generic/metric/metricregistrator.h b/storageframework/src/vespa/storageframework/generic/metric/metricregistrator.h
index f8ab6699d3c..6cdbf58d555 100644
--- a/storageframework/src/vespa/storageframework/generic/metric/metricregistrator.h
+++ b/storageframework/src/vespa/storageframework/generic/metric/metricregistrator.h
@@ -22,7 +22,7 @@ namespace storage::framework {
 struct MetricUpdateHook;
 
 struct MetricRegistrator {
-    virtual ~MetricRegistrator() {}
+    virtual ~MetricRegistrator() = default;
 
     virtual void registerMetric(metrics::Metric&) = 0;
     virtual void registerUpdateHook(vespalib::stringref name, MetricUpdateHook& hook, SecondTime period) = 0;