Merge pull request #6662 from vespa-engine/geirst/migrate-legacy-document-store-cache-metrics

Migrate legacy document store cache metrics to new naming scheme.

4 files changed, 74 insertions, 31 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp
index 335b6855d48..661b5e34f34 100644
--- a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp
+++ b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.cpp
@@ -51,12 +51,25 @@ DocumentDBTaggedMetrics::SubDBMetrics::LidSpaceMetrics::LidSpaceMetrics(MetricSe
 
 DocumentDBTaggedMetrics::SubDBMetrics::LidSpaceMetrics::~LidSpaceMetrics() { }
 
+DocumentDBTaggedMetrics::SubDBMetrics::DocumentStoreMetrics::CacheMetrics::CacheMetrics(MetricSet *parent)
+    : MetricSet("cache", "", "Document store cache metrics", parent),
+      memoryUsage("memory_usage", "", "Memory usage of the cache (in bytes)", this),
+      elements("elements", "", "Number of elements in the cache", this),
+      hitRate("hit_rate", "", "Rate of hits in the cache compared to number of lookups", this),
+      lookups("lookups", "", "Number of lookups in the cache (hits + misses)", this),
+      invalidations("invalidations", "", "Number of invalidations (erased elements) in the cache. ", this)
+{
+}
+
+DocumentDBTaggedMetrics::SubDBMetrics::DocumentStoreMetrics::CacheMetrics::~CacheMetrics() {}
+
 DocumentDBTaggedMetrics::SubDBMetrics::DocumentStoreMetrics::DocumentStoreMetrics(MetricSet *parent)
-    : MetricSet("document_store", "", "document store metrics for this document sub DB", parent),
+    : MetricSet("document_store", "", "Document store metrics for this document sub DB", parent),
       diskUsage("disk_usage", "", "Disk space usage in bytes", this),
       diskBloat("disk_bloat", "", "Disk space bloat in bytes", this),
       maxBucketSpread("max_bucket_spread", "", "Max bucket spread in underlying files (sum(unique buckets in each chunk)/unique buckets in file)", this),
-      memoryUsage(this)
+      memoryUsage(this),
+      cache(this)
 { }
 
 DocumentDBTaggedMetrics::SubDBMetrics::DocumentStoreMetrics::~DocumentStoreMetrics() { }
diff --git a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h
index 7a0de216343..952fb0304f5 100644
--- a/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h
+++ b/searchcore/src/vespa/searchcore/proton/metrics/documentdb_tagged_metrics.h
@@ -49,10 +49,23 @@ struct DocumentDBTaggedMetrics : metrics::MetricSet
 
         struct DocumentStoreMetrics : metrics::MetricSet
         {
+            struct CacheMetrics : metrics::MetricSet
+            {
+                metrics::LongValueMetric memoryUsage;
+                metrics::LongValueMetric elements;
+                metrics::LongAverageMetric hitRate;
+                metrics::LongCountMetric lookups;
+                metrics::LongCountMetric invalidations;
+
+                CacheMetrics(metrics::MetricSet *parent);
+                ~CacheMetrics();
+            };
+
             metrics::LongValueMetric diskUsage;
             metrics::LongValueMetric diskBloat;
             metrics::DoubleValueMetric maxBucketSpread;
             MemoryUsageMetrics memoryUsage;
+            CacheMetrics cache;
 
             DocumentStoreMetrics(metrics::MetricSet *parent);
             ~DocumentStoreMetrics();
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
index 24d40e15677..da636068deb 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp
@@ -1166,6 +1166,26 @@ updateMatchingMetrics(DocumentDBMetricsCollection &metrics, const IDocumentSubDB
 }
 
 void
+updateDocumentStoreCacheHitRate(const CacheStats &current, const CacheStats &last,
+                                metrics::LongAverageMetric &cacheHitRate)
+{
+    if (current.lookups() < last.lookups() || current.hits < last.hits) {
+        LOG(warning, "Not adding document store cache hit rate metrics as values calculated "
+                "are corrupt. current.lookups=%" PRIu64 ", last.lookups=%" PRIu64 ", current.hits=%" PRIu64 ", last.hits=%" PRIu64 ".",
+            current.lookups(), last.lookups(), current.hits, last.hits);
+    } else {
+        if ((current.lookups() - last.lookups()) > 0xffffffffull
+            || (current.hits - last.hits) > 0xffffffffull)
+        {
+            LOG(warning, "Document store cache hit rate metrics to add are suspiciously high."
+                    " lookups diff=%" PRIu64 ", hits diff=%" PRIu64 ".",
+                current.lookups() - last.lookups(), current.hits - last.hits);
+        }
+        cacheHitRate.addTotalValueWithCount(current.hits - last.hits, current.lookups() - last.lookups());
+    }
+}
+
+void
 updateDocstoreMetrics(LegacyDocumentDBMetrics::DocstoreMetrics &metrics,
                       const DocumentSubDBCollection &sub_dbs,
                       CacheStats &lastCacheStats)
@@ -1180,26 +1200,8 @@ updateDocstoreMetrics(LegacyDocumentDBMetrics::DocstoreMetrics &metrics,
         }
     }
     metrics.memoryUsage.set(memoryUsage);
-    size_t lookups = cache_stats.hits + cache_stats.misses;
-    metrics.cacheLookups.set(lookups);
-    size_t last_count = lastCacheStats.hits + lastCacheStats.misses;
-        // For the above code to add sane values to the metric, the following
-        // must be true
-    if (lookups < last_count || cache_stats.hits < lastCacheStats.hits) {
-        LOG(warning, "Not adding document db metrics as values calculated "
-                     "are corrupt. %" PRIu64 ", %" PRIu64 ", %" PRIu64 ", %" PRIu64 ".",
-            lookups, last_count, cache_stats.hits, lastCacheStats.hits);
-    } else {
-        if (lookups - last_count > 0xffffffffull
-            || cache_stats.hits - lastCacheStats.hits > 0xffffffffull)
-        {
-            LOG(warning, "Document db metrics to add are suspiciously high."
-                         " %" PRIu64 ", %" PRIu64 ".",
-                lookups - last_count, cache_stats.hits - lastCacheStats.hits);
-        }
-        metrics.cacheHitRate.addTotalValueWithCount(
-                cache_stats.hits - lastCacheStats.hits, lookups - last_count);
-    }
+    metrics.cacheLookups.set(cache_stats.lookups());
+    updateDocumentStoreCacheHitRate(cache_stats, lastCacheStats, metrics.cacheHitRate);
     metrics.hits = cache_stats.hits;
     metrics.cacheElements.set(cache_stats.elements);
     metrics.cacheMemoryUsed.set(cache_stats.memory_used);
@@ -1207,9 +1209,9 @@ updateDocstoreMetrics(LegacyDocumentDBMetrics::DocstoreMetrics &metrics,
 }
 
 void
-updateDocumentStoreMetrics(DocumentDBTaggedMetrics::SubDBMetrics::
-                           DocumentStoreMetrics &metrics,
-                           IDocumentSubDB *subDb)
+updateDocumentStoreMetrics(DocumentDBTaggedMetrics::SubDBMetrics::DocumentStoreMetrics &metrics,
+                           IDocumentSubDB *subDb,
+                           CacheStats &lastCacheStats)
 {
     const ISummaryManager::SP &summaryMgr = subDb->getSummaryManager();
     search::IDocumentStore &backingStore = summaryMgr->getBackingStore();
@@ -1218,6 +1220,14 @@ updateDocumentStoreMetrics(DocumentDBTaggedMetrics::SubDBMetrics::
     metrics.diskBloat.set(storageStats.diskBloat());
     metrics.maxBucketSpread.set(storageStats.maxBucketSpread());
     metrics.memoryUsage.update(backingStore.getMemoryUsage());
+
+    search::CacheStats cacheStats = backingStore.getCacheStats();
+    metrics.cache.memoryUsage.set(cacheStats.memory_used);
+    metrics.cache.elements.set(cacheStats.elements);
+    updateDocumentStoreCacheHitRate(cacheStats, lastCacheStats, metrics.cache.hitRate);
+    metrics.cache.lookups.set(cacheStats.lookups());
+    metrics.cache.invalidations.set(cacheStats.invalidations);
+    lastCacheStats = cacheStats;
 }
 
 template <typename MetricSetType>
@@ -1257,7 +1267,7 @@ DocumentDB::updateLegacyMetrics(LegacyDocumentDBMetrics &metrics, const Executor
     metrics.summaryExecutor.update(threadingServiceStats.getSummaryExecutorStats());
     metrics.indexExecutor.update(threadingServiceStats.getIndexExecutorStats());
     metrics.sessionManager.update(_sessionManager->getGroupingStats());
-    updateDocstoreMetrics(metrics.docstore, _subDBs, _lastDocStoreCacheStats);
+    updateDocstoreMetrics(metrics.docstore, _subDBs, _lastDocStoreCacheStats.total);
     metrics.numDocs.set(getNumDocs());
 
     DocumentMetaStoreReadGuards dmss(_subDBs);
@@ -1295,9 +1305,9 @@ DocumentDB::updateMetrics(DocumentDBTaggedMetrics &metrics, const ExecutorThread
     _jobTrackers.updateMetrics(metrics.job);
 
     updateMetrics(metrics.attribute);
-    updateDocumentStoreMetrics(metrics.ready.documentStore, _subDBs.getReadySubDB());
-    updateDocumentStoreMetrics(metrics.removed.documentStore, _subDBs.getRemSubDB());
-    updateDocumentStoreMetrics(metrics.notReady.documentStore, _subDBs.getNotReadySubDB());
+    updateDocumentStoreMetrics(metrics.ready.documentStore, _subDBs.getReadySubDB(), _lastDocStoreCacheStats.readySubDb);
+    updateDocumentStoreMetrics(metrics.removed.documentStore, _subDBs.getRemSubDB(), _lastDocStoreCacheStats.removedSubDb);
+    updateDocumentStoreMetrics(metrics.notReady.documentStore, _subDBs.getNotReadySubDB(), _lastDocStoreCacheStats.notReadySubDb);
     DocumentMetaStoreReadGuards dmss(_subDBs);
     updateLidSpaceMetrics(metrics.ready.lidSpace, dmss.readydms->get());
     updateLidSpaceMetrics(metrics.notReady.lidSpace, dmss.notreadydms->get());
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.h b/searchcore/src/vespa/searchcore/proton/server/documentdb.h
index 1a64a4013de..5c04c4057ae 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdb.h
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.h
@@ -73,6 +73,13 @@ private:
         DocumentDBMetricsCollection &getMetrics() { return _metrics; }
     };
 
+    struct DocumentStoreCacheStats {
+        search::CacheStats total;
+        search::CacheStats readySubDb;
+        search::CacheStats notReadySubDb;
+        search::CacheStats removedSubDb;
+        DocumentStoreCacheStats() : total(), readySubDb(), notReadySubDb(), removedSubDb() {}
+    };
 
     using InitializeThreads = std::shared_ptr<vespalib::ThreadStackExecutorBase>;
     using IFlushTargetList = std::vector<std::shared_ptr<searchcorespi::IFlushTarget>>;
@@ -129,8 +136,8 @@ private:
     ILidSpaceCompactionHandler::Vector _lidSpaceCompactionHandlers;
     DocumentDBJobTrackers         _jobTrackers;
 
-    // Last updated cache statistics. Necessary due to metrics implementation is upside down.
-    search::CacheStats            _lastDocStoreCacheStats;
+    // Last updated document store cache statistics. Necessary due to metrics implementation is upside down.
+    DocumentStoreCacheStats       _lastDocStoreCacheStats;
     IBucketStateCalculator::SP    _calc;
 
     void registerReference();