Merge pull request #16295 from vespa-engine/balder/various-code-health

Various code health.

20 files changed, 92 insertions, 127 deletions

diff --git a/searchcore/src/tests/proton/flushengine/flushengine_test.cpp b/searchcore/src/tests/proton/flushengine/flushengine_test.cpp
index a675a45aa54..fae2e9f2d52 100644
--- a/searchcore/src/tests/proton/flushengine/flushengine_test.cpp
+++ b/searchcore/src/tests/proton/flushengine/flushengine_test.cpp
@@ -31,9 +31,9 @@ using searchcorespi::IFlushTarget;
 using searchcorespi::FlushTask;
 using vespalib::Slime;
 
-const long LONG_TIMEOUT = 66666;
-const long SHORT_TIMEOUT = 1;
-const uint32_t IINTERVAL = 1000;
+constexpr long LONG_TIMEOUT = 66666;
+constexpr long SHORT_TIMEOUT = 1;
+constexpr vespalib::duration IINTERVAL = 1s;
 
 class SimpleExecutor : public vespalib::Executor {
 public:
@@ -426,15 +426,15 @@ struct Fixture
     SimpleStrategy::SP strategy;
     FlushEngine engine;
 
-    Fixture(uint32_t numThreads, uint32_t idleIntervalMS, SimpleStrategy::SP strategy_)
+    Fixture(uint32_t numThreads, vespalib::duration idleInterval, SimpleStrategy::SP strategy_)
         : tlsStatsFactory(std::make_shared<SimpleTlsStatsFactory>()),
           strategy(strategy_),
-          engine(tlsStatsFactory, strategy, numThreads, idleIntervalMS)
+          engine(tlsStatsFactory, strategy, numThreads, idleInterval)
     {
     }
 
-    Fixture(uint32_t numThreads, uint32_t idleIntervalMS)
-        : Fixture(numThreads, idleIntervalMS, std::make_shared<SimpleStrategy>())
+    Fixture(uint32_t numThreads, vespalib::duration idleInterval)
+        : Fixture(numThreads, idleInterval, std::make_shared<SimpleStrategy>())
     {
     }
 
@@ -488,12 +488,12 @@ TEST_F("require that strategy controls flush target", Fixture(1, IINTERVAL))
     EXPECT_EQUAL("bar", order[1]);
 }
 
-TEST_F("require that zero handlers does not core", Fixture(2, 50))
+TEST_F("require that zero handlers does not core", Fixture(2, 50ms))
 {
     f.engine.start();
 }
 
-TEST_F("require that zero targets does not core", Fixture(2, 50))
+TEST_F("require that zero targets does not core", Fixture(2, 50ms))
 {
     f.putFlushHandler("foo", std::make_shared<SimpleHandler>(Targets(), "foo"));
     f.putFlushHandler("bar", std::make_shared<SimpleHandler>(Targets(), "bar"));
@@ -692,7 +692,7 @@ assertThatHandlersInCurrentSet(FlushEngine & engine, const std::vector<const cha
     }
 }
 
-TEST_F("require that concurrency works", Fixture(2, 1))
+TEST_F("require that concurrency works", Fixture(2, 1ms))
 {
     auto target1 = std::make_shared<SimpleTarget>("target1", 1, false);
     auto target2 = std::make_shared<SimpleTarget>("target2", 2, false);
@@ -713,7 +713,7 @@ TEST_F("require that concurrency works", Fixture(2, 1))
     target2->_proceed.countDown();
 }
 
