diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-02-13 14:13:20 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2021-02-13 14:13:20 +0000 |
commit | c123db5020dbe5e05bbb83b12d71b77b69c49b2f (patch) | |
tree | 7be0274cf563430ff812fbe5985b4f891b5d3af4 | |
parent | e2b02d3b7978d831293d88bc3c75c6582ef9418b (diff) |
Move control of internal limits closer to where the limitation is.
12 files changed, 52 insertions, 90 deletions
diff --git a/searchcore/src/tests/proton/index/indexmanager_test.cpp b/searchcore/src/tests/proton/index/indexmanager_test.cpp index 065d95e82fa..7db7231c4e8 100644 --- a/searchcore/src/tests/proton/index/indexmanager_test.cpp +++ b/searchcore/src/tests/proton/index/indexmanager_test.cpp @@ -314,6 +314,15 @@ TEST_F(IndexManagerTest, require_that_memory_index_is_flushed) } } +TEST_F(IndexManagerTest, require_that_large_memory_footprint_triggers_urgenflush) { + using FlushStats = IndexMaintainer::FlushStats; + constexpr size_t G = 1024*1024*1024l; + // IndexMaintainer::FlushStats small_15G(15*G, 0, 1, 1); + EXPECT_FALSE(IndexFlushTarget(_index_manager->getMaintainer()).needUrgentFlush()); + EXPECT_FALSE(IndexFlushTarget(_index_manager->getMaintainer(), FlushStats(15*G)).needUrgentFlush()); + EXPECT_TRUE(IndexFlushTarget(_index_manager->getMaintainer(), FlushStats(17*G)).needUrgentFlush()); +} + TEST_F(IndexManagerTest, require_that_multiple_flushes_gives_multiple_indexes) { size_t flush_count = 10; diff --git a/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp b/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp index ff64d614169..23c0269f754 100644 --- a/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp +++ b/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp @@ -89,15 +89,6 @@ TEST("require that we cap configured limits based on available memory") { EXPECT_EQUAL(uint64_t(cfg.maxMemoryGain), LIMIT); } -TEST("require that we cap configured limits based on the absolute 16G limit") { - constexpr uint64_t LIMIT_16G = 16 * ONE_G; - constexpr uint64_t MEM_64G = 64 * ONE_G; - const HwInfo::Memory largeMemory(512 * ONE_G); - auto cfg = MemoryFlushConfigUpdater::convertConfig(getConfig(MEM_64G, MEM_64G, 30), largeMemory); - EXPECT_EQUAL(cfg.maxGlobalMemory, LIMIT_16G); - EXPECT_EQUAL(uint64_t(cfg.maxMemoryGain), LIMIT_16G); -} - TEST_F("require that strategy is updated with normal values if no limits are reached", Fixture) { f.updater.notifyDiskMemUsage(DiskMemUsageState()); diff --git a/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp b/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp index f2d45c996ae..fd880db5655 100644 --- a/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp @@ -111,15 +111,11 @@ MemoryFlushConfigUpdater::setNodeRetired(bool nodeRetired) } namespace { -// This is a hard limit to ensure that we stop before we reach some internal limits. -// This responsibility should not be here, but in each of the components - -constexpr uint64_t MAX_HARD_MEMORY_LIMIT_16G = 16ul * 1024ul * 1024ul * 1024ul; size_t getHardMemoryLimit(const HwInfo::Memory &memory) { - return std::min(memory.sizeBytes() / 4, MAX_HARD_MEMORY_LIMIT_16G); + return memory.sizeBytes() / 4; } } diff --git a/searchcorespi/src/vespa/searchcorespi/flush/flushstats.h b/searchcorespi/src/vespa/searchcorespi/flush/flushstats.h index 43636f34075..d3b09b00d1e 100644 --- a/searchcorespi/src/vespa/searchcorespi/flush/flushstats.h +++ b/searchcorespi/src/vespa/searchcorespi/flush/flushstats.h @@ -12,7 +12,7 @@ class FlushStats { private: vespalib::string _path; // path to data flushed - size_t _pathElementsToLog; + size_t _pathElementsToLog; public: FlushStats(); diff --git a/searchcorespi/src/vespa/searchcorespi/index/CMakeLists.txt b/searchcorespi/src/vespa/searchcorespi/index/CMakeLists.txt index 06e492e59e4..cd9f033f203 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/CMakeLists.txt +++ b/searchcorespi/src/vespa/searchcorespi/index/CMakeLists.txt @@ -20,7 +20,6 @@ vespa_add_library(searchcorespi_index OBJECT indexmanagerconfig.cpp indexreadutilities.cpp index_searchable_stats.cpp - memory_index_stats.cpp indexwriteutilities.cpp warmupindexcollection.cpp isearchableindexcollection.cpp diff --git a/searchcorespi/src/vespa/searchcorespi/index/index_manager_stats.cpp b/searchcorespi/src/vespa/searchcorespi/index/index_manager_stats.cpp index 3ff19497949..e1fd5a98e10 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/index_manager_stats.cpp +++ b/searchcorespi/src/vespa/searchcorespi/index/index_manager_stats.cpp @@ -3,6 +3,7 @@ #include "index_manager_stats.h" #include "iindexmanager.h" #include "indexsearchablevisitor.h" +#include "imemoryindex.h" namespace searchcorespi { @@ -14,15 +15,12 @@ public: std::vector<index::DiskIndexStats> _diskIndexes; std::vector<index::MemoryIndexStats> _memoryIndexes; - Visitor() - : _diskIndexes(), - _memoryIndexes() - { - } - virtual void visit(const index::IDiskIndex &index) override { + Visitor(); + ~Visitor(); + void visit(const index::IDiskIndex &index) override { _diskIndexes.emplace_back(index); } - virtual void visit(const index::IMemoryIndex &index) override { + void visit(const index::IMemoryIndex &index) override { _memoryIndexes.emplace_back(index); } @@ -32,6 +30,9 @@ public: } }; +Visitor::Visitor() = default; +Visitor::~Visitor() = default; + } IndexManagerStats::IndexManagerStats() @@ -52,8 +53,6 @@ IndexManagerStats::IndexManagerStats(const IIndexManager &indexManager) _memoryIndexes = std::move(visitor._memoryIndexes); } -IndexManagerStats::~IndexManagerStats() -{ -} +IndexManagerStats::~IndexManagerStats() = default; -} // namespace searchcorespi +} diff --git a/searchcorespi/src/vespa/searchcorespi/index/index_searchable_stats.h b/searchcorespi/src/vespa/searchcorespi/index/index_searchable_stats.h index 587a560f663..f37a7422e3d 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/index_searchable_stats.h +++ b/searchcorespi/src/vespa/searchcorespi/index/index_searchable_stats.h @@ -5,11 +5,9 @@ #include <vespa/searchlib/common/serialnum.h> #include <vespa/searchlib/util/searchable_stats.h> -namespace searchcorespi { +namespace searchcorespi { class IndexSearchable; } -class IndexSearchable; - -namespace index { +namespace searchcorespi::index { /** * Information about a searchable index usable by state explorer. @@ -18,7 +16,7 @@ class IndexSearchableStats { using SerialNum = search::SerialNum; using SearchableStats = search::SearchableStats; - SerialNum _serialNum; + SerialNum _serialNum; SearchableStats _searchableStats; public: IndexSearchableStats(); @@ -28,5 +26,4 @@ public: const SearchableStats &getSearchableStats() const { return _searchableStats; } }; -} // namespace searchcorespi::index -} // namespace searchcorespi +} diff --git a/searchcorespi/src/vespa/searchcorespi/index/indexflushtarget.cpp b/searchcorespi/src/vespa/searchcorespi/index/indexflushtarget.cpp index 1b07f2b2c4f..8ae6e18a28a 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/indexflushtarget.cpp +++ b/searchcorespi/src/vespa/searchcorespi/index/indexflushtarget.cpp @@ -7,10 +7,10 @@ LOG_SETUP(".searchcorespi.index.indexflushtarget"); namespace searchcorespi::index { -IndexFlushTarget::IndexFlushTarget(IndexMaintainer &indexMaintainer) +IndexFlushTarget::IndexFlushTarget(IndexMaintainer &indexMaintainer, IndexMaintainer::FlushStats flushStats) : IFlushTarget("memoryindex.flush", Type::FLUSH, Component::INDEX), _indexMaintainer(indexMaintainer), - _flushStats(indexMaintainer.getFlushStats()), + _flushStats(flushStats), _numFrozenMemoryIndexes(indexMaintainer.getNumFrozenMemoryIndexes()), _maxFrozenMemoryIndexes(indexMaintainer.getMaxFrozenMemoryIndexes()), _lastStats() @@ -18,6 +18,10 @@ IndexFlushTarget::IndexFlushTarget(IndexMaintainer &indexMaintainer) _lastStats.setPathElementsToLog(7); } +IndexFlushTarget::IndexFlushTarget(IndexMaintainer &indexMaintainer) + : IndexFlushTarget(indexMaintainer, indexMaintainer.getFlushStats()) +{} + IndexFlushTarget::~IndexFlushTarget() = default; IFlushTarget::MemoryGain @@ -32,18 +36,20 @@ IndexFlushTarget::getApproxDiskGain() const return DiskGain(0, 0); } - bool IndexFlushTarget::needUrgentFlush() const { - bool urgent = _numFrozenMemoryIndexes > _maxFrozenMemoryIndexes; + // Due to limitation of 16G address space of single datastore + // TODO: Even better if urgency was decided by memory index itself. + constexpr int64_t G = 1024*1024*1024l; + bool urgent = (_numFrozenMemoryIndexes > _maxFrozenMemoryIndexes) || + (getApproxMemoryGain().gain() > 16*G); SerialNum flushedSerial = _indexMaintainer.getFlushedSerialNum(); - LOG(debug, "Num frozen: %u Urgent: %d, flushedSerial=%" PRIu64, - _numFrozenMemoryIndexes, static_cast<int>(urgent), flushedSerial); + LOG(debug, "Num frozen: %u Memory gain: %ld Urgent: %d, flushedSerial=%" PRIu64, + _numFrozenMemoryIndexes, getApproxMemoryGain().gain(), static_cast<int>(urgent), flushedSerial); return urgent; } - IFlushTarget::Time IndexFlushTarget::getLastFlushTime() const { @@ -63,12 +69,10 @@ IndexFlushTarget::initFlush(SerialNum serialNum, std::shared_ptr<search::IFlushT return _indexMaintainer.initFlush(serialNum, &_lastStats); } - uint64_t IndexFlushTarget::getApproxBytesToWriteToDisk() const { - MemoryGain gain(_flushStats.memory_before_bytes, - _flushStats.memory_after_bytes); + MemoryGain gain(_flushStats.memory_before_bytes, _flushStats.memory_after_bytes); if (gain.getAfter() < gain.getBefore()) { return gain.getBefore() - gain.getAfter(); } else { diff --git a/searchcorespi/src/vespa/searchcorespi/index/indexflushtarget.h b/searchcorespi/src/vespa/searchcorespi/index/indexflushtarget.h index 5573a646d1b..0d4947bc69a 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/indexflushtarget.h +++ b/searchcorespi/src/vespa/searchcorespi/index/indexflushtarget.h @@ -11,15 +11,16 @@ namespace searchcorespi::index { **/ class IndexFlushTarget : public IFlushTarget { private: - IndexMaintainer &_indexMaintainer; - IndexMaintainer::FlushStats _flushStats; - uint32_t _numFrozenMemoryIndexes; - uint32_t _maxFrozenMemoryIndexes; - FlushStats _lastStats; + IndexMaintainer &_indexMaintainer; + const IndexMaintainer::FlushStats _flushStats; + uint32_t _numFrozenMemoryIndexes; + uint32_t _maxFrozenMemoryIndexes; + FlushStats _lastStats; public: - IndexFlushTarget(IndexMaintainer &indexMaintainer); - ~IndexFlushTarget(); + explicit IndexFlushTarget(IndexMaintainer &indexMaintainer); + IndexFlushTarget(IndexMaintainer &indexMaintainer, IndexMaintainer::FlushStats flushStats); + ~IndexFlushTarget() override; // Implements IFlushTarget MemoryGain getApproxMemoryGain() const override; diff --git a/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.h b/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.h index f002d6253c3..58ea36d685b 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.h +++ b/searchcorespi/src/vespa/searchcorespi/index/indexmaintainer.h @@ -285,8 +285,8 @@ public: void removeOldDiskIndexes(); struct FlushStats { - FlushStats() : - memory_before_bytes(0), + explicit FlushStats(uint64_t memory_before=0) : + memory_before_bytes(memory_before), memory_after_bytes(0), disk_write_bytes(0), cpu_time_required(0) diff --git a/searchcorespi/src/vespa/searchcorespi/index/memory_index_stats.cpp b/searchcorespi/src/vespa/searchcorespi/index/memory_index_stats.cpp deleted file mode 100644 index 6034c09a9be..00000000000 --- a/searchcorespi/src/vespa/searchcorespi/index/memory_index_stats.cpp +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "memory_index_stats.h" -#include "imemoryindex.h" - -namespace searchcorespi::index { - -MemoryIndexStats::MemoryIndexStats() - : IndexSearchableStats() -{ -} - -MemoryIndexStats::MemoryIndexStats(const IMemoryIndex &index) - : IndexSearchableStats(index) -{ -} - -MemoryIndexStats::~MemoryIndexStats() -{ -} - -} // namespace searchcorespi::index diff --git a/searchcorespi/src/vespa/searchcorespi/index/memory_index_stats.h b/searchcorespi/src/vespa/searchcorespi/index/memory_index_stats.h index a32b0ec32a0..62863bcb726 100644 --- a/searchcorespi/src/vespa/searchcorespi/index/memory_index_stats.h +++ b/searchcorespi/src/vespa/searchcorespi/index/memory_index_stats.h @@ -3,20 +3,8 @@ #include "index_searchable_stats.h" -namespace searchcorespi { -namespace index { +namespace searchcorespi::index { -struct IMemoryIndex; +using MemoryIndexStats = IndexSearchableStats; -/** - * Information about a memory index usable by state explorer. - */ -class MemoryIndexStats : public IndexSearchableStats { -public: - MemoryIndexStats(); - MemoryIndexStats(const IMemoryIndex &index); - ~MemoryIndexStats(); -}; - -} // namespace searchcorespi::index -} // namespace searchcorespi +} |