Merge pull request #32909 from vespa-engine/toregge/adjust-get-search…

…able-stats-api-to-limit-disk-io-stats-clearing-to-metrics-updater Adjust getSearchableStats() API to limit disk io stats clearing to
vespa-engine · Nov 20, 2024 · b621426 · b621426
2 parents e78502d + b3405f9
commit b621426
Show file tree

Hide file tree

Showing 27 changed files with 49 additions and 43 deletions.
diff --git a/searchcore/src/vespa/searchcore/proton/index/diskindexwrapper.h b/searchcore/src/vespa/searchcore/proton/index/diskindexwrapper.h
@@ -31,8 +31,8 @@ class DiskIndexWrapper : public searchcorespi::index::IDiskIndex {
     createBlueprint(const IRequestContext & requestContext, const FieldSpecList &fields, const Node &term) override {
         return _index.createBlueprint(requestContext, fields, term);
     }
-    search::SearchableStats getSearchableStats() const override {
-        return _index.get_stats();
+    search::SearchableStats getSearchableStats(bool clear_disk_io_stats) const override {
+        return _index.get_stats(clear_disk_io_stats);
     }
 
     search::SerialNum getSerialNum() const override;

diff --git a/searchcore/src/vespa/searchcore/proton/index/indexmanager.h b/searchcore/src/vespa/searchcore/proton/index/indexmanager.h
@@ -120,8 +120,8 @@ class IndexManager : public searchcorespi::IIndexManager
         return _maintainer.getSearchable();
     }
 
-    search::SearchableStats getSearchableStats() const override {
-        return _maintainer.getSearchableStats();
+    search::SearchableStats getSearchableStats(bool clear_disk_io_stats) const override {
+        return _maintainer.getSearchableStats(clear_disk_io_stats);
     }
 
     searchcorespi::IFlushTarget::List getFlushTargets() override {

diff --git a/searchcore/src/vespa/searchcore/proton/index/memoryindexwrapper.h b/searchcore/src/vespa/searchcore/proton/index/memoryindexwrapper.h
@@ -49,7 +49,7 @@ class MemoryIndexWrapper : public searchcorespi::index::IMemoryIndex {
     {
         return _index.createBlueprint(requestContext, fields, term);
     }
-    search::SearchableStats getSearchableStats() const override {
+    search::SearchableStats getSearchableStats(bool) const override {
         return _index.get_stats();
     }
 

diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb_metrics_updater.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb_metrics_updater.cpp
@@ -303,7 +303,7 @@ DocumentDBMetricsUpdater::updateMetrics(const metrics::MetricLockGuard & guard,
 {
     TotalStats totalStats;
     ExecutorThreadingServiceStats threadingServiceStats = _writeService.getStats();
-    updateIndexMetrics(metrics, _subDBs.getReadySubDB()->getSearchableStats(), totalStats);
+    updateIndexMetrics(metrics, _subDBs.getReadySubDB()->getSearchableStats(true), totalStats);
     updateAttributeMetrics(metrics, _subDBs, totalStats);
     updateMatchingMetrics(guard, metrics, *_subDBs.getReadySubDB());
     updateDocumentsMetrics(metrics, _subDBs);

diff --git a/searchcore/src/vespa/searchcore/proton/server/idocumentsubdb.h b/searchcore/src/vespa/searchcore/proton/server/idocumentsubdb.h
@@ -122,7 +122,7 @@ class IDocumentSubDB
     virtual SerialNum getNewestFlushedSerial()  = 0;
     virtual void pruneRemovedFields(SerialNum serialNum) = 0;
     virtual void setIndexSchema(std::shared_ptr<const Schema> schema, SerialNum serialNum) = 0;
-    virtual search::SearchableStats getSearchableStats() const = 0;
+    virtual search::SearchableStats getSearchableStats(bool clear_disk_io_stats) const = 0;
     virtual std::shared_ptr<IDocumentRetriever> getDocumentRetriever() = 0;
 
     virtual matching::MatchingStats getMatcherStats(const std::string &rankProfile) const = 0;

diff --git a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.cpp
@@ -319,9 +319,9 @@ SearchableDocSubDB::getNumActiveDocs() const
 }
 
 search::SearchableStats
-SearchableDocSubDB::getSearchableStats() const
+SearchableDocSubDB::getSearchableStats(bool clear_disk_io_stats) const
 {
-    return _indexMgr ? _indexMgr->getSearchableStats() : search::SearchableStats();
+    return _indexMgr ? _indexMgr->getSearchableStats(clear_disk_io_stats) : search::SearchableStats();
 }
 
 std::shared_ptr<IDocumentRetriever>
@@ -375,7 +375,7 @@ SearchableDocSubDB::get_transient_resource_usage() const
     auto result = FastAccessDocSubDB::get_transient_resource_usage();
     // Transient disk usage is measured as the total disk usage of all current fusion indexes.
     // Transient memory usage is measured as the total memory usage of all memory indexes.
-    auto stats = getSearchableStats();
+    auto stats = getSearchableStats(false);
     result.merge({stats.fusion_size_on_disk(), stats.memoryUsage().allocatedBytes()});
     return result;
 }

diff --git a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h
@@ -134,7 +134,7 @@ SearchableDocSubDB : public FastAccessDocSubDB,
     SerialNum getNewestFlushedSerial() override;
     void setIndexSchema(std::shared_ptr<const Schema> schema, SerialNum serialNum) override;
     size_t getNumActiveDocs() const override;
-    search::SearchableStats getSearchableStats() const override ;
+    search::SearchableStats getSearchableStats(bool clear_disk_io_stats) const override ;
     std::shared_ptr<IDocumentRetriever> getDocumentRetriever() override;
     matching::MatchingStats getMatcherStats(const std::string &rankProfile) const override;
     void close() override;

diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp
@@ -544,7 +544,7 @@ StoreOnlyDocSubDB::setIndexSchema(std::shared_ptr<const Schema>, SerialNum)
 }
 
 search::SearchableStats
-StoreOnlyDocSubDB::getSearchableStats() const
+StoreOnlyDocSubDB::getSearchableStats(bool) const
 {
     return {};
 }

diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h
@@ -231,7 +231,7 @@ class StoreOnlyDocSubDB : public DocSubDB
 
     void pruneRemovedFields(SerialNum serialNum) override;
     void setIndexSchema(std::shared_ptr<const Schema> schema, SerialNum serialNum) override;
-    search::SearchableStats getSearchableStats() const override;
+    search::SearchableStats getSearchableStats(bool) const override;
     std::shared_ptr<IDocumentRetriever> getDocumentRetriever() override;
     matching::MatchingStats getMatcherStats(const std::string &rankProfile) const override;
     void close() override;

diff --git a/searchcore/src/vespa/searchcore/proton/test/dummy_document_sub_db.h b/searchcore/src/vespa/searchcore/proton/test/dummy_document_sub_db.h
@@ -74,7 +74,7 @@ struct DummyDocumentSubDb : public IDocumentSubDB
     SerialNum getNewestFlushedSerial() override { return 0; }
     void pruneRemovedFields(SerialNum) override { }
     void setIndexSchema(std::shared_ptr<const Schema>, SerialNum) override { }
-    search::SearchableStats getSearchableStats() const override {
+    search::SearchableStats getSearchableStats(bool) const override {
         return {};
     }
     std::shared_ptr<IDocumentRetriever> getDocumentRetriever() override {

diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_index_manager.h b/searchcore/src/vespa/searchcore/proton/test/mock_index_manager.h
@@ -19,7 +19,7 @@ struct MockIndexManager : public searchcorespi::IIndexManager
     searchcorespi::IndexSearchable::SP getSearchable() const override {
         return searchcorespi::IndexSearchable::SP();
     }
-    search::SearchableStats getSearchableStats() const override {
+    search::SearchableStats getSearchableStats(bool) const override {
         return search::SearchableStats();
     }
     searchcorespi::IFlushTarget::List getFlushTargets() override {

diff --git a/searchcore/src/vespa/searchcorespi/index/fakeindexsearchable.h b/searchcore/src/vespa/searchcorespi/index/fakeindexsearchable.h
@@ -28,7 +28,7 @@ class FakeIndexSearchable : public IndexSearchable {
         return _fake.createBlueprint(requestContext, field, term);
     }
 
-    search::SearchableStats getSearchableStats() const override {
+    search::SearchableStats getSearchableStats(bool) const override {
         return search::SearchableStats();
     }
 

diff --git a/searchcore/src/vespa/searchcorespi/index/iindexmanager.h b/searchcore/src/vespa/searchcorespi/index/iindexmanager.h
@@ -178,7 +178,7 @@ class IIndexManager {
      *
      * @return statistics gathered about underlying memory and disk indexes.
      */
-    virtual search::SearchableStats getSearchableStats() const = 0;
+    virtual search::SearchableStats getSearchableStats(bool clear_disk_io_stats) const = 0;
 
     /**
      * Returns the list of all flush targets contained in this index manager.

diff --git a/searchcore/src/vespa/searchcorespi/index/index_searchable_stats.cpp b/searchcore/src/vespa/searchcorespi/index/index_searchable_stats.cpp
@@ -14,7 +14,7 @@ IndexSearchableStats::IndexSearchableStats()
 
 IndexSearchableStats::IndexSearchableStats(const IndexSearchable &index)
     : _serialNum(index.getSerialNum()),
-      _searchableStats(index.getSearchableStats())
+      _searchableStats(index.getSearchableStats(false))
 {
 }
 

diff --git a/searchcore/src/vespa/searchcorespi/index/indexcollection.cpp b/searchcore/src/vespa/searchcorespi/index/indexcollection.cpp
@@ -109,11 +109,11 @@ IndexCollection::getSourceId(uint32_t i) const
 }
 
 search::SearchableStats
-IndexCollection::getSearchableStats() const
+IndexCollection::getSearchableStats(bool clear_disk_io_stats) const
 {
     search::SearchableStats stats;
     for (size_t i = 0; i < _sources.size(); ++i) {
-        stats.merge(_sources[i].source_wrapper->getSearchableStats());
+        stats.merge(_sources[i].source_wrapper->getSearchableStats(clear_disk_io_stats));
     }
     return stats;
 }

diff --git a/searchcore/src/vespa/searchcorespi/index/indexcollection.h b/searchcore/src/vespa/searchcorespi/index/indexcollection.h
@@ -50,7 +50,7 @@ class IndexCollection : public ISearchableIndexCollection
     createBlueprint(const IRequestContext & requestContext, const FieldSpec &field, const Node &term) override;
     std::unique_ptr<search::queryeval::Blueprint>
     createBlueprint(const IRequestContext & requestContext, const FieldSpecList &fields, const Node &term) override;
-    search::SearchableStats getSearchableStats() const  override;
+    search::SearchableStats getSearchableStats(bool clear_disk_io_stats) const  override;
     search::SerialNum getSerialNum() const override;
     void accept(IndexSearchableVisitor &visitor) const override;
 

diff --git a/searchcore/src/vespa/searchcorespi/index/indexmaintainer.cpp b/searchcore/src/vespa/searchcorespi/index/indexmaintainer.cpp
@@ -135,7 +135,7 @@ class DiskIndexWithDestructorCallback : public IDiskIndex {
     {
         return _index->createBlueprint(requestContext, fields, term);
     }
-    search::SearchableStats getSearchableStats() const override;
+    search::SearchableStats getSearchableStats(bool clear_disk_io_stats) const override;
     search::SerialNum getSerialNum() const override {
         return _index->getSerialNum();
     }
@@ -161,9 +161,9 @@ class DiskIndexWithDestructorCallback : public IDiskIndex {
 DiskIndexWithDestructorCallback::~DiskIndexWithDestructorCallback() = default;
 
 search::SearchableStats
-DiskIndexWithDestructorCallback::getSearchableStats() const
+DiskIndexWithDestructorCallback::getSearchableStats(bool clear_disk_io_stats) const
 {
-    auto stats = _index->getSearchableStats();
+    auto stats = _index->getSearchableStats(clear_disk_io_stats);
     uint64_t transient_size = _disk_indexes.get_transient_size(_layout, _index_disk_dir);
     stats.fusion_size_on_disk(transient_size);
     return stats;
@@ -315,7 +315,7 @@ IndexMaintainer::loadDiskIndex(const string &indexDir)
     }
     vespalib::Timer timer;
     auto index = _operations.loadDiskIndex(indexDir);
-    auto stats = index->getSearchableStats();
+    auto stats = index->getSearchableStats(false);
     _disk_indexes->setActive(indexDir, stats.sizeOnDisk());
     auto retval = std::make_shared<DiskIndexWithDestructorCallback>(
             std::move(index),
@@ -338,7 +338,7 @@ IndexMaintainer::reloadDiskIndex(const IDiskIndex &oldIndex)
     vespalib::Timer timer;
     const IDiskIndex &wrappedDiskIndex = (dynamic_cast<const DiskIndexWithDestructorCallback &>(oldIndex)).getWrapped();
     auto index = _operations.reloadDiskIndex(wrappedDiskIndex);
-    auto stats = index->getSearchableStats();
+    auto stats = index->getSearchableStats(false);
     _disk_indexes->setActive(indexDir, stats.sizeOnDisk());
     auto retval = std::make_shared<DiskIndexWithDestructorCallback>(
             std::move(index),
@@ -1184,7 +1184,7 @@ IndexMaintainer::getFusionStats() const
         source_list = _source_list;
         stats.maxFlushed = _maxFlushed;
     }
-    stats.diskUsage = source_list->getSearchableStats().sizeOnDisk();
+    stats.diskUsage = source_list->getSearchableStats(false).sizeOnDisk();
     {
         LockGuard guard(_fusion_lock);
         stats.numUnfused = _fusion_spec.flush_ids.size() + ((_fusion_spec.last_fusion_id != 0) ? 1 : 0);

diff --git a/searchcore/src/vespa/searchcorespi/index/indexmaintainer.h b/searchcore/src/vespa/searchcorespi/index/indexmaintainer.h
@@ -361,9 +361,9 @@ class IndexMaintainer : public IIndexManager,
         return _source_list;
     }
 
-    search::SearchableStats getSearchableStats() const override {
+    search::SearchableStats getSearchableStats(bool clear_disk_io_stats) const override {
         LockGuard lock(_new_search_lock);
-        return _source_list->getSearchableStats();
+        return _source_list->getSearchableStats(clear_disk_io_stats);
     }
 
     IFlushTarget::List getFlushTargets() override;

diff --git a/searchcore/src/vespa/searchcorespi/index/indexsearchable.h b/searchcore/src/vespa/searchcorespi/index/indexsearchable.h
@@ -40,7 +40,7 @@ class IndexSearchable : public search::queryeval::Searchable,
     /**
      * Returns the searchable stats for this index searchable.
      */
-    virtual search::SearchableStats getSearchableStats() const = 0;
+    virtual search::SearchableStats getSearchableStats(bool clear_disk_io_stats) const = 0;
 
     /**
      * Returns the serial number for this index searchable.

diff --git a/searchcore/src/vespa/searchcorespi/index/warmupindexcollection.cpp b/searchcore/src/vespa/searchcorespi/index/warmupindexcollection.cpp
@@ -226,9 +226,9 @@ WarmupIndexCollection::createBlueprint(const IRequestContext & requestContext,
 }
 
 search::SearchableStats
-WarmupIndexCollection::getSearchableStats() const
+WarmupIndexCollection::getSearchableStats(bool clear_disk_io_stats) const
 {
-    return _prev->getSearchableStats();
+    return _prev->getSearchableStats(clear_disk_io_stats);
 }
 
 

diff --git a/searchcore/src/vespa/searchcorespi/index/warmupindexcollection.h b/searchcore/src/vespa/searchcorespi/index/warmupindexcollection.h
@@ -48,7 +48,7 @@ class WarmupIndexCollection : public ISearchableIndexCollection,
     createBlueprint(const IRequestContext & requestContext, const FieldSpec &field, const Node &term) override;
     std::unique_ptr<search::queryeval::Blueprint>
     createBlueprint(const IRequestContext & requestContext, const FieldSpecList &fields, const Node &term) override;
-    search::SearchableStats getSearchableStats() const override;
+    search::SearchableStats getSearchableStats(bool clear_disk_io_stats) const override;
     search::SerialNum getSerialNum() const override;
     void accept(IndexSearchableVisitor &visitor) const override;
 

diff --git a/searchlib/src/tests/diskindex/diskindex/diskindex_test.cpp b/searchlib/src/tests/diskindex/diskindex/diskindex_test.cpp
@@ -448,7 +448,7 @@ DiskIndexTest::build_index(const IOSettings& io_settings, const EmptySettings& e
 void
 DiskIndexTest::require_that_get_stats_works()
 {
-    auto stats = getIndex().get_stats();
+    auto stats = getIndex().get_stats(false);
     auto& schema = getIndex().getSchema();
     EXPECT_LT(0, stats.sizeOnDisk());
     auto field_stats = stats.get_field_stats();

diff --git a/searchlib/src/vespa/searchlib/diskindex/diskindex.cpp b/searchlib/src/vespa/searchlib/diskindex/diskindex.cpp
@@ -399,13 +399,13 @@ DiskIndex::get_field_length_info(const std::string& field_name) const
 }
 
 SearchableStats
-DiskIndex::get_stats() const
+DiskIndex::get_stats(bool clear_disk_io_stats) const
 {
     SearchableStats stats;
     uint64_t size_on_disk = _nonfield_size_on_disk;
     uint32_t field_id = 0;
     for (auto& field_index : _field_indexes) {
-        auto field_stats = field_index.get_stats();
+        auto field_stats = field_index.get_stats(clear_disk_io_stats);
         size_on_disk += field_stats.size_on_disk();
         stats.add_field_stats(_schema.getIndexField(field_id).getName(), field_stats);
         ++field_id;

diff --git a/searchlib/src/vespa/searchlib/diskindex/diskindex.h b/searchlib/src/vespa/searchlib/diskindex/diskindex.h
@@ -110,7 +110,7 @@ class DiskIndex : public queryeval::Searchable {
     /**
      * Get stats for this index.
      */
-    SearchableStats get_stats() const;
+    SearchableStats get_stats(bool clear_disk_io_stats) const;
     const index::Schema &getSchema() const { return _schema; }
     const std::string &getIndexDir() const { return _indexDir; }
 

diff --git a/searchlib/src/vespa/searchlib/diskindex/field_index.cpp b/searchlib/src/vespa/searchlib/diskindex/field_index.cpp
@@ -279,9 +279,9 @@ FieldIndex::get_field_length_info() const
 }
 
 FieldIndexStats
-FieldIndex::get_stats() const
+FieldIndex::get_stats(bool clear_disk_io_stats) const
 {
-    auto cache_disk_io_stats = _cache_disk_io_stats->read_and_clear();
+    auto cache_disk_io_stats = _cache_disk_io_stats->read_and_maybe_clear(clear_disk_io_stats);
     return FieldIndexStats().size_on_disk(_size_on_disk).cache_disk_io_stats(cache_disk_io_stats);
 }
 

diff --git a/searchlib/src/vespa/searchlib/diskindex/field_index.h b/searchlib/src/vespa/searchlib/diskindex/field_index.h
@@ -42,9 +42,9 @@ class FieldIndex : public IPostingListCache::IPostingListFileBacking {
             _stats.add_cached_read_operation(bytes);
         }
 
-        CacheDiskIoStats read_and_clear() {
+        CacheDiskIoStats read_and_maybe_clear(bool clear_disk_io_stats) {
             std::lock_guard guard(_mutex);
-            return _stats.read_and_clear();
+            return _stats.read_and_maybe_clear(clear_disk_io_stats);
         }
     };
 
@@ -86,7 +86,7 @@ class FieldIndex : public IPostingListCache::IPostingListFileBacking {
     index::FieldLengthInfo get_field_length_info() const;
 
     index::DictionaryFileRandRead* get_dictionary() noexcept { return _dict.get(); }
-    FieldIndexStats get_stats() const;
+    FieldIndexStats get_stats(bool clear_disk_io_stats) const;
     uint32_t get_field_id() const noexcept { return _field_id; }
     bool is_posting_list_cache_enabled() const noexcept { return _posting_list_cache_enabled; }
 };

diff --git a/searchlib/src/vespa/searchlib/util/cache_disk_io_stats.h b/searchlib/src/vespa/searchlib/util/cache_disk_io_stats.h
@@ -32,7 +32,13 @@ class CacheDiskIoStats {
         return _read == rhs.read() &&
                _cached_read == rhs.cached_read();
     }
-    CacheDiskIoStats read_and_clear() noexcept { auto result = *this; clear(); return result; }
+    CacheDiskIoStats read_and_maybe_clear(bool clear_disk_io_stats) noexcept {
+        auto result = *this;
+        if (clear_disk_io_stats) {
+            clear();
+        }
+        return result;
+    }
     void clear() noexcept {
         _read.clear();
         _cached_read.clear();