Merge pull request #8443 from vespa-engine/vekterli/add-per-bucket-space-data-metrics-on-content-node

Expose data metrics per bucket space on content node

6 files changed, 159 insertions, 45 deletions

diff --git a/storage/src/tests/bucketdb/bucketmanagertest.cpp b/storage/src/tests/bucketdb/bucketmanagertest.cpp
index 829a080ad01..54b3bf4b8d0 100644
--- a/storage/src/tests/bucketdb/bucketmanagertest.cpp
+++ b/storage/src/tests/bucketdb/bucketmanagertest.cpp
@@ -67,6 +67,7 @@ public:
     CPPUNIT_TEST(testRemoveLastModifiedFailed);
     CPPUNIT_TEST(testSwallowNotifyBucketChangeReply);
     CPPUNIT_TEST(testMetricsGeneration);
+    CPPUNIT_TEST(metrics_are_tracked_per_bucket_space);
     CPPUNIT_TEST(testSplitReplyOrderedAfterBucketReply);
     CPPUNIT_TEST(testJoinReplyOrderedAfterBucketReply);
     CPPUNIT_TEST(testDeleteReplyOrderedAfterBucketReply);
@@ -136,6 +137,7 @@ public:
 
     void testSwallowNotifyBucketChangeReply();
     void testMetricsGeneration();
+    void metrics_are_tracked_per_bucket_space();
     void testSplitReplyOrderedAfterBucketReply();
     void testJoinReplyOrderedAfterBucketReply();
     void testDeleteReplyOrderedAfterBucketReply();
@@ -152,7 +154,6 @@ public:
     void testConflictSetOnlyClearedAfterAllBucketRequestsDone();
     void testRejectRequestWithMismatchingDistributionHash();
     void testDbNotIteratedWhenAllRequestsRejected();
-    void testReceivedDistributionHashIsNormalized();
 
 public:
     static constexpr uint32_t DIR_SPREAD = 3;
@@ -641,6 +642,51 @@ BucketManagerTest::testMetricsGeneration()
     CPPUNIT_ASSERT_EQUAL(int64_t(2), m.ready.getLast());
 }
 
+void BucketManagerTest::metrics_are_tracked_per_bucket_space() {
+    setupTestEnvironment();
+    _top->open();
+    auto& repo = _node->getComponentRegister().getBucketSpaceRepo();
+    {
+        bucketdb::StorageBucketInfo entry;
+        entry.disk = 0;
+        api::BucketInfo info(50, 100, 200);
+        info.setReady(true);
+        entry.setBucketInfo(info);
+        repo.get(document::FixedBucketSpaces::default_space()).bucketDatabase()
+                .insert(document::BucketId(16, 1234), entry, "foo");
+    }
+    {
+        bucketdb::StorageBucketInfo entry;
+        entry.disk = 0;
+        api::BucketInfo info(60, 150, 300);
+        info.setActive(true);
+        entry.setBucketInfo(info);
+        repo.get(document::FixedBucketSpaces::global_space()).bucketDatabase()
+                .insert(document::BucketId(16, 1234), entry, "foo");
+    }
+    _node->getDoneInitializeHandler().notifyDoneInitializing();
+    _top->doneInit();
+    vespalib::Monitor l;
+    _manager->updateMetrics(BucketManager::MetricLockGuard(l));
+
+    auto& spaces = _manager->_metrics->bucket_spaces;
+    auto default_m = spaces.find(document::FixedBucketSpaces::default_space());
+    CPPUNIT_ASSERT(default_m != spaces.end());
+    CPPUNIT_ASSERT_EQUAL(int64_t(1),   default_m->second->buckets_total.getLast());
+    CPPUNIT_ASSERT_EQUAL(int64_t(100), default_m->second->docs.getLast());
+    CPPUNIT_ASSERT_EQUAL(int64_t(200), default_m->second->bytes.getLast());
+    CPPUNIT_ASSERT_EQUAL(int64_t(0),   default_m->second->active_buckets.getLast());
+    CPPUNIT_ASSERT_EQUAL(int64_t(1),   default_m->second->ready_buckets.getLast());
+
+    auto global_m = spaces.find(document::FixedBucketSpaces::global_space());
+    CPPUNIT_ASSERT(global_m != spaces.end());
+    CPPUNIT_ASSERT_EQUAL(int64_t(1),   global_m->second->buckets_total.getLast());
+    CPPUNIT_ASSERT_EQUAL(int64_t(150), global_m->second->docs.getLast());
+    CPPUNIT_ASSERT_EQUAL(int64_t(300), global_m->second->bytes.getLast());
+    CPPUNIT_ASSERT_EQUAL(int64_t(1),   global_m->second->active_buckets.getLast());
+    CPPUNIT_ASSERT_EQUAL(int64_t(0),   global_m->second->ready_buckets.getLast());
+}
+
 void
 BucketManagerTest::insertSingleBucket(const document::BucketId& bucket,
                                       const api::BucketInfo& info)