-TEST_F("require that state explorer can list flush targets", Fixture(1, 1))
+TEST_F("require that state explorer can list flush targets", Fixture(1, 1ms))
 {
     auto target = std::make_shared<SimpleTarget>("target1", 100, false);
     f.putFlushHandler("handler",
@@ -744,7 +744,7 @@ TEST_F("require that state explorer can list flush targets", Fixture(1, 1))
     target->_taskDone.await(LONG_TIMEOUT);
 }
 
-TEST_F("require that oldest serial is updated when closing engine", Fixture(1, 100))
+TEST_F("require that oldest serial is updated when closing engine", Fixture(1, 100ms))
 {
     auto target1 = std::make_shared<SimpleTarget>("target1", 10, false);
     auto handler = f.addSimpleHandler({ target1 });
@@ -754,7 +754,7 @@ TEST_F("require that oldest serial is updated when closing engine", Fixture(1, 1
     EXPECT_EQUAL(20u, handler->_oldestSerial);
 }
 
-TEST_F("require that oldest serial is updated when finishing priority flush strategy", Fixture(1, 100, std::make_shared<NoFlushStrategy>()))
+TEST_F("require that oldest serial is updated when finishing priority flush strategy", Fixture(1, 100ms, std::make_shared<NoFlushStrategy>()))
 {
     auto target1 = std::make_shared<SimpleTarget>("target1", 10, true);
     auto handler = f.addSimpleHandler({ target1 });
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
index cc1af923fc0..0b00e274522 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
@@ -328,7 +328,7 @@ AttributeManager::flushAll(SerialNum currentSerial)
     for (const auto &ft : flushTargets) {
         vespalib::Executor::Task::UP task;
         task = ft->initFlush(currentSerial, std::make_shared<search::FlushToken>());
-        if (task.get() != NULL) {
+        if (task) {
             task->run();
         }
     }
diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp
index b563cee1d0c..5e6a9f354f9 100644
--- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp
+++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp
@@ -39,9 +39,7 @@ private:
     bool saveDocumentMetaStore(); // not updating snap info.
 public:
     Flusher(DocumentMetaStoreFlushTarget &dmsft, uint64_t syncToken, AttributeDirectory::Writer &writer);
-    ~Flusher();
-    uint64_t getSyncToken() const { return _syncToken; }
-    bool saveSnapInfo();
+    ~Flusher() override;
     bool flush(AttributeDirectory::Writer &writer);
     void updateStats();
     bool cleanUp(AttributeDirectory::Writer &writer);
@@ -66,29 +64,23 @@ Flusher(DocumentMetaStoreFlushTarget &dmsft,
     assert(_saver);
 }
 
-DocumentMetaStoreFlushTarget::Flusher::~Flusher()
-{
-    // empty
-}
+DocumentMetaStoreFlushTarget::Flusher::~Flusher() = default;
 
 bool
 DocumentMetaStoreFlushTarget::Flusher::saveDocumentMetaStore()
 {
     vespalib::mkdir(_flushDir, false);
-    SerialNumFileHeaderContext fileHeaderContext(_dmsft._fileHeaderContext,
-                                                 _syncToken);
+    SerialNumFileHeaderContext fileHeaderContext(_dmsft._fileHeaderContext, _syncToken);
     bool saveSuccess = false;
     if (_dmsft._hwInfo.disk().slow()) {
         search::AttributeMemorySaveTarget memorySaveTarget;
         saveSuccess = _saver->save(memorySaveTarget);
         _saver.reset();
         if (saveSuccess) {
-            saveSuccess = memorySaveTarget.writeToFile(_dmsft._tuneFileAttributes,
-                                                       fileHeaderContext);
+            saveSuccess = memorySaveTarget.writeToFile(_dmsft._tuneFileAttributes, fileHeaderContext);
         }
     } else {
-        search::AttributeFileSaveTarget saveTarget(_dmsft._tuneFileAttributes,
-                                                   fileHeaderContext);
+        search::AttributeFileSaveTarget saveTarget(_dmsft._tuneFileAttributes, fileHeaderContext);
         saveSuccess = _saver->save(saveTarget);
         _saver.reset();
     }
diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.cpp
index 94bba1bf64f..9a4054d02ca 100644
--- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.cpp
+++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.cpp
@@ -83,7 +83,7 @@ DocumentMetaStoreSaver(vespalib::GenerationHandler::Guard &&guard,
 }
 
 
-DocumentMetaStoreSaver::~DocumentMetaStoreSaver() { }
+DocumentMetaStoreSaver::~DocumentMetaStoreSaver() = default;
 
 
 bool
diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.h
index 2f745929667..a11f60af4b6 100644
--- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.h
+++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoresaver.h
@@ -32,14 +32,14 @@ private:
     const MetaDataStore &_metaDataStore;
     bool _writeDocSize;
 
-    virtual bool onSave(search::IAttributeSaveTarget &saveTarget) override;
+    bool onSave(search::IAttributeSaveTarget &saveTarget) override;
 public:
     DocumentMetaStoreSaver(vespalib::GenerationHandler::Guard &&guard,
                            const search::attribute::AttributeHeader &header,
                            const GidIterator &gidIterator,
                            const MetaDataStore &metaDataStore);
 
-    virtual ~DocumentMetaStoreSaver();
+    ~DocumentMetaStoreSaver() override;
 };
 
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/cachedflushtarget.h b/searchcore/src/vespa/searchcore/proton/flushengine/cachedflushtarget.h
index ecd66f9cbfb..f2b3514fcf9 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/cachedflushtarget.h
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/cachedflushtarget.h
@@ -42,16 +42,18 @@ public:
     const IFlushTarget::SP & getFlushTarget() { return _target; }
 
     // Implements IFlushTarget.
-    virtual MemoryGain getApproxMemoryGain() const override { return _memoryGain; }
-    virtual   DiskGain   getApproxDiskGain() const override { return _diskGain; }
-    virtual  SerialNum getFlushedSerialNum() const override { return _flushedSerialNum; }
-    virtual       Time    getLastFlushTime() const override { return _lastFlushTime; }
-    virtual       bool     needUrgentFlush() const override { return _needUrgentFlush; }
-
-    virtual Task::UP initFlush(SerialNum currentSerial, std::shared_ptr<search::IFlushToken> flush_token) override { return _target->initFlush(currentSerial, std::move(flush_token)); }
-    virtual FlushStats getLastFlushStats() const override { return _target->getLastFlushStats(); }
-
-    virtual uint64_t getApproxBytesToWriteToDisk() const override;
+    MemoryGain getApproxMemoryGain() const override { return _memoryGain; }
+    DiskGain   getApproxDiskGain() const override { return _diskGain; }
+    SerialNum getFlushedSerialNum() const override { return _flushedSerialNum; }
+    Time    getLastFlushTime() const override { return _lastFlushTime; }
+    bool     needUrgentFlush() const override { return _needUrgentFlush; }
+
+    Task::UP initFlush(SerialNum currentSerial, std::shared_ptr<search::IFlushToken> flush_token) override {
+        return _target->initFlush(currentSerial, std::move(flush_token));
+    }
+    FlushStats getLastFlushStats() const override { return _target->getLastFlushStats(); }
+
+    uint64_t getApproxBytesToWriteToDisk() const override;
 };
 
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.cpp
index 2a447957334..1f5ad8ae3a3 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.cpp
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.cpp
@@ -1,7 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "flush_engine_explorer.h"
-
+#include "flushengine.h"
 #include <vespa/vespalib/data/slime/cursor.h>
 #include <vespa/vespalib/data/slime/inserter.h>
 
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.h b/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.h
index a674b39bc26..c2a90597ca2 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.h
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_engine_explorer.h
@@ -2,11 +2,12 @@
 
 #pragma once
 
-#include "flushengine.h"
 #include <vespa/vespalib/net/state_explorer.h>
 
 namespace proton {
 
+class FlushEngine;
+
 /**
  * Class used to explore the state of a flush engine and its flush targets.
  */
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushcontext.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flushcontext.cpp
index b569e499876..ed31317cb69 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/flushcontext.cpp
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushcontext.cpp
@@ -16,20 +16,17 @@ FlushContext::FlushContext(
       _target(target),
       _task(),
       _lastSerial(lastSerial)
-{
-    // empty
-}
+{ }
 
-vespalib::string FlushContext::createName(const IFlushHandler & handler, const IFlushTarget & target)
-{
+vespalib::string
+FlushContext::createName(const IFlushHandler & handler, const IFlushTarget & target) {
     return (handler.getName() + "." + target.getName());
 }
 
 FlushContext::~FlushContext()
 {
-    if (_task.get() != NULL) {
-        LOG(warning, "Unexecuted flush task for '%s' destroyed.",
-            _name.c_str());
+    if (_task) {
+        LOG(warning, "Unexecuted flush task for '%s' destroyed.", _name.c_str());
     }
 }
 
@@ -38,11 +35,11 @@ FlushContext::initFlush(std::shared_ptr<search::IFlushToken> flush_token)
 {
     LOG(debug, "Attempting to flush '%s'.", _name.c_str());
     _task = _target->initFlush(std::max(_handler->getCurrentSerialNumber(), _lastSerial), std::move(flush_token));
-    if (_task.get() == NULL) {
+    if ( ! _task ) {
         LOG(debug, "Target refused to init flush.");
         return false;
     }
     return true;
 }
 
-} // namespace proton
+}
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp
index ab5b1ac5937..b4ab34d6866 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp
@@ -77,10 +77,10 @@ FlushEngine::FlushInfo::FlushInfo(uint32_t taskId, const IFlushTarget::SP &targe
 }
 
 FlushEngine::FlushEngine(std::shared_ptr<flushengine::ITlsStatsFactory> tlsStatsFactory,
-                         IFlushStrategy::SP strategy, uint32_t numThreads, uint32_t idleIntervalMS)
+                         IFlushStrategy::SP strategy, uint32_t numThreads, vespalib::duration idleInterval)
     : _closed(false),
       _maxConcurrent(numThreads),
-      _idleIntervalMS(idleIntervalMS),
+      _idleInterval(idleInterval),
       _taskId(0),
       _threadPool(128 * 1024),
       _strategy(std::move(strategy)),
@@ -151,11 +151,11 @@ FlushEngine::canFlushMore(const std::unique_lock<std::mutex> &guard) const
 }
 
 bool
-FlushEngine::wait(size_t minimumWaitTimeIfReady)
+FlushEngine::wait(vespalib::duration minimumWaitTimeIfReady)
 {
     std::unique_lock<std::mutex> guard(_lock);
-    if ( (minimumWaitTimeIfReady > 0) && canFlushMore(guard) && _pendingPrune.empty()) {
-        _cond.wait_for(guard, std::chrono::milliseconds(minimumWaitTimeIfReady));
+    if ( (minimumWaitTimeIfReady != vespalib::duration::zero()) && canFlushMore(guard) && _pendingPrune.empty()) {
+        _cond.wait_for(guard, minimumWaitTimeIfReady);
     }
     while ( ! canFlushMore(guard) && _pendingPrune.empty()) {
         _cond.wait_for(guard, 1s); // broadcast when flush done
@@ -168,7 +168,7 @@ FlushEngine::Run(FastOS_ThreadInterface *, void *)
 {
     bool shouldIdle = false;
     vespalib::string prevFlushName;
-    while (wait(shouldIdle ? _idleIntervalMS : 0)) {
+    while (wait(shouldIdle ? _idleInterval : vespalib::duration::zero())) {
         shouldIdle = false;
         if (prune()) {
             continue; // Prune attempted on one or more handlers
@@ -181,8 +181,8 @@ FlushEngine::Run(FastOS_ThreadInterface *, void *)
         } else {
             shouldIdle = true;
         }
-        LOG(debug, "Making another wait(idle=%s, timeMS=%d) last was '%s'",
-            shouldIdle ? "true" : "false", shouldIdle ? _idleIntervalMS : 0, prevFlushName.c_str());
+        LOG(debug, "Making another wait(idle=%s, timeS=%1.3f) last was '%s'",
+            shouldIdle ? "true" : "false", shouldIdle ? vespalib::to_s(_idleInterval) : 0, prevFlushName.c_str());
     }
     _executor.sync();
     prune();
@@ -220,7 +220,8 @@ FlushEngine::prune()
     return true;
 }
 
-bool FlushEngine::isFlushing(const std::lock_guard<std::mutex> & guard, const vespalib::string & name) const
+bool
+FlushEngine::isFlushing(const std::lock_guard<std::mutex> & guard, const vespalib::string & name) const
 {
     (void) guard;
     for(const auto & it : _flushing) {
@@ -306,7 +307,7 @@ FlushEngine::flushAll(const FlushContext::List &lst)
 {
     LOG(debug, "%ld targets to flush.", lst.size());
     for (const FlushContext::SP & ctx : lst) {
-        if (wait(0)) {
+        if (wait(vespalib::duration::zero())) {
             if (ctx->initFlush(get_flush_token(*ctx))) {
                 logTarget("initiated", *ctx);
                 _executor.execute(std::make_unique<FlushTask>(initFlush(*ctx), *this, ctx));
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h
index f51e93f0fbd..e4d9e269215 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h
@@ -50,7 +50,7 @@ private:
     typedef HandlerMap<IFlushHandler> FlushHandlerMap;
     bool                           _closed;
     const uint32_t                 _maxConcurrent;
-    const uint32_t                 _idleIntervalMS;
+    const vespalib::duration       _idleInterval;
     uint32_t                       _taskId;    
     FastOS_ThreadPool              _threadPool;
     IFlushStrategy::SP             _strategy;
@@ -79,7 +79,7 @@ private:
     uint32_t initFlush(const IFlushHandler::SP &handler, const IFlushTarget::SP &target);
     void flushDone(const FlushContext &ctx, uint32_t taskId);
     bool canFlushMore(const std::unique_lock<std::mutex> &guard) const;
-    bool wait(size_t minimumWaitTimeIfReady);
+    bool wait(vespalib::duration minimumWaitTimeIfReady);
     bool isFlushing(const std::lock_guard<std::mutex> &guard, const vespalib::string & name) const;
 
     friend class FlushTask;
@@ -102,7 +102,7 @@ public:
      * @param idleInterval The interval between when flushes are checked whne there are no one progressing.
      */
     FlushEngine(std::shared_ptr<flushengine::ITlsStatsFactory> tlsStatsFactory,
-                IFlushStrategy::SP strategy, uint32_t numThreads, uint32_t idleIntervalMS);
+                IFlushStrategy::SP strategy, uint32_t numThreads, vespalib::duration idleIntervalMS);
 
     /**
      * Destructor. Waits for all pending tasks to complete.
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushtargetproxy.h b/searchcore/src/vespa/searchcore/proton/flushengine/flushtargetproxy.h
index 84bf91a2f9a..2d0be231adc 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/flushtargetproxy.h
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushtargetproxy.h
@@ -31,8 +31,7 @@ public:
      * @param target   The target to decorate.
      * @param prefix   The prefix to prepend to the target
      */
-    FlushTargetProxy(const IFlushTarget::SP &target,
-                     const vespalib::string & prefix);
+    FlushTargetProxy(const IFlushTarget::SP &target, const vespalib::string & prefix);
     /**
      * Returns the decorated flush target. This should not be used for anything
      * but testing, as invoking a method on the returned target beats the
@@ -40,34 +39,16 @@ public:
      *
      * @return The decorated flush target.
      */
-    const IFlushTarget::SP &
-    getFlushTarget() const
-    {
-        return _target;
-    }
+    const IFlushTarget::SP & getFlushTarget() const { return _target; }
     // Implements IFlushTarget.
-    virtual MemoryGain
-    getApproxMemoryGain() const override;
-
-    virtual DiskGain
-    getApproxDiskGain() const override;
-
-    virtual SerialNum
-    getFlushedSerialNum() const override;
-
-    virtual Time
-    getLastFlushTime() const override;
-
-    virtual bool
-    needUrgentFlush() const override;
-
-    virtual Task::UP
-    initFlush(SerialNum currentSerial, std::shared_ptr<search::IFlushToken> flush_token) override;
-
-    virtual searchcorespi::FlushStats
-    getLastFlushStats() const override;
-
-    virtual uint64_t getApproxBytesToWriteToDisk() const override;
+    MemoryGain getApproxMemoryGain() const override;
+    DiskGain getApproxDiskGain() const override;
+    SerialNum getFlushedSerialNum() const override;
+    Time getLastFlushTime() const override;
+    bool needUrgentFlush() const override;
+    Task::UP initFlush(SerialNum currentSerial, std::shared_ptr<search::IFlushToken> flush_token) override;
+    searchcorespi::FlushStats getLastFlushStats() const override;
+    uint64_t getApproxBytesToWriteToDisk() const override;
 };
 
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.cpp
index 94476d93b15..2b1bd3b423d 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.cpp
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.cpp
@@ -1,17 +1,18 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "flushtask.h"
+#include "flushengine.h"
 
 namespace proton {
 
 FlushTask::FlushTask(uint32_t taskId,
                      FlushEngine &engine,
-                     const FlushContext::SP &ctx)
+                     std::shared_ptr<FlushContext> ctx)
     : _taskId(taskId),
       _engine(engine),
-      _context(ctx)
+      _context(std::move(ctx))
 {
-    assert(_context.get() != NULL);
+    assert(_context);
 }
 
 FlushTask::~FlushTask()
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.h b/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.h
index 7860acaab41..fb3474e2a90 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.h
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushtask.h
@@ -1,10 +1,13 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
-#include <vespa/searchcore/proton/flushengine/flushengine.h>
+#include <memory>
+#include <vespa/vespalib/util/executor.h>
 
 namespace proton {
 
+class FlushEngine;
+class FlushContext;
 /**
  * This class decorates a task returned by initFlush() in IFlushTarget so that
  * the appropriate callback is invoked on the running FlushEngine.
@@ -14,7 +17,7 @@ class FlushTask : public vespalib::Executor::Task
 private:
     uint32_t                          _taskId;
     FlushEngine                      &_engine;
-    FlushContext::SP                  _context;
+    std::shared_ptr<FlushContext>     _context;
 
 public:
     FlushTask(const FlushTask &) = delete;
@@ -26,15 +29,14 @@ public:
      * @param engine The running flush engine.
      * @param ctx    The context of the flush to perform.
      */
-    FlushTask(uint32_t taskId, FlushEngine &engine, const FlushContext::SP &ctx);
+    FlushTask(uint32_t taskId, FlushEngine &engine, std::shared_ptr<FlushContext> ctx);
 
     /**
      * Destructor. Notifies the engine that the flush is done to prevent the
      * engine from locking targets because of a glitch.
      */
-    ~FlushTask();
+    ~FlushTask() override;
 
-    // Implements Executor::Task.
     void run() override;
 };
 
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h b/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h
index ca23f27ed49..66ca730bfbc 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h
@@ -2,7 +2,6 @@
 #pragma once
 
 #include <vespa/searchcorespi/flush/iflushtarget.h>
-#include <vector>
 
 namespace proton {
 
@@ -35,7 +34,7 @@ public:
     /**
      * Virtual destructor required for inheritance.
      */
-    virtual ~IFlushHandler() { }
+    virtual ~IFlushHandler() = default;
 
     /**
      * Returns the unique name of this handler.
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h b/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h
index 67f6771650a..8c6a31ff270 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h
@@ -14,17 +14,12 @@ namespace flushengine { class TlsStatsMap; }
  */
 class IFlushStrategy {
 public:
-    /**
-     * Convenience typedefs.
-     */
     typedef std::shared_ptr<IFlushStrategy> SP;
 
     IFlushStrategy(const IFlushStrategy &) = delete;
     IFlushStrategy & operator = (const IFlushStrategy &) = delete;
-    /**
-     * Virtual destructor required for inheritance.
-     */
-    virtual ~IFlushStrategy() { }
+
+    virtual ~IFlushStrategy() = default;
 
     /**
      * Takes an input of targets that are candidates for flush and returns
@@ -34,11 +29,10 @@ public:
      * @return A prioritized list of targets to flush.
      */
     virtual FlushContext::List getFlushTargets(const FlushContext::List & targetList,
-                                               const flushengine::TlsStatsMap &
-                                               tlsStatsMap) const = 0;
+                                               const flushengine::TlsStatsMap & tlsStatsMap) const = 0;
 protected:
     IFlushStrategy() = default;
 };
 
-} // namespace proton
+}
 
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.cpp
index 59a5504b57b..3db341ecb70 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.cpp
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.cpp
@@ -49,11 +49,9 @@ ThreadedFlushTarget::initFlush(SerialNum currentSerial, std::shared_ptr<search::
 {
     std::promise<Task::UP> promise;
     std::future<Task::UP> future = promise.get_future();
-    _executor.execute(makeLambdaTask([&]()
-                      { promise.set_value(callInitFlush(_target.get(),
-                                                        currentSerial,
-                                                        &_getSerialNum,
-                                                        flush_token)); }));
+    _executor.execute(makeLambdaTask([&]() {
+        promise.set_value(callInitFlush(_target.get(), currentSerial, &_getSerialNum, flush_token));
+    }));
     return future.get();
 }
 
diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.h b/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.h
index cd2ee11265f..d949ffa9c50 100644
--- a/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.h
+++ b/searchcore/src/vespa/searchcore/proton/flushengine/threadedflushtarget.h
@@ -18,7 +18,7 @@ class ThreadedFlushTarget : public FlushTargetProxy
 {
 private:
     using IFlushTarget = searchcorespi::IFlushTarget;
-    vespalib::Executor &_executor;
+    vespalib::Executor  &_executor;
     const IGetSerialNum &_getSerialNum;
 
 public:
@@ -48,9 +48,7 @@ public:
                         const IFlushTarget::SP &target,
                         const vespalib::string & prefix);
 
-    // Implements IFlushTarget.
-    virtual Task::UP
-    initFlush(SerialNum currentSerial, std::shared_ptr<search::IFlushToken> flush_token) override;
+    Task::UP initFlush(SerialNum currentSerial, std::shared_ptr<search::IFlushToken> flush_token) override;
 };
 
 } // namespace proton
diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp
index 5aa5d88eda9..1b615d765f0 100644
--- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp
@@ -304,7 +304,7 @@ Proton::init(const BootstrapConfig::SP & configSnapshot)
     vespalib::chdir(protonConfig.basedir);
     _tls->start();
     _flushEngine = std::make_unique<FlushEngine>(std::make_shared<flushengine::TlsStatsFactory>(_tls->getTransLogServer()),
-                                                 strategy, flush.maxconcurrent, flush.idleinterval*1000);
+                                                 strategy, flush.maxconcurrent, vespalib::from_s(flush.idleinterval));
     _metricsEngine->addExternalMetrics(_summaryEngine->getMetrics());
 
     char tmp[1024];
diff --git a/searchlib/src/vespa/searchlib/attribute/attributesaver.h b/searchlib/src/vespa/searchlib/attribute/attributesaver.h
index f90f9492487..c1bf18e775c 100644
--- a/searchlib/src/vespa/searchlib/attribute/attributesaver.h
+++ b/searchlib/src/vespa/searchlib/attribute/attributesaver.h
@@ -2,11 +2,10 @@
 
 #pragma once
 
-#include <vespa/vespalib/util/generationhandler.h>
 #include "attribute_header.h"
+#include <vespa/vespalib/util/generationhandler.h>
 
-namespace search
-{
+namespace search {
 
 class IAttributeSaveTarget;