From 3d20dde708032ac70ab4cb24fa8284686a0001f3 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Sat, 30 Jan 2021 21:57:04 +0000 Subject: Various code health. --- .../proton/attribute/attributemanager.cpp | 2 +- .../documentmetastoreflushtarget.cpp | 18 +++------- .../documentmetastore/documentmetastoresaver.cpp | 2 +- .../documentmetastore/documentmetastoresaver.h | 4 +-- .../proton/flushengine/cachedflushtarget.h | 22 ++++++------ .../proton/flushengine/flush_engine_explorer.cpp | 2 +- .../proton/flushengine/flush_engine_explorer.h | 3 +- .../searchcore/proton/flushengine/flushcontext.cpp | 17 ++++------ .../searchcore/proton/flushengine/flushengine.cpp | 3 +- .../proton/flushengine/flushtargetproxy.h | 39 ++++++---------------- .../searchcore/proton/flushengine/flushtask.cpp | 7 ++-- .../searchcore/proton/flushengine/flushtask.h | 12 ++++--- .../searchcore/proton/flushengine/iflushhandler.h | 3 +- .../searchcore/proton/flushengine/iflushstrategy.h | 14 +++----- .../proton/flushengine/threadedflushtarget.cpp | 8 ++--- .../proton/flushengine/threadedflushtarget.h | 6 ++-- .../src/vespa/searchlib/attribute/attributesaver.h | 5 ++- 17 files changed, 66 insertions(+), 101 deletions(-) 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()); - 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 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 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 #include 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 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 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..42b38bec821 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp @@ -220,7 +220,8 @@ FlushEngine::prune() return true; } -bool FlushEngine::isFlushing(const std::lock_guard & guard, const vespalib::string & name) const +bool +FlushEngine::isFlushing(const std::lock_guard & guard, const vespalib::string & name) const { (void) guard; for(const auto & it : _flushing) { 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 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 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 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 +#include +#include 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 _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 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 -#include 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 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 promise; std::future 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 flush_token) override; + Task::UP initFlush(SerialNum currentSerial, std::shared_ptr flush_token) override; }; } // namespace proton 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 #include "attribute_header.h" +#include -namespace search -{ +namespace search { class IAttributeSaveTarget; -- cgit v1.2.3 From c9418858a4a1c8848114564007e743f1f9d51247 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Sat, 30 Jan 2021 22:18:26 +0000 Subject: Use vespalib::duration for time. --- .../tests/proton/flushengine/flushengine_test.cpp | 26 +++++++++++----------- .../searchcore/proton/flushengine/flushengine.cpp | 18 +++++++-------- .../searchcore/proton/flushengine/flushengine.h | 6 ++--- .../src/vespa/searchcore/proton/server/proton.cpp | 2 +- 4 files changed, 26 insertions(+), 26 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()), 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()) + Fixture(uint32_t numThreads, vespalib::duration idleInterval) + : Fixture(numThreads, idleInterval, std::make_shared()) { } @@ -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(Targets(), "foo")); f.putFlushHandler("bar", std::make_shared(Targets(), "bar")); @@ -692,7 +692,7 @@ assertThatHandlersInCurrentSet(FlushEngine & engine, const std::vector("target1", 1, false); auto target2 = std::make_shared("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("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("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())) +TEST_F("require that oldest serial is updated when finishing priority flush strategy", Fixture(1, 100ms, std::make_shared())) { auto target1 = std::make_shared("target1", 10, true); auto handler = f.addSimpleHandler({ target1 }); diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp index 42b38bec821..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 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 &guard) const } bool -FlushEngine::wait(size_t minimumWaitTimeIfReady) +FlushEngine::wait(vespalib::duration minimumWaitTimeIfReady) { std::unique_lock 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(); @@ -307,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(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 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 &guard) const; - bool wait(size_t minimumWaitTimeIfReady); + bool wait(vespalib::duration minimumWaitTimeIfReady); bool isFlushing(const std::lock_guard &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 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/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(std::make_shared(_tls->getTransLogServer()), - strategy, flush.maxconcurrent, flush.idleinterval*1000); + strategy, flush.maxconcurrent, vespalib::from_s(flush.idleinterval)); _metricsEngine->addExternalMetrics(_summaryEngine->getMetrics()); char tmp[1024]; -- cgit v1.2.3