diff --git a/storage/src/tests/common/metricstest.cpp b/storage/src/tests/common/metricstest.cpp
index 867d132031e..b0c82aa3166 100644
--- a/storage/src/tests/common/metricstest.cpp
+++ b/storage/src/tests/common/metricstest.cpp
@@ -82,44 +82,43 @@ MetricsTest::MetricsTest()
 }
 
 void MetricsTest::setUp() {
-    _config.reset(new vdstestlib::DirConfig(getStandardConfig(true, "metricstest")));
+    _config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, "metricstest"));
     assert(system(("rm -rf " + getRootFolder(*_config)).c_str()) == 0);
     try {
-        _node.reset(new TestServiceLayerApp(DiskCount(4), NodeIndex(0),
-                    _config->getConfigId()));
+        _node = std::make_unique<TestServiceLayerApp>(
+                DiskCount(4), NodeIndex(0), _config->getConfigId());
         _node->setupDummyPersistence();
         _clock = &_node->getClock();
         _clock->setAbsoluteTimeInSeconds(1000000);
-        _top.reset(new DummyStorageLink);
+        _top = std::make_unique<DummyStorageLink>();
     } catch (config::InvalidConfigException& e) {
         fprintf(stderr, "%s\n", e.what());
     }
-    _metricManager.reset(new metrics::MetricManager(
-            std::unique_ptr<metrics::MetricManager::Timer>(
-                new MetricClock(*_clock))));
+    _metricManager = std::make_unique<metrics::MetricManager>(
+            std::make_unique<MetricClock>(*_clock));
     _topSet.reset(new metrics::MetricSet("vds", {}, ""));
     {
         metrics::MetricLockGuard guard(_metricManager->getMetricLock());
         _metricManager->registerMetric(guard, *_topSet);
     }
 
-    _metricsConsumer.reset(new StatusMetricConsumer(
+    _metricsConsumer = std::make_unique<StatusMetricConsumer>(
             _node->getComponentRegister(),
             *_metricManager,
-            "status"));
+            "status");
 
     uint16_t diskCount = _node->getPartitions().size();
     documentapi::LoadTypeSet::SP loadTypes(_node->getLoadTypes());
 
-    _filestorMetrics.reset(new FileStorMetrics(_node->getLoadTypes()->getMetricLoadTypes()));
+    _filestorMetrics = std::make_shared<FileStorMetrics>(_node->getLoadTypes()->getMetricLoadTypes());
     _filestorMetrics->initDiskMetrics(diskCount, loadTypes->getMetricLoadTypes(), 1, 1);
     _topSet->registerMetric(*_filestorMetrics);
 
-    _bucketManagerMetrics.reset(new BucketManagerMetrics);
+    _bucketManagerMetrics = std::make_shared<BucketManagerMetrics>(_node->getComponentRegister().getBucketSpaceRepo());
     _bucketManagerMetrics->setDisks(diskCount);
     _topSet->registerMetric(*_bucketManagerMetrics);
 
