From 24610d1845db120cc55f4c88e9d7a40ce900dd84 Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Tue, 19 May 2026 13:53:16 +0200 Subject: [PATCH 1/3] Add FlushStrategyResult. Add strategy_info to flush history. --- .../flush_history/flush_history_test.cpp | 31 ++++---- .../proton/flushengine/flushengine_test.cpp | 37 ++++++--- .../prepare_restart_flush_strategy_test.cpp | 3 +- .../server/memoryflush/memoryflush_test.cpp | 5 +- .../prepare_restart2_rpc_handler_test.cpp | 16 ++-- .../searchcore/proton/common/eventlogger.cpp | 4 +- .../searchcore/proton/common/eventlogger.h | 2 +- .../proton/flushengine/CMakeLists.txt | 1 + .../proton/flushengine/flush_all_strategy.cpp | 12 +-- .../proton/flushengine/flush_all_strategy.h | 5 +- .../proton/flushengine/flush_history.cpp | 15 ++-- .../proton/flushengine/flush_history.h | 6 +- .../flushengine/flush_history_entry.cpp | 11 ++- .../proton/flushengine/flush_history_entry.h | 31 ++++---- .../flushengine/flush_history_explorer.cpp | 1 + .../flushengine/flush_strategy_result.cpp | 33 ++++++++ .../flushengine/flush_strategy_result.h | 42 ++++++++++ .../proton/flushengine/flushengine.cpp | 76 ++++++++++--------- .../proton/flushengine/flushengine.h | 23 ++---- .../proton/flushengine/iflushstrategy.h | 7 +- .../prepare_restart_flush_strategy.cpp | 21 +++-- .../prepare_restart_flush_strategy.h | 6 +- .../searchcore/proton/server/memoryflush.cpp | 21 +++-- .../searchcore/proton/server/memoryflush.h | 6 +- .../searchcore/proton/server/simpleflush.cpp | 15 +++- .../searchcore/proton/server/simpleflush.h | 5 +- 26 files changed, 282 insertions(+), 153 deletions(-) create mode 100644 searchcore/src/vespa/searchcore/proton/flushengine/flush_strategy_result.cpp create mode 100644 searchcore/src/vespa/searchcore/proton/flushengine/flush_strategy_result.h diff --git a/searchcore/src/tests/proton/flushengine/flush_history/flush_history_test.cpp b/searchcore/src/tests/proton/flushengine/flush_history/flush_history_test.cpp index 3fb304b77ed8..17da65680c37 100644 --- a/searchcore/src/tests/proton/flushengine/flush_history/flush_history_test.cpp +++ b/searchcore/src/tests/proton/flushengine/flush_history/flush_history_test.cpp @@ -22,6 +22,7 @@ const std::string NORMAL_STRATEGY("normal"); const std::string ALL_STRATEGY("all"); const std::string HANDLER1("handler1"); const std::string HANDLER2("handler2"); +const std::string strategy_info("info"); template std::vector make_names(const std::vector& entries) { std::vector result; @@ -148,9 +149,9 @@ TEST_F(FlushHistoryTest, empty_history) { } TEST_F(FlushHistoryTest, track_flushes) { - _flush_history.start_flush(HANDLER1, "a1", 3s, 5); - _flush_history.start_flush(HANDLER2, "a2", 1s, 6); - _flush_history.start_flush(HANDLER1, "a3", 4s, 7); + _flush_history.start_flush(HANDLER1, "a1", strategy_info, 3s, 5); + _flush_history.start_flush(HANDLER2, "a2", strategy_info, 1s, 6); + _flush_history.start_flush(HANDLER1, "a3", strategy_info, 4s, 7); _flush_history.flush_done(6); _flush_history.flush_done(5); _flush_history.prune_done(6); @@ -165,12 +166,12 @@ TEST_F(FlushHistoryTest, track_flushes) { } TEST_F(FlushHistoryTest, tracks_pending_flushes) { - _flush_history.add_pending_flush(HANDLER1, "a1", 3s); - _flush_history.add_pending_flush(HANDLER2, "a2", 1s); - _flush_history.add_pending_flush(HANDLER2, "a3", 4s); - _flush_history.add_pending_flush(HANDLER1, "a4", 7s); - _flush_history.start_flush(HANDLER1, "a1", 3s, 5); - _flush_history.start_flush(HANDLER2, "a2", 1s, 6); + _flush_history.add_pending_flush(HANDLER1, "a1", strategy_info, 3s); + _flush_history.add_pending_flush(HANDLER2, "a2", strategy_info, 1s); + _flush_history.add_pending_flush(HANDLER2, "a3", strategy_info, 4s); + _flush_history.add_pending_flush(HANDLER1, "a4", strategy_info, 7s); + _flush_history.start_flush(HANDLER1, "a1", strategy_info, 3s, 5); + _flush_history.start_flush(HANDLER2, "a2", strategy_info, 1s, 6); _flush_history.flush_done(6); _flush_history.prune_done(6); auto view = _flush_history.make_view(); @@ -184,7 +185,7 @@ TEST_F(FlushHistoryTest, tracks_pending_flushes) { } TEST_F(FlushHistoryTest, pending_flushes_can_be_cleared) { - _flush_history.add_pending_flush(HANDLER1, "a1", 3s); + _flush_history.add_pending_flush(HANDLER1, "a1", strategy_info, 3s); _flush_history.clear_pending_flushes(); auto view = _flush_history.make_view(); EXPECT_TRUE(view->pending().empty()); @@ -210,12 +211,12 @@ TEST_F(FlushHistoryTest, flush_strategy_can_be_changed) { * { "all", id = 43, priority_strategy = true } started 2 flushes: handler2.a2 and handler1.a3 * { "normal", id = 44, priority_strategy = false } started no flushes */ - _flush_history.start_flush(HANDLER1, "a1", 3s, 5); + _flush_history.start_flush(HANDLER1, "a1", strategy_info, 3s, 5); _flush_history.set_strategy(ALL_STRATEGY, 43, true); - _flush_history.add_pending_flush(HANDLER2, "a2", 1s); - _flush_history.add_pending_flush(HANDLER1, "a3", 4s); - _flush_history.start_flush(HANDLER2, "a2", 1s, 6); - _flush_history.start_flush(HANDLER1, "a3", 4s, 7); + _flush_history.add_pending_flush(HANDLER2, "a2", strategy_info, 1s); + _flush_history.add_pending_flush(HANDLER1, "a3", strategy_info, 4s); + _flush_history.start_flush(HANDLER2, "a2", strategy_info, 1s, 6); + _flush_history.start_flush(HANDLER1, "a3", strategy_info, 4s, 7); _flush_history.set_strategy(NORMAL_STRATEGY, 44, false); /* diff --git a/searchcore/src/tests/proton/flushengine/flushengine_test.cpp b/searchcore/src/tests/proton/flushengine/flushengine_test.cpp index 096f82c9cd15..477e7ae74931 100644 --- a/searchcore/src/tests/proton/flushengine/flushengine_test.cpp +++ b/searchcore/src/tests/proton/flushengine/flushengine_test.cpp @@ -353,6 +353,7 @@ class SimpleStrategy : public IFlushStrategy { enum class OrderBy { INDEX_OF, SERIAL }; std::vector _targets; OrderBy _orderBy; + bool _priority_strategy; struct CompareIndexOf { CompareIndexOf(const SimpleStrategy& flush) : _flush(flush) {} @@ -362,8 +363,8 @@ class SimpleStrategy : public IFlushStrategy { const SimpleStrategy& _flush; }; - FlushContext::List getFlushTargets(const FlushContext::List& targetList, const flushengine::TlsStatsMap&, - const flushengine::ActiveFlushStats&) const override { + FlushStrategyResult getFlushTargets(const FlushContext::List& targetList, const flushengine::TlsStatsMap&, + const flushengine::ActiveFlushStats&) const override { FlushContext::List fv(targetList); if (_orderBy == OrderBy::INDEX_OF) { std::sort(fv.begin(), fv.end(), CompareIndexOf(*this)); @@ -372,17 +373,28 @@ class SimpleStrategy : public IFlushStrategy { return a->getTarget()->getFlushedSerialNum() < b->getTarget()->getFlushedSerialNum(); }); } - return fv; + return FlushStrategyResult(std::move(fv), name(), _id, _priority_strategy, order_name()); } std::string name() const override { return "flush_simple"; } + std::string order_name() const noexcept { + switch (_orderBy) { + case OrderBy::INDEX_OF: + return "index_of"; + case OrderBy::SERIAL: + default: + return "serial"; + } + } + bool compare(const IFlushTarget::SP& lhs, const IFlushTarget::SP& rhs) const { LOG(info, "SimpleStrategy::compare(%p, %p)", lhs.get(), rhs.get()); return indexOf(lhs) < indexOf(rhs); } - SimpleStrategy(OrderBy orderBy) noexcept : _targets(), _orderBy(orderBy) {} + SimpleStrategy(OrderBy orderBy, bool priority_strategy) noexcept + : _targets(), _orderBy(orderBy), _priority_strategy(priority_strategy) {} uint32_t indexOf(const IFlushTarget::SP& target) const { IFlushTarget* raw = target.get(); @@ -407,10 +419,10 @@ class SimpleStrategy : public IFlushStrategy { class NoFlushStrategy : public SimpleStrategy { public: - NoFlushStrategy() noexcept : SimpleStrategy(OrderBy::INDEX_OF) {} - FlushContext::List getFlushTargets(const FlushContext::List&, const flushengine::TlsStatsMap&, - const flushengine::ActiveFlushStats&) const override { - return {}; + NoFlushStrategy() noexcept : SimpleStrategy(OrderBy::INDEX_OF, false) {} + FlushStrategyResult getFlushTargets(const FlushContext::List&, const flushengine::TlsStatsMap&, + const flushengine::ActiveFlushStats&) const override { + return FlushStrategyResult({}, name(), _id, false, name()); } std::string name() const override { return "flush_nothing"; } }; @@ -446,7 +458,8 @@ struct Fixture { engine(tlsStatsFactory, strategy, numThreads, idleInterval, std::numeric_limits::max()) {} Fixture(uint32_t numThreads, vespalib::duration idleInterval) - : Fixture(numThreads, idleInterval, std::make_shared(SimpleStrategy::OrderBy::INDEX_OF)) {} + : Fixture(numThreads, idleInterval, + std::make_shared(SimpleStrategy::OrderBy::INDEX_OF, false)) {} ~Fixture(); @@ -737,7 +750,7 @@ TEST(FlushEngineTest, require_that_concurrency_works) { } TEST(FlushEngineTest, require_that_there_is_room_for_one_and_only_one_high_pri_target) { - Fixture f(2, 1ms, std::make_unique(SimpleStrategy::OrderBy::SERIAL)); + Fixture f(2, 1ms, std::make_unique(SimpleStrategy::OrderBy::SERIAL, false)); auto target1 = std::make_shared("target1", 1, false); auto target2 = std::make_shared("target2", 2, false); auto target3 = std::make_shared("target3", 3, false); @@ -770,7 +783,7 @@ TEST(FlushEngineTest, require_that_there_is_room_for_one_and_only_one_high_pri_t } TEST(FlushEngineTest, require_that_high_priority_does_not_jump_the_queue) { - Fixture f(2, 1ms, std::make_unique(SimpleStrategy::OrderBy::SERIAL)); + Fixture f(2, 1ms, std::make_unique(SimpleStrategy::OrderBy::SERIAL, false)); auto target1 = std::make_shared("target1", 1, false); auto target2 = std::make_shared("target2", 2, false); auto target3 = std::make_shared("target3", 3, false); @@ -872,7 +885,7 @@ TEST(FlushEngineTest, require_that_oldest_serial_is_updated_when_finishing_prior auto target1 = std::make_shared("target1", 10, true); auto handler = f.addSimpleHandler({target1}); f.assertOldestSerial(*handler, 10); - f.engine.set_strategy(std::make_shared(SimpleStrategy::OrderBy::INDEX_OF)).wait(); + f.engine.set_strategy(std::make_shared(SimpleStrategy::OrderBy::INDEX_OF, true)).wait(); EXPECT_EQ(20u, handler->_oldestSerial); } diff --git a/searchcore/src/tests/proton/flushengine/prepare_restart_flush_strategy/prepare_restart_flush_strategy_test.cpp b/searchcore/src/tests/proton/flushengine/prepare_restart_flush_strategy/prepare_restart_flush_strategy_test.cpp index 25a5e104a0e3..96dd66be90f7 100644 --- a/searchcore/src/tests/proton/flushengine/prepare_restart_flush_strategy/prepare_restart_flush_strategy_test.cpp +++ b/searchcore/src/tests/proton/flushengine/prepare_restart_flush_strategy/prepare_restart_flush_strategy_test.cpp @@ -12,6 +12,7 @@ #include using namespace proton; +using proton::flushengine::FlushStrategyResult; using search::SerialNum; using searchcorespi::IFlushTarget; @@ -201,7 +202,7 @@ struct FlushStrategyFixture { [[nodiscard]] FlushContext::List getFlushTargets(const FlushContext::List& targetList, const flushengine::TlsStatsMap& tlsStatsMap) const { flushengine::ActiveFlushStats active_flushes; - return strategy.getFlushTargets(targetList, tlsStatsMap, active_flushes); + return strategy.getFlushTargets(targetList, tlsStatsMap, active_flushes).list(); } }; diff --git a/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp b/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp index acf6b91c1a8a..95ab4916c699 100644 --- a/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp +++ b/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp @@ -18,6 +18,7 @@ using DiskGain = IFlushTarget::DiskGain; class MyFlushHandler : public IFlushHandler { SerialNum _current_serial; + public: MyFlushHandler(const std::string& name) noexcept : IFlushHandler(name), _current_serial(100) {} ~MyFlushHandler() override; @@ -99,10 +100,10 @@ class ContextBuilder { return flushengine::TlsStatsMap(std::move(map)); } FlushContext::List flush_targets(const IFlushStrategy& strategy) const { - return strategy.getFlushTargets(list(), tlsStats(), _active_flushes); + return strategy.getFlushTargets(list(), tlsStats(), _active_flushes).list(); } [[nodiscard]] StringList flush_target_names(const IFlushStrategy& strategy) const { - auto ctx_list = flush_targets(strategy); + auto ctx_list = flush_targets(strategy); StringList target_names; target_names.reserve(ctx_list.size()); for (auto& ctx : ctx_list) { diff --git a/searchcore/src/tests/proton/server/prepare_restart2_rpc_handler_test.cpp b/searchcore/src/tests/proton/server/prepare_restart2_rpc_handler_test.cpp index c1cf28b6327a..d943e61e4105 100644 --- a/searchcore/src/tests/proton/server/prepare_restart2_rpc_handler_test.cpp +++ b/searchcore/src/tests/proton/server/prepare_restart2_rpc_handler_test.cpp @@ -231,6 +231,8 @@ std::shared_ptr make_flush_strategy_id_notifier() { return notifier; } +const std::string strategy_info("info"); + } // namespace class PrepareRestart2RpcHandlerTest : public ::testing::Test { @@ -476,13 +478,13 @@ TEST_F(PrepareRestart2RpcHandlerTest, poll_sequence) { * { "prepare_restart", id = 201, priority_strategy = true } started 2 flushes: handler2.a2 and handler1.a3 * and set 1 flush as pending: handler1.a4 */ - _history->start_flush(HANDLER1, "a1", 3s, 5); + _history->start_flush(HANDLER1, "a1", strategy_info, 3s, 5); _history->set_strategy(PREPARE_RESTART_STRATEGY, 201, true); - _history->add_pending_flush(HANDLER2, "a2", 1s); - _history->add_pending_flush(HANDLER1, "a3", 4s); - _history->add_pending_flush(HANDLER1, "a4", 1s); - _history->start_flush(HANDLER2, "a2", 1s, 6); - _history->start_flush(HANDLER1, "a3", 4s, 7); + _history->add_pending_flush(HANDLER2, "a2", strategy_info, 1s); + _history->add_pending_flush(HANDLER1, "a3", strategy_info, 4s); + _history->add_pending_flush(HANDLER1, "a4", strategy_info, 1s); + _history->start_flush(HANDLER2, "a2", strategy_info, 1s, 6); + _history->start_flush(HANDLER1, "a3", strategy_info, 4s, 7); auto future = test_handler(1, 0s); future.wait(); @@ -502,7 +504,7 @@ TEST_F(PrepareRestart2RpcHandlerTest, poll_sequence) { */ _history->flush_done(6); _history->prune_done(6); - _history->start_flush(HANDLER1, "a4", 1s, 8); + _history->start_flush(HANDLER1, "a4", strategy_info, 1s, 8); _history->set_strategy(MEMORY_STRATEGY, 202, false); _return_handler->alloc_req(); future = test_handler(1, 0s); diff --git a/searchcore/src/vespa/searchcore/proton/common/eventlogger.cpp b/searchcore/src/vespa/searchcore/proton/common/eventlogger.cpp index 12c2bf2afe43..53ee70473a9b 100644 --- a/searchcore/src/vespa/searchcore/proton/common/eventlogger.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/eventlogger.cpp @@ -74,10 +74,12 @@ void EventLogger::transactionLogReplayComplete(const string& domainName, vespali doTransactionLogReplayComplete(domainName, elapsedTime, "transactionlog.replay.complete"); } -void EventLogger::flushInit(const string& name) { +void EventLogger::flushInit(const string& name, const std::string& strategy_name, const std::string& strategy_info) { JSONStringer jstr; jstr.beginObject(); jstr.appendKey("name").appendString(name); + jstr.appendKey("strategy_name").appendString(strategy_name); + jstr.appendKey("strategy_info").appendString(strategy_info); jstr.endObject(); EV_STATE("flush.init", jstr.str().c_str()); } diff --git a/searchcore/src/vespa/searchcore/proton/common/eventlogger.h b/searchcore/src/vespa/searchcore/proton/common/eventlogger.h index 88f16411106f..3f09b2a69de2 100644 --- a/searchcore/src/vespa/searchcore/proton/common/eventlogger.h +++ b/searchcore/src/vespa/searchcore/proton/common/eventlogger.h @@ -35,7 +35,7 @@ class EventLogger { static void transactionLogReplayStart(const string& domainName, SerialNum first, SerialNum last); static void transactionLogReplayProgress(const string& domainName, float progress, SerialNum first, SerialNum last, SerialNum current); - static void flushInit(const string& name); + static void flushInit(const string& name, const std::string& strategy_name, const std::string& strategy_info); static void flushStart(const string& name, int64_t beforeMemory, int64_t afterMemory, int64_t toFreeMemory, SerialNum unflushed, SerialNum current); static void flushComplete(const string& name, vespalib::duration elapsedTime, SerialNum flushed, diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/flushengine/CMakeLists.txt index c918755f7ffd..72fce12689f5 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/flushengine/CMakeLists.txt @@ -16,6 +16,7 @@ vespa_add_library(searchcore_flushengine STATIC flush_strategy_history_entry.cpp flush_strategy_id_listener.cpp flush_strategy_id_notifier.cpp + flush_strategy_result.cpp flush_target_candidate.cpp flush_target_candidates.cpp flushtargetproxy.cpp diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_all_strategy.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flush_all_strategy.cpp index db3c80fab0da..8872a0efc56f 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flush_all_strategy.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_all_strategy.cpp @@ -4,6 +4,7 @@ #include +using proton::flushengine::FlushStrategyResult; using search::SerialNum; using searchcorespi::IFlushTarget; @@ -24,21 +25,22 @@ bool CompareTarget::operator()(const FlushContext::SP& lfc, const FlushContext:: } std::string strategy_name("flush_all"); +std::string strategy_info("all"); } // namespace FlushAllStrategy::FlushAllStrategy() : IFlushStrategy() { } -FlushContext::List FlushAllStrategy::getFlushTargets(const FlushContext::List& targetList, - const flushengine::TlsStatsMap&, - const flushengine::ActiveFlushStats&) const { +FlushStrategyResult FlushAllStrategy::getFlushTargets(const FlushContext::List& targetList, + const flushengine::TlsStatsMap&, + const flushengine::ActiveFlushStats&) const { if (targetList.empty()) { - return {}; + return FlushStrategyResult({}, strategy_name, _id, true, strategy_info); } FlushContext::List fv(targetList); std::sort(fv.begin(), fv.end(), CompareTarget()); - return fv; + return FlushStrategyResult(std::move(fv), strategy_name, _id, true, strategy_info); } std::string FlushAllStrategy::name() const { diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_all_strategy.h b/searchcore/src/vespa/searchcore/proton/flushengine/flush_all_strategy.h index a540f03ceff0..7493e7ef2e06 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flush_all_strategy.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_all_strategy.h @@ -13,8 +13,9 @@ class FlushAllStrategy : public IFlushStrategy { public: FlushAllStrategy(); - FlushContext::List getFlushTargets(const FlushContext::List& targetList, const flushengine::TlsStatsMap&, - const flushengine::ActiveFlushStats&) const override; + flushengine::FlushStrategyResult getFlushTargets(const FlushContext::List& targetList, + const flushengine::TlsStatsMap&, + const flushengine::ActiveFlushStats&) const override; std::string name() const override; }; diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_history.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flush_history.cpp index 6416ec412ada..34766df1aeb3 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flush_history.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_history.cpp @@ -109,7 +109,7 @@ void FlushHistory::strategy_flush_done(uint32_t strategy_id, time_point now) { } void FlushHistory::start_flush(const std::string& handler_name, const std::string& target_name, - duration last_flush_duration, uint32_t id) { + const std::string& strategy_info, duration last_flush_duration, uint32_t id) { // Note: this member function is called when queueing flush engine task, initFlush has already completed. auto name = build_name(handler_name, target_name); std::lock_guard guard(_mutex); @@ -122,7 +122,8 @@ void FlushHistory::start_flush(const std::string& handler_name, const std::strin } else { auto now = steady_clock::now(); active_it = _active.emplace_hint( - active_it, id, FlushHistoryEntry(name, _active_strategy, now, last_flush_duration, ++_pending_id)); + active_it, id, + FlushHistoryEntry(name, _active_strategy, strategy_info, now, last_flush_duration, ++_pending_id)); } _active_strategy.start_flush(); auto now = steady_clock::now(); @@ -152,17 +153,19 @@ void FlushHistory::prune_done(uint32_t id) { } void FlushHistory::add_pending_flush(const std::string& handler_name, const std::string& target_name, - duration last_flush_duration) { + const std::string& strategy_info, duration last_flush_duration) { // Called when priority flush strategy is used. auto name = build_name(handler_name, target_name); std::lock_guard guard(_mutex); auto pending_it = _pending.lower_bound(name); auto now = steady_clock::now(); if (pending_it != _pending.end() && pending_it->first == name) { - pending_it->second = FlushHistoryEntry(name, _active_strategy, now, last_flush_duration, ++_pending_id); + pending_it->second = + FlushHistoryEntry(name, _active_strategy, strategy_info, now, last_flush_duration, ++_pending_id); } else { - _pending.emplace_hint(pending_it, name, - FlushHistoryEntry(name, _active_strategy, now, last_flush_duration, ++_pending_id)); + _pending.emplace_hint( + pending_it, name, + FlushHistoryEntry(name, _active_strategy, strategy_info, now, last_flush_duration, ++_pending_id)); } } diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_history.h b/searchcore/src/vespa/searchcore/proton/flushengine/flush_history.h index 43b74ed63f6a..f0caeea7ecb9 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flush_history.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_history.h @@ -75,12 +75,12 @@ class FlushHistory { ~FlushHistory(); FlushHistory& operator=(const FlushHistory&) = delete; FlushHistory& operator=(FlushHistory&&) = delete; - void start_flush(const std::string& handler_name, const std::string& target_name, duration last_flush_duration, - uint32_t id); + void start_flush(const std::string& handler_name, const std::string& target_name, + const std::string& strategy_info, duration last_flush_duration, uint32_t id); void flush_done(uint32_t id); void prune_done(uint32_t id); void add_pending_flush(const std::string& handler_name, const std::string& target_name, - duration last_flush_duration); + const std::string& strategy_info, duration last_flush_duration); void drop_pending_flush(const std::string& handler_name, const std::string& target_name); void clear_pending_flushes(); void set_strategy(std::string stategy, uint32_t strategy_id, bool priority_strategy); diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_entry.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_entry.cpp index f07bdbab9b60..d2a8cf179857 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_entry.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_entry.cpp @@ -7,12 +7,13 @@ namespace proton::flushengine { FlushHistoryEntry::FlushHistoryEntry(std::string name_in, std::string strategy_in, uint32_t strategy_id_in, - bool priority_strategy_in, time_point create_time_in, - duration last_flush_duration_in, uint32_t id_in) + bool priority_strategy_in, const std::string& strategy_info_in, + time_point create_time_in, duration last_flush_duration_in, uint32_t id_in) : _name(std::move(name_in)), _strategy(std::move(strategy_in)), _strategy_id(strategy_id_in), _priority_strategy(priority_strategy_in), + _strategy_info(strategy_info_in), _create_time(create_time_in), _start_time(), _finish_time(), @@ -22,9 +23,11 @@ FlushHistoryEntry::FlushHistoryEntry(std::string name_in, std::string strategy_i } FlushHistoryEntry::FlushHistoryEntry(std::string name_in, const FlushStrategyHistoryEntry& strategy_entry, - time_point create_time_in, duration last_flush_duration_in, uint32_t id_in) + const std::string& strategy_info_in, time_point create_time_in, + duration last_flush_duration_in, uint32_t id_in) : FlushHistoryEntry(std::move(name_in), strategy_entry.name(), strategy_entry.id(), - strategy_entry.priority_strategy(), create_time_in, last_flush_duration_in, id_in) { + strategy_entry.priority_strategy(), strategy_info_in, create_time_in, last_flush_duration_in, + id_in) { } FlushHistoryEntry::FlushHistoryEntry(const FlushHistoryEntry&) = default; diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_entry.h b/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_entry.h index 789b070a9373..ad15c81920a2 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_entry.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_entry.h @@ -21,6 +21,7 @@ class FlushHistoryEntry { std::string _strategy; uint32_t _strategy_id; bool _priority_strategy; + std::string _strategy_info; time_point _create_time; time_point _start_time; time_point _finish_time; @@ -30,28 +31,30 @@ class FlushHistoryEntry { public: FlushHistoryEntry(std::string name_in, std::string strategy_in, uint32_t strategy_id_in, - bool priority_strategy_in, time_point create_time_in, duration last_flush_duration_in, - uint32_t id_in); - FlushHistoryEntry(std::string name_in, const FlushStrategyHistoryEntry& strategy_entry, time_point create_time_in, + bool priority_strategy_in, const std::string& strategy_info_in, time_point create_time_in, duration last_flush_duration_in, uint32_t id_in); + FlushHistoryEntry(std::string name_in, const FlushStrategyHistoryEntry& strategy_entry, + const std::string& strategy_info_in, time_point create_time_in, duration last_flush_duration_in, + uint32_t id_in); FlushHistoryEntry(const FlushHistoryEntry&); FlushHistoryEntry(FlushHistoryEntry&&) noexcept; ~FlushHistoryEntry(); FlushHistoryEntry& operator=(const FlushHistoryEntry&); FlushHistoryEntry& operator=(FlushHistoryEntry&&) noexcept; - const std::string& name() const noexcept { return _name; } - const std::string& strategy() const noexcept { return _strategy; } - uint32_t strategy_id() const noexcept { return _strategy_id; } - bool priority_strategy() const noexcept { return _priority_strategy; } - time_point create_time() const noexcept { return _create_time; } - time_point start_time() const noexcept { return _start_time; } - time_point finish_time() const noexcept { return _finish_time; } - time_point prune_time() const noexcept { return _prune_time; } - duration flush_duration() const noexcept { + [[nodiscard]] const std::string& name() const noexcept { return _name; } + [[nodiscard]] const std::string& strategy() const noexcept { return _strategy; } + [[nodiscard]] uint32_t strategy_id() const noexcept { return _strategy_id; } + [[nodiscard]] bool priority_strategy() const noexcept { return _priority_strategy; } + [[nodiscard]] const std::string& strategy_info() const noexcept { return _strategy_info; } + [[nodiscard]] time_point create_time() const noexcept { return _create_time; } + [[nodiscard]] time_point start_time() const noexcept { return _start_time; } + [[nodiscard]] time_point finish_time() const noexcept { return _finish_time; } + [[nodiscard]] time_point prune_time() const noexcept { return _prune_time; } + [[nodiscard]] duration flush_duration() const noexcept { return _finish_time != time_point() ? _finish_time - _start_time : duration(); } - duration last_flush_duration() const noexcept { return _last_flush_duration; } - uint32_t id() const noexcept { return _id; } + [[nodiscard]] duration last_flush_duration() const noexcept { return _last_flush_duration; } + [[nodiscard]] uint32_t id() const noexcept { return _id; } void start_flush(time_point start_time_in, uint32_t id_in) noexcept; void flush_done(time_point finish_time_in) noexcept; void prune_done(time_point prune_time_in) noexcept; diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_explorer.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_explorer.cpp index a3b172543857..4da706c90d90 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_explorer.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_history_explorer.cpp @@ -39,6 +39,7 @@ void convert_to_slime(const FlushHistoryEntry& entry, Inserter& inserter) { object.setString("strategy", entry.strategy()); object.setLong("strategy_id", entry.strategy_id()); object.setBool("priority_strategy", entry.priority_strategy()); + object.setString("strategy_info", entry.strategy_info()); object.setLong("create_time", as_system_microseconds(entry.create_time())); if (entry.start_time() != steady_clock::time_point()) { object.setLong("start_time_usecs", as_system_microseconds(entry.start_time())); diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_strategy_result.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flush_strategy_result.cpp new file mode 100644 index 000000000000..8c3d9d8bbfed --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_strategy_result.cpp @@ -0,0 +1,33 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "flush_strategy_result.h" + +#include "flushcontext.h" + +#include + +using searchcorespi::IFlushTarget; + +namespace proton::flushengine { + +FlushStrategyResult::FlushStrategyResult(std::vector> list_in, + std::string strategy_name_in, uint32_t strategy_id_in, + bool priority_strategy_in, std::string strategy_info_in) + : _list(std::move(list_in)), + _strategy_name(std::move(strategy_name_in)), + _strategy_id(strategy_id_in), + _priority_strategy(priority_strategy_in), + _strategy_info(std::move(strategy_info_in)) { +} + +FlushStrategyResult::~FlushStrategyResult() = default; + +void FlushStrategyResult::drop_non_high_priority_targets() { + std::vector> high_pri_list; + if (_list.front()->getTarget()->getPriority() > IFlushTarget::Priority::NORMAL) { + high_pri_list.push_back(_list.front()); + } + std::swap(high_pri_list, _list); +} + +} // namespace proton::flushengine diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flush_strategy_result.h b/searchcore/src/vespa/searchcore/proton/flushengine/flush_strategy_result.h new file mode 100644 index 000000000000..dca93d504f39 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flush_strategy_result.h @@ -0,0 +1,42 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include +#include +#include + +namespace proton { +class FlushContext; +} + +namespace proton::flushengine { + +/* + * This class contains a sorted list of flush contexts and tracking info used for initiating + * flushing, updating flush history and logging events for flush operations. + */ +class FlushStrategyResult { + std::vector> _list; + std::string _strategy_name; + uint32_t _strategy_id; + bool _priority_strategy; + std::string _strategy_info; + +public: + explicit FlushStrategyResult(std::vector> list_in, std::string strategy_name_in, + uint32_t strategy_id_in, bool priority_strategy_in, std::string stratewgy_info_in); + FlushStrategyResult(const FlushStrategyResult&) = delete; + FlushStrategyResult(FlushStrategyResult&&) = default; + ~FlushStrategyResult(); + FlushStrategyResult& operator=(const FlushStrategyResult&) = delete; + FlushStrategyResult& operator=(FlushStrategyResult&&) = default; + void drop_non_high_priority_targets(); + [[nodiscard]] const std::vector>& list() const noexcept { return _list; } + [[nodiscard]] const std::string& strategy_name() const noexcept { return _strategy_name; } + [[nodiscard]] uint32_t strategy_id() const noexcept { return _strategy_id; } + [[nodiscard]] bool priority_strategy() const noexcept { return _priority_strategy; } + [[nodiscard]] const std::string& strategy_info() const noexcept { return _strategy_info; } +}; + +} // namespace proton::flushengine diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp index 6f5c462b0838..ee953b700dbb 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp @@ -25,6 +25,7 @@ using Task = vespalib::Executor::Task; using proton::flushengine::FlushHistory; using proton::flushengine::FlushHistoryEntry; using proton::flushengine::FlushStrategyIdNotifier; +using proton::flushengine::FlushStrategyResult; using proton::flushengine::SetStrategyResult; using searchcorespi::FlushStats; using searchcorespi::IFlushTarget; @@ -121,6 +122,7 @@ FlushEngine::FlushEngine(std::shared_ptr tlsStats _flush_history(std::make_shared(_strategy->name(), _strategy_id, _maxConcurrentNormal)), _max_summary_file_size(max_summary_file_size) { _flushing_strategies[_strategy_id] = 1u; // Account for initial flush strategy + _strategy->set_id(_strategy_id); } FlushEngine::~FlushEngine() { @@ -197,20 +199,15 @@ bool FlushEngine::has_slot(IFlushTarget::Priority priority) { std::string FlushEngine::checkAndFlush(std::string prev) { auto lst = getSortedTargetList(); - if (lst._priority_flush) { + if (lst.priority_strategy()) { // Everything returned from a priority strategy should be flushed - flushAll(lst._ctx_list, lst._strategy_id); + flushAll(lst); return "[priority_targets]"; // prevent idle_wait in FlushEngine::run() - } else if (!lst._ctx_list.empty()) { - if (has_slot(IFlushTarget::Priority::NORMAL)) { - prev = flushNextTarget(prev, lst._ctx_list, lst._strategy_id); - } else { - FlushContext::List highPri; - if (lst._ctx_list.front()->getTarget()->getPriority() > IFlushTarget::Priority::NORMAL) { - highPri.push_back(lst._ctx_list.front()); - } - prev = flushNextTarget(prev, highPri, lst._strategy_id); + } else if (!lst.list().empty()) { + if (!has_slot(IFlushTarget::Priority::NORMAL)) { + lst.drop_non_high_priority_targets(); } + prev = flushNextTarget(prev, lst); if (!prev.empty()) { // Sleep 1 ms after a successful flush in order to avoid busy loop in case // of strategy or target error. @@ -326,6 +323,7 @@ void FlushEngine::maybe_apply_changed_strategy(std::vector& str _strategy_id = _priorityStrategy->get_id(); } else { ++_strategy_id; + _strategy->set_id(_strategy_id); } _flush_history->clear_pending_flushes(); _flush_history->set_strategy(std::move(strategy_name), _strategy_id, priority_strategy); @@ -391,21 +389,16 @@ flushengine::ActiveFlushStats make_active_flushes(const FlushEngine::FlushMetaSe } // namespace -FlushEngine::BoundFlushContextList FlushEngine::getSortedTargetList() { +FlushStrategyResult FlushEngine::getSortedTargetList() { auto unsortedTargets = getTargetList(false); auto tlsStatsMap = _tlsStatsFactory->create(); auto active_flushes = make_active_flushes(getCurrentlyFlushingSet()); std::vector strategy_ids_for_finished_flushes; std::unique_lock strategy_guard(_strategyLock); maybe_apply_changed_strategy(strategy_ids_for_finished_flushes, strategy_guard); - BoundFlushContextList ret; - if (_priorityStrategy) { - ret = BoundFlushContextList(_priorityStrategy->getFlushTargets(unsortedTargets, tlsStatsMap, active_flushes), - _strategy_id, true); - } else { - ret = BoundFlushContextList(_strategy->getFlushTargets(unsortedTargets, tlsStatsMap, active_flushes), - _strategy_id, false); - } + FlushStrategyResult ret = _priorityStrategy + ? _priorityStrategy->getFlushTargets(unsortedTargets, tlsStatsMap, active_flushes) + : _strategy->getFlushTargets(unsortedTargets, tlsStatsMap, active_flushes); strategy_guard.unlock(); prune_flushing_strategies(std::move(strategy_ids_for_finished_flushes)); return ret; @@ -419,11 +412,14 @@ std::shared_ptr FlushEngine::get_flush_token(const FlushCon } } -FlushContext::SP FlushEngine::initNextFlush(const FlushContext::List& lst) { +FlushContext::SP FlushEngine::initNextFlush(const FlushStrategyResult& flush_strategy_result) { + auto& lst = flush_strategy_result.list(); FlushContext::SP ctx; for (const FlushContext::SP& it : lst) { if (LOG_WOULD_LOG(event)) { - EventLogger::flushInit(it->getName()); + auto& strategy_info = flush_strategy_result.strategy_info(); + auto& strategy_name = flush_strategy_result.strategy_name(); + EventLogger::flushInit(it->getName(), strategy_name, strategy_info); } if (it->initFlush(get_flush_token(*it))) { ctx = it; @@ -436,17 +432,21 @@ FlushContext::SP FlushEngine::initNextFlush(const FlushContext::List& lst) { return ctx; } -void FlushEngine::flushAll(const FlushContext::List& lst, uint32_t strategy_id) { +void FlushEngine::flushAll(const FlushStrategyResult& flush_strategy_result) { + auto& lst = flush_strategy_result.list(); + auto& strategy_info = flush_strategy_result.strategy_info(); LOG(debug, "%ld targets to flush.", lst.size()); for (const FlushContext::SP& ctx : lst) { - _flush_history->add_pending_flush(ctx->getHandler()->getName(), ctx->getTarget()->getName(), + _flush_history->add_pending_flush(ctx->getHandler()->getName(), ctx->getTarget()->getName(), strategy_info, ctx->getTarget()->last_flush_duration()); } + auto strategy_id = flush_strategy_result.strategy_id(); for (const FlushContext::SP& ctx : lst) { if (wait_for_slot(IFlushTarget::Priority::NORMAL)) { if (ctx->initFlush(get_flush_token(*ctx))) { logTarget("initiated", *ctx); - _executor.execute(std::make_unique(initFlush(*ctx, strategy_id), *this, ctx)); + _executor.execute( + std::make_unique(initFlush(*ctx, strategy_id, strategy_info), *this, ctx)); } else { logTarget("failed to initiate", *ctx); _flush_history->drop_pending_flush(ctx->getHandler()->getName(), ctx->getTarget()->getName()); @@ -463,13 +463,12 @@ void FlushEngine::flushAll(const FlushContext::List& lst, uint32_t strategy_id) } } -std::string FlushEngine::flushNextTarget(const std::string& name, const FlushContext::List& contexts, - uint32_t strategy_id) { - if (contexts.empty()) { +std::string FlushEngine::flushNextTarget(const std::string& name, const FlushStrategyResult& flush_strategy_result) { + if (flush_strategy_result.list().empty()) { LOG(debug, "No target to flush."); return ""; } - FlushContext::SP ctx = initNextFlush(contexts); + FlushContext::SP ctx = initNextFlush(flush_strategy_result); if (!ctx) { LOG(debug, "All targets refused to flush."); return ""; @@ -478,21 +477,23 @@ std::string FlushEngine::flushNextTarget(const std::string& name, const FlushCon LOG(info, "The same target %s out of %ld has been asked to flush again. " "This might indicate flush logic flaw so I will wait 100 ms before doing it.", - name.c_str(), contexts.size()); + name.c_str(), flush_strategy_result.list().size()); std::this_thread::sleep_for(100ms); } - _executor.execute(std::make_unique(initFlush(*ctx, strategy_id), *this, ctx)); + auto strategy_id = flush_strategy_result.strategy_id(); + auto& strategy_info = flush_strategy_result.strategy_info(); + _executor.execute(std::make_unique(initFlush(*ctx, strategy_id, strategy_info), *this, ctx)); return ctx->getName(); } -uint32_t FlushEngine::initFlush(const FlushContext& ctx, uint32_t strategy_id) { +uint32_t FlushEngine::initFlush(const FlushContext& ctx, uint32_t strategy_id, const std::string& strategy_info) { if (LOG_WOULD_LOG(event)) { IFlushTarget::MemoryGain mgain(ctx.getTarget()->getApproxMemoryGain()); EventLogger::flushStart(ctx.getName(), mgain.getBefore(), mgain.getAfter(), mgain.gain(), ctx.getTarget()->getFlushedSerialNum() + 1, ctx.getHandler()->getCurrentSerialNumber()); } - return initFlush(ctx.getHandler(), ctx.getTarget(), strategy_id); + return initFlush(ctx.getHandler(), ctx.getTarget(), strategy_id, strategy_info); } void FlushEngine::flushDone(const FlushContext& ctx, uint32_t taskId) { @@ -582,18 +583,19 @@ FlushEngine::FlushMetaSet FlushEngine::getCurrentlyFlushingSet() const { } uint32_t FlushEngine::initFlush(const IFlushHandler::SP& handler, const IFlushTarget::SP& target, - uint32_t strategy_id) { + uint32_t strategy_id, const std::string& strategy_info) { uint32_t taskId; { std::lock_guard guard(_lock); taskId = _taskId++; FlushInfo flush(taskId, handler->getName(), target, strategy_id); _flushing[taskId] = flush; - _flush_history->start_flush(handler->getName(), target->getName(), target->last_flush_duration(), taskId); + _flush_history->start_flush(handler->getName(), target->getName(), strategy_info, + target->last_flush_duration(), taskId); mark_active_strategy(strategy_id, guard); } - LOG(debug, "FlushEngine::initFlush(handler='%s', target='%s') => taskId='%d'", handler->getName().c_str(), - target->getName().c_str(), taskId); + LOG(debug, "FlushEngine::initFlush(handler='%s', target='%s', strategy_info='%s') => taskId='%d'", + handler->getName().c_str(), target->getName().c_str(), strategy_info.c_str(), taskId); return taskId; } diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h index 03664e0c97ec..eb5bf95f62ec 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.h @@ -71,15 +71,6 @@ class FlushEngine : public IReservedDiskSpaceProvider { PruneMeta(uint32_t flush_id, uint32_t strategy_id) noexcept : _flush_id(flush_id), _strategy_id(strategy_id) {} }; - struct BoundFlushContextList { - FlushContext::List _ctx_list; - uint32_t _strategy_id; - bool _priority_flush; - - BoundFlushContextList() : _ctx_list(), _strategy_id(0), _priority_flush(false) {} - BoundFlushContextList(FlushContext::List ctx_list, uint32_t strategy_id, bool priority_flush) noexcept - : _ctx_list(std::move(ctx_list)), _strategy_id(strategy_id), _priority_flush(priority_flush) {} - }; using FlushMap = std::map; using FlushHandlerMap = HandlerMap; using PendingPrunes = std::map, std::vector>; @@ -115,11 +106,12 @@ class FlushEngine : public IReservedDiskSpaceProvider { std::atomic _max_summary_file_size; FlushContext::List getTargetList(bool includeFlushingTargets) const; - BoundFlushContextList getSortedTargetList(); + flushengine::FlushStrategyResult getSortedTargetList(); std::shared_ptr get_flush_token(const FlushContext& ctx); - FlushContext::SP initNextFlush(const FlushContext::List& lst); - std::string flushNextTarget(const std::string& name, const FlushContext::List& contexts, uint32_t strategy_id); - void flushAll(const FlushContext::List& lst, uint32_t strategy_id); + FlushContext::SP initNextFlush(const flushengine::FlushStrategyResult& flush_strategy_result); + std::string flushNextTarget(const std::string& name, + const flushengine::FlushStrategyResult& flush_strategy_result); + void flushAll(const flushengine::FlushStrategyResult& flush_strategy_result); bool prune(); void prune_done(std::vector& strategy_ids_for_finished_flushes, const std::vector& prune_metas); @@ -127,8 +119,9 @@ class FlushEngine : public IReservedDiskSpaceProvider { void maybe_apply_changed_strategy(std::vector& strategy_ids_for_finished_flushes, std::unique_lock& strategy_guard); void mark_active_strategy(uint32_t strategy_id, std::lock_guard&); - uint32_t initFlush(const FlushContext& ctx, uint32_t strategy_id); - uint32_t initFlush(const IFlushHandler::SP& handler, const IFlushTarget::SP& target, uint32_t strategy_id); + uint32_t initFlush(const FlushContext& ctx, uint32_t strategy_id, const std::string& strategy_info); + uint32_t initFlush(const IFlushHandler::SP& handler, const IFlushTarget::SP& target, uint32_t strategy_id, + const std::string& strategy_info); void flushDone(const FlushContext& ctx, uint32_t taskId); bool canFlushMore(const std::unique_lock& guard, IFlushTarget::Priority priority) const; void wait_for_slot_or_pending_prune(IFlushTarget::Priority priority); diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h b/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h index 2e2d4c551445..15d01b9ca1c0 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/iflushstrategy.h @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #pragma once +#include "flush_strategy_result.h" #include "flushcontext.h" #include "iflushhandler.h" @@ -35,9 +36,9 @@ class IFlushStrategy { * @param active_flushes Statistics of active (ongoing) flushes per flush handler. * @return A prioritized list of targets to flush. */ - virtual FlushContext::List getFlushTargets(const FlushContext::List& targetList, - const flushengine::TlsStatsMap& tlsStatsMap, - const flushengine::ActiveFlushStats& active_flushes) const = 0; + virtual flushengine::FlushStrategyResult + getFlushTargets(const FlushContext::List& targetList, const flushengine::TlsStatsMap& tlsStatsMap, + const flushengine::ActiveFlushStats& active_flushes) const = 0; virtual std::string name() const = 0; void set_id(uint32_t id) noexcept { _id = id; } uint32_t get_id() const noexcept { return _id; } diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/prepare_restart_flush_strategy.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/prepare_restart_flush_strategy.cpp index 962721db04c6..b5b5346459c0 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/prepare_restart_flush_strategy.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/prepare_restart_flush_strategy.cpp @@ -12,6 +12,8 @@ #include LOG_SETUP(".proton.flushengine.prepare_restart_flush_strategy"); +using proton::flushengine::FlushStrategyResult; + namespace proton { using search::SerialNum; @@ -21,6 +23,12 @@ using Config = PrepareRestartFlushStrategy::Config; using FlushContextsMap = std::map; using FlushTargetCandidatesList = std::vector; +namespace { + +const std::string strategy_name("prepare_restart"); + +} + PrepareRestartFlushStrategy::Config::Config(double tlsReplayByteCost_, double tlsReplayOperationCost_, double flushTargetWriteCost_, double flush_target_read_cost_) : tlsReplayByteCost(tlsReplayByteCost_), @@ -132,15 +140,16 @@ FlushContextsMap findBestTargetsToFlushPerHandler(const FlushContextsMap& flushC } // namespace -FlushContext::List PrepareRestartFlushStrategy::getFlushTargets(const FlushContext::List& targetList, - const flushengine::TlsStatsMap& tlsStatsMap, - const flushengine::ActiveFlushStats&) const { - return flatten( - findBestTargetsToFlushPerHandler(groupByFlushHandler(removeGCFlushTargets(targetList)), _cfg, tlsStatsMap)); +FlushStrategyResult PrepareRestartFlushStrategy::getFlushTargets(const FlushContext::List& targetList, + const flushengine::TlsStatsMap& tlsStatsMap, + const flushengine::ActiveFlushStats&) const { + return FlushStrategyResult(flatten(findBestTargetsToFlushPerHandler( + groupByFlushHandler(removeGCFlushTargets(targetList)), _cfg, tlsStatsMap)), + strategy_name, _id, true, strategy_name); } std::string PrepareRestartFlushStrategy::name() const { - return "prepare_restart"; + return strategy_name; } } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/prepare_restart_flush_strategy.h b/searchcore/src/vespa/searchcore/proton/flushengine/prepare_restart_flush_strategy.h index 2e64fdd1f5a7..ade5bbc00c49 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/prepare_restart_flush_strategy.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/prepare_restart_flush_strategy.h @@ -34,9 +34,9 @@ class PrepareRestartFlushStrategy : public IFlushStrategy { public: PrepareRestartFlushStrategy(const Config& cfg); - FlushContext::List getFlushTargets(const FlushContext::List& targetList, - const flushengine::TlsStatsMap& tlsStatsMap, - const flushengine::ActiveFlushStats&) const override; + flushengine::FlushStrategyResult getFlushTargets(const FlushContext::List& targetList, + const flushengine::TlsStatsMap& tlsStatsMap, + const flushengine::ActiveFlushStats&) const override; std::string name() const override; }; diff --git a/searchcore/src/vespa/searchcore/proton/server/memoryflush.cpp b/searchcore/src/vespa/searchcore/proton/server/memoryflush.cpp index adf5d3e1e87c..4963e22b659b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/memoryflush.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/memoryflush.cpp @@ -14,6 +14,7 @@ #include LOG_SETUP(".proton.server.memoryflush"); +using proton::flushengine::FlushStrategyResult; using proton::flushengine::TlsStats; using search::SerialNum; using searchcorespi::IFlushTarget; @@ -41,6 +42,9 @@ uint64_t estimateNeededTlsSizeForFlushTarget(const TlsStats& tlsStats, SerialNum return bytesPerEntry * (tlsStats.getLastSerial() - flushedSerialNum); } +const std::string strategy_name("memory"); +const std::string no_info("none"); + } // namespace MemoryFlush::Config::Config() @@ -117,9 +121,9 @@ size_t computeGain(const IFlushTarget::DiskGain& gain) { } // namespace -FlushContext::List MemoryFlush::getFlushTargets(const FlushContext::List& targetList, - const flushengine::TlsStatsMap& tlsStatsMap, - const flushengine::ActiveFlushStats& active_flushes) const { +FlushStrategyResult MemoryFlush::getFlushTargets(const FlushContext::List& targetList, + const flushengine::TlsStatsMap& tlsStatsMap, + const flushengine::ActiveFlushStats& active_flushes) const { OrderType order(DEFAULT); uint64_t totalMemory(0); IFlushTarget::DiskGain totalDisk; @@ -155,7 +159,8 @@ FlushContext::List MemoryFlush::getFlushTargets(const FlushContext::List& // that started before the last flush time of the flush target to evaluate. // Instead we should wait for the active (ongoing) flush to be finished before doing another evaluation. if ((!oldest_start_time.has_value() || lastFlushTime < oldest_start_time.value()) && - target.getType() != IFlushTarget::Type::GC) { + target.getType() != IFlushTarget::Type::GC) + { if (visitedHandlers.insert(&handler).second) { totalTlsSize += tlsStats.getNumBytes(); if ((totalTlsSize > config.maxGlobalTlsSize) && (order < TLSSIZE)) { @@ -167,8 +172,8 @@ FlushContext::List MemoryFlush::getFlushTargets(const FlushContext::List& order = MEMORY; } else if ((dgain.gain() > config.diskBloatFactor * computeGain(dgain)) && (order < DISKBLOAT)) { order = DISKBLOAT; - } else if ((timeDiff >= config.maxTimeGain) && (order < MAXAGE) && - target.getType() != IFlushTarget::Type::GC) { + } else if ((timeDiff >= config.maxTimeGain) && (order < MAXAGE) && target.getType() != IFlushTarget::Type::GC) + { order = MAXAGE; } LOG(debug, @@ -195,7 +200,7 @@ FlushContext::List MemoryFlush::getFlushTargets(const FlushContext::List& // No desired order and no urgent needs; no flush required at this moment. if (order == DEFAULT && !fv.empty() && !fv[0]->getTarget()->needUrgentFlush()) { LOG(debug, "getFlushTargets(): empty list"); - return {}; + return FlushStrategyResult({}, strategy_name, _id, false, no_info); } if (LOG_WOULD_LOG(debug)) { vespalib::asciistream oss; @@ -207,7 +212,7 @@ FlushContext::List MemoryFlush::getFlushTargets(const FlushContext::List& } LOG(debug, "getFlushTargets(): %zu sorted targets: [%s]", fv.size(), oss.str().c_str()); } - return fv; + return FlushStrategyResult(std::move(fv), strategy_name, _id, false, getOrderName(order)); } std::string MemoryFlush::name() const { diff --git a/searchcore/src/vespa/searchcore/proton/server/memoryflush.h b/searchcore/src/vespa/searchcore/proton/server/memoryflush.h index 6294bcaa1060..f7a2c56ca4ed 100644 --- a/searchcore/src/vespa/searchcore/proton/server/memoryflush.h +++ b/searchcore/src/vespa/searchcore/proton/server/memoryflush.h @@ -68,9 +68,9 @@ class MemoryFlush : public IFlushStrategy { MemoryFlush(const Config& config, vespalib::system_time startTime); ~MemoryFlush(); - FlushContext::List getFlushTargets(const FlushContext::List& targetList, - const flushengine::TlsStatsMap& tlsStatsMap, - const flushengine::ActiveFlushStats& active_flushes) const override; + flushengine::FlushStrategyResult + getFlushTargets(const FlushContext::List& targetList, const flushengine::TlsStatsMap& tlsStatsMap, + const flushengine::ActiveFlushStats& active_flushes) const override; std::string name() const override; void setConfig(const Config& config); diff --git a/searchcore/src/vespa/searchcore/proton/server/simpleflush.cpp b/searchcore/src/vespa/searchcore/proton/server/simpleflush.cpp index b055c361ce95..25c3a269939d 100644 --- a/searchcore/src/vespa/searchcore/proton/server/simpleflush.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/simpleflush.cpp @@ -8,15 +8,24 @@ #include LOG_SETUP(".proton.server.simpleflush"); +using proton::flushengine::FlushStrategyResult; + namespace proton { +namespace { + +const std::string strategy_name("simple"); + +} + SimpleFlush::SimpleFlush() = default; -FlushContext::List SimpleFlush::getFlushTargets(const FlushContext::List& targetList, const flushengine::TlsStatsMap&, - const flushengine::ActiveFlushStats&) const { +FlushStrategyResult SimpleFlush::getFlushTargets(const FlushContext::List& targetList, + const flushengine::TlsStatsMap&, + const flushengine::ActiveFlushStats&) const { FlushContext::List fv(targetList); std::sort(fv.begin(), fv.end(), CompareTarget()); - return fv; + return FlushStrategyResult(std::move(fv), strategy_name, _id, false, strategy_name); } std::string SimpleFlush::name() const { diff --git a/searchcore/src/vespa/searchcore/proton/server/simpleflush.h b/searchcore/src/vespa/searchcore/proton/server/simpleflush.h index 4535e3539234..a4e69d5057da 100644 --- a/searchcore/src/vespa/searchcore/proton/server/simpleflush.h +++ b/searchcore/src/vespa/searchcore/proton/server/simpleflush.h @@ -22,8 +22,9 @@ class SimpleFlush : public IFlushStrategy { SimpleFlush(); // Implements IFlushStrategy - FlushContext::List getFlushTargets(const FlushContext::List& targetList, const flushengine::TlsStatsMap&, - const flushengine::ActiveFlushStats&) const override; + flushengine::FlushStrategyResult getFlushTargets(const FlushContext::List& targetList, + const flushengine::TlsStatsMap&, + const flushengine::ActiveFlushStats&) const override; std::string name() const override; }; From f7914a4d0a32ed314333a95f57a61f1c1acb0253 Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Wed, 20 May 2026 15:12:27 +0200 Subject: [PATCH 2/3] Use const auto-refs. Use empty string for no info. --- .../src/vespa/searchcore/proton/flushengine/flushengine.cpp | 4 ++-- searchcore/src/vespa/searchcore/proton/server/memoryflush.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp index ee953b700dbb..1dcb03715331 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp @@ -417,8 +417,8 @@ FlushContext::SP FlushEngine::initNextFlush(const FlushStrategyResult& flush_str FlushContext::SP ctx; for (const FlushContext::SP& it : lst) { if (LOG_WOULD_LOG(event)) { - auto& strategy_info = flush_strategy_result.strategy_info(); - auto& strategy_name = flush_strategy_result.strategy_name(); + const auto& strategy_info = flush_strategy_result.strategy_info(); + const auto& strategy_name = flush_strategy_result.strategy_name(); EventLogger::flushInit(it->getName(), strategy_name, strategy_info); } if (it->initFlush(get_flush_token(*it))) { diff --git a/searchcore/src/vespa/searchcore/proton/server/memoryflush.cpp b/searchcore/src/vespa/searchcore/proton/server/memoryflush.cpp index 4963e22b659b..6fcb6abb4566 100644 --- a/searchcore/src/vespa/searchcore/proton/server/memoryflush.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/memoryflush.cpp @@ -43,7 +43,7 @@ uint64_t estimateNeededTlsSizeForFlushTarget(const TlsStats& tlsStats, SerialNum } const std::string strategy_name("memory"); -const std::string no_info("none"); +const std::string no_info(""); } // namespace From 3260d0c98a009854ff83798871c2a2fd0abbfc8d Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Wed, 20 May 2026 15:39:22 +0200 Subject: [PATCH 3/3] Use const auto-refs. --- .../searchcore/proton/flushengine/flushengine.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp index 1dcb03715331..3cb11f461d88 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp +++ b/searchcore/src/vespa/searchcore/proton/flushengine/flushengine.cpp @@ -413,7 +413,7 @@ std::shared_ptr FlushEngine::get_flush_token(const FlushCon } FlushContext::SP FlushEngine::initNextFlush(const FlushStrategyResult& flush_strategy_result) { - auto& lst = flush_strategy_result.list(); + const auto& lst = flush_strategy_result.list(); FlushContext::SP ctx; for (const FlushContext::SP& it : lst) { if (LOG_WOULD_LOG(event)) { @@ -433,8 +433,8 @@ FlushContext::SP FlushEngine::initNextFlush(const FlushStrategyResult& flush_str } void FlushEngine::flushAll(const FlushStrategyResult& flush_strategy_result) { - auto& lst = flush_strategy_result.list(); - auto& strategy_info = flush_strategy_result.strategy_info(); + const auto& lst = flush_strategy_result.list(); + const auto& strategy_info = flush_strategy_result.strategy_info(); LOG(debug, "%ld targets to flush.", lst.size()); for (const FlushContext::SP& ctx : lst) { _flush_history->add_pending_flush(ctx->getHandler()->getName(), ctx->getTarget()->getName(), strategy_info, @@ -480,8 +480,8 @@ std::string FlushEngine::flushNextTarget(const std::string& name, const FlushStr name.c_str(), flush_strategy_result.list().size()); std::this_thread::sleep_for(100ms); } - auto strategy_id = flush_strategy_result.strategy_id(); - auto& strategy_info = flush_strategy_result.strategy_info(); + const auto strategy_id = flush_strategy_result.strategy_id(); + const auto& strategy_info = flush_strategy_result.strategy_info(); _executor.execute(std::make_unique(initFlush(*ctx, strategy_id, strategy_info), *this, ctx)); return ctx->getName(); }