-    _visitorMetrics.reset(new VisitorMetrics);
+    _visitorMetrics = std::make_shared<VisitorMetrics>();
     _visitorMetrics->initThreads(4, loadTypes->getMetricLoadTypes());
     _topSet->registerMetric(*_visitorMetrics);
     _metricManager->init(_config->getConfigId(), _node->getThreadPool());
@@ -127,12 +126,12 @@ void MetricsTest::setUp() {
 
 void MetricsTest::tearDown() {
     _metricManager->stop();
-    _metricsConsumer.reset(0);
-    _topSet.reset(0);
-    _metricManager.reset(0);
-    _top.reset(0);
-    _node.reset(0);
-    _config.reset(0);
+    _metricsConsumer.reset();
+    _topSet.reset();
+    _metricManager.reset();
+    _top.reset();
+    _node.reset();
+    _config.reset();
     _filestorMetrics.reset();
     _bucketManagerMetrics.reset();
     _visitorMetrics.reset();
diff --git a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp
index 551f6fc726d..41de215d877 100644
--- a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp
+++ b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp
@@ -42,10 +42,10 @@ BucketManager::BucketManager(const config::ConfigUri & configUri,
       _firstEqualClusterStateVersion(0),
       _lastClusterStateSeen(0),
       _lastUnifiedClusterState(""),
-      _metrics(new BucketManagerMetrics),
       _doneInitialized(false),
       _requestsCurrentlyProcessing(0),
-      _component(compReg, "bucketmanager")
+      _component(compReg, "bucketmanager"),
+      _metrics(std::make_shared<BucketManagerMetrics>(_component.getBucketSpaceRepo()))
 {
     _metrics->setDisks(_component.getDiskCount());
     _component.registerStatusPage(*this);
@@ -197,6 +197,19 @@ namespace {
 
             return StorBucketDatabase::CONTINUE;
         };
+
+        void add(const MetricsUpdater& rhs) {
+            assert(diskCount == rhs.diskCount);
+            for (uint16_t i = 0; i < diskCount; i++) {
+                auto& d = disk[i];
+                auto& s = rhs.disk[i];
+                d.buckets += s.buckets;
+                d.docs    += s.docs;
+                d.bytes   += s.bytes;
+                d.ready   += s.ready;
+                d.active  += s.active;
+            }
+        }
     };
 
 }   // End of anonymous namespace
@@ -216,18 +229,33 @@ BucketManager::updateMetrics(bool updateDocCount)
         updateDocCount ? "" : ", minusedbits only",
         _doneInitialized ? "" : ", server is not done initializing");
 
-    uint32_t diskCount = _component.getDiskCount();
+    uint16_t diskCount = _component.getDiskCount();
+    assert(diskCount >= 1);
     if (!updateDocCount || _doneInitialized) {
-        MetricsUpdater m(diskCount);
-        _component.getBucketSpaceRepo().forEachBucketChunked(
-                m, "BucketManager::updateMetrics");
+        MetricsUpdater total(diskCount);
+        for (auto& space : _component.getBucketSpaceRepo()) {
+            MetricsUpdater m(diskCount);
+            space.second->bucketDatabase().chunkedAll(m, "BucketManager::updateMetrics");
+            total.add(m);
+            if (updateDocCount) {
+                auto bm = _metrics->bucket_spaces.find(space.first);
+                assert(bm != _metrics->bucket_spaces.end());
+                // No system with multiple bucket spaces has more than 1 "disk"
+                // TODO remove disk concept entirely as it's a VDS relic
+                bm->second->buckets_total.set(m.disk[0].buckets);
+                bm->second->docs.set(m.disk[0].docs);
+                bm->second->bytes.set(m.disk[0].bytes);
+                bm->second->active_buckets.set(m.disk[0].active);
+                bm->second->ready_buckets.set(m.disk[0].ready);
+            }
+        }
         if (updateDocCount) {
             for (uint16_t i = 0; i< diskCount; i++) {
-                _metrics->disks[i]->buckets.addValue(m.disk[i].buckets);
-                _metrics->disks[i]->docs.addValue(m.disk[i].docs);
-                _metrics->disks[i]->bytes.addValue(m.disk[i].bytes);
-                _metrics->disks[i]->active.addValue(m.disk[i].active);
-                _metrics->disks[i]->ready.addValue(m.disk[i].ready);
+                _metrics->disks[i]->buckets.addValue(total.disk[i].buckets);
+                _metrics->disks[i]->docs.addValue(total.disk[i].docs);
+                _metrics->disks[i]->bytes.addValue(total.disk[i].bytes);
+                _metrics->disks[i]->active.addValue(total.disk[i].active);
+                _metrics->disks[i]->ready.addValue(total.disk[i].ready);
             }
         }
     }
diff --git a/storage/src/vespa/storage/bucketdb/bucketmanager.h b/storage/src/vespa/storage/bucketdb/bucketmanager.h
index 440e0d3b125..0503cb33d49 100644
--- a/storage/src/vespa/storage/bucketdb/bucketmanager.h
+++ b/storage/src/vespa/storage/bucketdb/bucketmanager.h
@@ -79,10 +79,10 @@ private:
      * The unified version of the last cluster state.
      */
     std::string _lastUnifiedClusterState;
-    std::shared_ptr<BucketManagerMetrics> _metrics;
     bool _doneInitialized;
     size_t _requestsCurrentlyProcessing;
     ServiceLayerComponent _component;
+    std::shared_ptr<BucketManagerMetrics> _metrics;
     framework::Thread::UP _thread;
 
     BucketManager(const BucketManager&);
diff --git a/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.cpp b/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.cpp
index a0f5f2f7ec9..b5efb5570bb 100644
--- a/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.cpp
+++ b/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.cpp
@@ -1,6 +1,8 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "bucketmanagermetrics.h"
+#include <vespa/document/bucket/fixed_bucket_spaces.h>
+#include <vespa/storage/common/content_bucket_space_repo.h>
 #include <vespa/vespalib/util/exceptions.h>
 #include <cassert>
 
@@ -23,11 +25,28 @@ DataStoredMetrics::DataStoredMetrics(const std::string& name, metrics::MetricSet
     ready.logOnlyIfSet();
 }
 
-DataStoredMetrics::~DataStoredMetrics() { }
+DataStoredMetrics::~DataStoredMetrics() = default;
 
-BucketManagerMetrics::BucketManagerMetrics()
+BucketSpaceMetrics::BucketSpaceMetrics(const vespalib::string& space_name, metrics::MetricSet* owner)
+        : metrics::MetricSet("bucket_space", {{"bucketSpace", space_name}}, "", owner),
+          buckets_total("buckets_total", {}, "Total number buckets present in the bucket space (ready + not ready)", this),
+          docs("docs", {}, "Documents stored in the bucket space", this),
+          bytes("bytes", {}, "Bytes stored across all documents in the bucket space", this),
+          active_buckets("active_buckets", {}, "Number of active buckets in the bucket space", this),
+          ready_buckets("ready_buckets", {}, "Number of ready buckets in the bucket space", this)
+{
+    docs.logOnlyIfSet();
+    bytes.logOnlyIfSet();
+    active_buckets.logOnlyIfSet();
+    ready_buckets.logOnlyIfSet();
+}
+
+BucketSpaceMetrics::~BucketSpaceMetrics() = default;
+
+BucketManagerMetrics::BucketManagerMetrics(const ContentBucketSpaceRepo& repo)
     : metrics::MetricSet("datastored", {}, ""),
       disks(),
+      bucket_spaces(),
       total("alldisks", {{"sum"}}, "Sum of data stored metrics for all disks", this),
       simpleBucketInfoRequestSize("simplebucketinforeqsize", {},
             "Amount of buckets returned in simple bucket info requests",
@@ -36,9 +55,14 @@ BucketManagerMetrics::BucketManagerMetrics()
             "Amount of distributors answered at once in full bucket info requests.", this),
       fullBucketInfoLatency("fullbucketinfolatency", {},
             "Amount of time spent to process a full bucket info request", this)
-{ }
+{
+    for (const auto& space : repo) {
+        bucket_spaces.emplace(space.first, std::make_unique<BucketSpaceMetrics>(
+                document::FixedBucketSpaces::to_string(space.first), this));
+    }
+}
 
-BucketManagerMetrics::~BucketManagerMetrics() { }
+BucketManagerMetrics::~BucketManagerMetrics() = default;
 
 void
 BucketManagerMetrics::setDisks(uint16_t numDisks) {
@@ -47,8 +71,7 @@ BucketManagerMetrics::setDisks(uint16_t numDisks) {
         throw IllegalStateException("Cannot initialize disks twice", VESPA_STRLOC);
     }
     for (uint16_t i = 0; i<numDisks; i++) {
-        disks.push_back(DataStoredMetrics::SP(
-                new DataStoredMetrics(make_string("disk%d", i), this)));
+        disks.push_back(std::make_shared<DataStoredMetrics>(make_string("disk%d", i), this));
         total.addMetricToSum(*disks.back());
     }
 }
diff --git a/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.h b/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.h
index 259075bf518..1e7b8960f14 100644
--- a/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.h
+++ b/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.h
@@ -2,12 +2,15 @@
 
 #pragma once
 
+#include <vespa/document/bucket/bucketspace.h>
 #include <vespa/metrics/metrics.h>
 
+#include <unordered_map>
+#include <memory>
+
 namespace storage {
 
-struct DataStoredMetrics : public metrics::MetricSet
-{
+struct DataStoredMetrics : metrics::MetricSet {
     typedef std::shared_ptr<DataStoredMetrics> SP;
 
     metrics::LongValueMetric buckets;
@@ -17,20 +20,35 @@ struct DataStoredMetrics : public metrics::MetricSet
     metrics::LongValueMetric ready;
 
     DataStoredMetrics(const std::string& name, metrics::MetricSet* owner);
-    ~DataStoredMetrics();
+    ~DataStoredMetrics() override;
 };
 
-class BucketManagerMetrics : public metrics::MetricSet
-{
+struct BucketSpaceMetrics : metrics::MetricSet {
+    // Superficially very similar to DataStoredMetrics, but metric naming and dimensions differ
+    metrics::LongValueMetric buckets_total;
+    metrics::LongValueMetric docs;
+    metrics::LongValueMetric bytes;
+    metrics::LongValueMetric active_buckets;
+    metrics::LongValueMetric ready_buckets;
+
+    BucketSpaceMetrics(const vespalib::string& space_name, metrics::MetricSet* owner);
+    ~BucketSpaceMetrics() override;
+};
+
+class ContentBucketSpaceRepo;
+
+class BucketManagerMetrics : public metrics::MetricSet {
 public:
-    std::vector<std::shared_ptr<DataStoredMetrics> > disks;
+    std::vector<std::shared_ptr<DataStoredMetrics>> disks;
+    using BucketSpaceMap = std::unordered_map<document::BucketSpace, std::unique_ptr<BucketSpaceMetrics>, document::BucketSpace::hash>;
+    BucketSpaceMap bucket_spaces;
     metrics::SumMetric<metrics::MetricSet> total;
     metrics::LongValueMetric simpleBucketInfoRequestSize;
     metrics::LongAverageMetric fullBucketInfoRequestSize;
     metrics::LongAverageMetric fullBucketInfoLatency;
 
-    BucketManagerMetrics();
-    ~BucketManagerMetrics();
+    explicit BucketManagerMetrics(const ContentBucketSpaceRepo& repo);
+    ~BucketManagerMetrics() override;
     void setDisks(uint16_t numDisks);
 };