Merge pull request #19988 from vespa-engine/balder/less-eager-compaction-during-retirementv7.500.40

When node is retired we can relax compaction strategy as we have peaked

12 files changed, 159 insertions, 18 deletions

diff --git a/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp b/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp
index 60d5f57ac8c..5e42231d866 100644
--- a/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/document_subdbs/document_subdbs_test.cpp
@@ -305,7 +305,7 @@ struct FixtureBase
           _bucketDBHandler(*_bucketDB),
           _ctx(_writeService, _bucketDB, _bucketDBHandler),
           _baseSchema(),
-          _snapshot(new MyConfigSnapshot(_baseSchema, Traits::ConfigDir::dir())),
+          _snapshot(std::make_unique<MyConfigSnapshot>(_baseSchema, Traits::ConfigDir::dir())),
           _baseDir(BASE_DIR + "/" + SUB_NAME, BASE_DIR),
           _subDb(_cfg._cfg, _ctx._ctx),
           _tmpFeedView()
@@ -557,6 +557,60 @@ TEST_F("require that attribute manager can be reconfigured", SearchableFixture)
     requireThatAttributeManagerCanBeReconfigured(f);
 }
 
+TEST_F("require that subdb reflect retirement", FastAccessFixture)
+{
+    search::CompactionStrategy cfg(0.1, 0.3);
+
+    EXPECT_FALSE(f._subDb.isNodeRetired());
+    auto unretired_cfg = f._subDb.computeCompactionStrategy(cfg);
+    EXPECT_TRUE(cfg == unretired_cfg);
+
+    auto calc = std::make_shared<proton::test::BucketStateCalculator>();
+    calc->setNodeRetired(true);
+    f._subDb.setBucketStateCalculator(calc);
+    EXPECT_TRUE(f._subDb.isNodeRetired());
+    auto retired_cfg = f._subDb.computeCompactionStrategy(cfg);
+    EXPECT_TRUE(cfg != retired_cfg);
+    EXPECT_TRUE(search::CompactionStrategy(0.5, 0.5) == retired_cfg);
+
+    calc->setNodeRetired(false);
+    f._subDb.setBucketStateCalculator(calc);
+    EXPECT_FALSE(f._subDb.isNodeRetired());
+    unretired_cfg = f._subDb.computeCompactionStrategy(cfg);
+    EXPECT_TRUE(cfg == unretired_cfg);
+}
+
+TEST_F("require that attribute compaction config reflect retirement", FastAccessFixture) {
+    search::CompactionStrategy default_cfg(0.05, 0.2);
+    search::CompactionStrategy retired_cfg(0.5, 0.5);
+
+    auto guard = f._subDb.getAttributeManager()->getAttribute("attr1");
+    EXPECT_EQUAL(default_cfg, (*guard)->getConfig().getCompactionStrategy());
+    EXPECT_EQUAL(default_cfg, dynamic_cast<const proton::DocumentMetaStore &>(f._subDb.getDocumentMetaStoreContext().get()).getConfig().getCompactionStrategy());
+
+    auto calc = std::make_shared<proton::test::BucketStateCalculator>();
+    calc->setNodeRetired(true);
+    f._subDb.setBucketStateCalculator(calc);
+    f._writeService.sync();
+    guard = f._subDb.getAttributeManager()->getAttribute("attr1");
+    EXPECT_EQUAL(retired_cfg, (*guard)->getConfig().getCompactionStrategy());
+    EXPECT_EQUAL(retired_cfg, dynamic_cast<const proton::DocumentMetaStore &>(f._subDb.getDocumentMetaStoreContext().get()).getConfig().getCompactionStrategy());
+
+    f.basicReconfig(10);
+    f._writeService.sync();
+    guard = f._subDb.getAttributeManager()->getAttribute("attr1");
+    EXPECT_EQUAL(retired_cfg, (*guard)->getConfig().getCompactionStrategy());
+    EXPECT_EQUAL(retired_cfg, dynamic_cast<const proton::DocumentMetaStore &>(f._subDb.getDocumentMetaStoreContext().get()).getConfig().getCompactionStrategy());
+
+    calc->setNodeRetired(false);
+    f._subDb.setBucketStateCalculator(calc);
+    f._writeService.sync();
+    guard = f._subDb.getAttributeManager()->getAttribute("attr1");
+    EXPECT_EQUAL(default_cfg, (*guard)->getConfig().getCompactionStrategy());
+    EXPECT_EQUAL(default_cfg, dynamic_cast<const proton::DocumentMetaStore &>(f._subDb.getDocumentMetaStoreContext().get()).getConfig().getCompactionStrategy());
+
+}
+
 template <typename Fixture>
 void
 requireThatReconfiguredAttributesAreAccessibleViaFeedView(Fixture &f)
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
index 4b6e67212e1..032be9e1dc8 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp
@@ -575,6 +575,21 @@ AttributeManager::asyncForEachAttribute(std::shared_ptr<IConstAttributeFunctor>
 {
     for (const auto &attr : _attributes) {
         if (attr.second.isExtra()) {
+            // We must skip extra attributes as they must be handled in other threads. (DocumentMetaStore)
+            continue;
+        }
+        AttributeVector::SP attrsp = attr.second.getAttribute();
+        _attributeFieldWriter.execute(_attributeFieldWriter.getExecutorIdFromName(attrsp->getNamePrefix()),
+                                      [attrsp, func]() { (*func)(*attrsp); });
+    }
+}
+
+void
+AttributeManager::asyncForEachAttribute(std::shared_ptr<IAttributeFunctor> func) const
+{
+    for (const auto &attr : _attributes) {
+        if (attr.second.isExtra()) {
+            // We must skip extra attributes as they must be handled in other threads.(DocumentMetaStore)
             continue;
         }
         AttributeVector::SP attrsp = attr.second.getAttribute();
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h
index 64f0418c299..e2b9550435d 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h
+++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.h
@@ -178,6 +178,7 @@ public:
     const std::vector<search::AttributeVector *> &getWritableAttributes() const override;
 
     void asyncForEachAttribute(std::shared_ptr<IConstAttributeFunctor> func) const override;
+    void asyncForEachAttribute(std::shared_ptr<IAttributeFunctor> func) const override;
     void asyncForAttribute(const vespalib::string &name, std::unique_ptr<IAttributeFunctor> func) const override;
 
     ExclusiveAttributeReadAccessor::UP getExclusiveReadAccessor(const vespalib::string &name) const override;
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp
index 07bc1c638b5..c7ab83ae590 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.cpp
@@ -206,6 +206,22 @@ FilterAttributeManager::asyncForEachAttribute(std::shared_ptr<IConstAttributeFun
 }
 
 void
+FilterAttributeManager::asyncForEachAttribute(std::shared_ptr<IAttributeFunctor> func) const
+{
+    // Run by document db master thread
+    std::vector<AttributeGuard> completeList;
+    _mgr->getAttributeList(completeList);
+    vespalib::ISequencedTaskExecutor &attributeFieldWriter = getAttributeFieldWriter();
+    for (auto &guard : completeList) {
+        search::AttributeVector::SP attrsp = guard.getSP();
+        // Name must be extracted in document db master thread or attribute
+        // writer thread
+        attributeFieldWriter.execute(attributeFieldWriter.getExecutorIdFromName(attrsp->getNamePrefix()),
+                                     [attrsp, func]() { (*func)(*attrsp); });
+    }
+}
+
+void
 FilterAttributeManager::asyncForAttribute(const vespalib::string &name, std::unique_ptr<IAttributeFunctor> func) const {
     AttributeGuard::UP attr = _mgr->getAttribute(name);
     if (!attr) { return; }
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h
index 9d09aef8faf..1ae5f452218 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h
+++ b/searchcore/src/vespa/searchcore/proton/attribute/filter_attribute_manager.h
@@ -52,6 +52,7 @@ public:
     search::AttributeVector * getWritableAttribute(const vespalib::string &name) const override;
     const std::vector<search::AttributeVector *> & getWritableAttributes() const override;
     void asyncForEachAttribute(std::shared_ptr<IConstAttributeFunctor> func) const override;
+    void asyncForEachAttribute(std::shared_ptr<IAttributeFunctor> func) const override;
     ExclusiveAttributeReadAccessor::UP getExclusiveReadAccessor(const vespalib::string &name) const override;
     void setImportedAttributes(std::unique_ptr<ImportedAttributesRepo> attributes) override;
     const ImportedAttributesRepo *getImportedAttributes() const override;
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h b/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h
index 65796fd4c74..d55cd45d014 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h
+++ b/searchcore/src/vespa/searchcore/proton/attribute/i_attribute_manager.h
@@ -98,6 +98,7 @@ struct IAttributeManager : public search::IAttributeManager
     virtual const std::vector<search::AttributeVector *> &getWritableAttributes() const = 0;
 
     virtual void asyncForEachAttribute(std::shared_ptr<IConstAttributeFunctor> func) const = 0;
+    virtual void asyncForEachAttribute(std::shared_ptr<IAttributeFunctor> func) const = 0;
 
     virtual ExclusiveAttributeReadAccessor::UP getExclusiveReadAccessor(const vespalib::string &name) const = 0;
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/fast_access_doc_subdb.cpp b/searchcore/src/vespa/searchcore/proton/server/fast_access_doc_subdb.cpp
index 5bd9ba64bae..8451f3268b8 100644
--- a/searchcore/src/vespa/searchcore/proton/server/fast_access_doc_subdb.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/fast_access_doc_subdb.cpp
@@ -10,13 +10,11 @@
 #include <vespa/searchcore/proton/attribute/attribute_collection_spec_factory.h>
 #include <vespa/searchcore/proton/attribute/attribute_factory.h>
 #include <vespa/searchcore/proton/attribute/attribute_manager_initializer.h>
-#include <vespa/searchcore/proton/attribute/attribute_populator.h>
 #include <vespa/searchcore/proton/attribute/filter_attribute_manager.h>
 #include <vespa/searchcore/proton/attribute/sequential_attributes_initializer.h>
 #include <vespa/searchcore/proton/common/alloc_config.h>
 #include <vespa/searchcore/proton/matching/sessionmanager.h>
 #include <vespa/searchcore/proton/reprocessing/attribute_reprocessing_initializer.h>
-#include <vespa/searchcore/proton/reprocessing/document_reprocessing_handler.h>
 #include <vespa/searchcore/proton/reprocessing/reprocess_documents_task.h>
 #include <vespa/searchlib/docstore/document_store_visitor_progress.h>
 #include <vespa/log/log.h>
@@ -275,6 +273,9 @@ FastAccessDocSubDB::applyConfig(const DocumentDBConfig &newConfigSnapshot, const
             reconfigureAttributeMetrics(*newMgr, *oldMgr);
         }
         _iFeedView.set(_fastAccessFeedView.get());
+        if (isNodeRetired()) {
+            reconfigureAttributesConsideringNodeState();
+        }
     }
     return tasks;
 }
diff --git a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.cpp
index 3322722a642..b512143c5fd 100644
--- a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.cpp
@@ -5,7 +5,6 @@
 #include "document_subdb_initializer.h"
 #include "reconfig_params.h"
 #include "i_document_subdb_owner.h"
-#include "ibucketstatecalculator.h"
 #include <vespa/searchcore/proton/attribute/attribute_writer.h>
 #include <vespa/searchcore/proton/common/alloc_config.h>
 #include <vespa/searchcore/proton/flushengine/threadedflushtarget.h>
@@ -43,8 +42,7 @@ SearchableDocSubDB::SearchableDocSubDB(const Config &cfg, const Context &ctx)
                   getSubDbName(), ctx._fastUpdCtx._storeOnlyCtx._owner.getDistributionKey()),
       _warmupExecutor(ctx._warmupExecutor),
       _realGidToLidChangeHandler(std::make_shared<GidToLidChangeHandler>()),
-      _flushConfig(),
-      _nodeRetired(false)
+      _flushConfig()
 {
     _gidToLidChangeHandler = _realGidToLidChangeHandler;
 }
@@ -177,14 +175,14 @@ SearchableDocSubDB::applyFlushConfig(const DocumentDBFlushConfig &flushConfig)
 void
 SearchableDocSubDB::propagateFlushConfig()
 {
-    uint32_t maxFlushed = _nodeRetired ? _flushConfig.getMaxFlushedRetired() : _flushConfig.getMaxFlushed();
+    uint32_t maxFlushed = isNodeRetired() ? _flushConfig.getMaxFlushedRetired() : _flushConfig.getMaxFlushed();
     _indexMgr->setMaxFlushed(maxFlushed);
 }
 
 void
 SearchableDocSubDB::setBucketStateCalculator(const std::shared_ptr<IBucketStateCalculator> &calc)
 {
-    _nodeRetired = calc->nodeRetired();
+    FastAccessDocSubDB::setBucketStateCalculator(calc);
     propagateFlushConfig();
 }
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h
index c310aeb2a2b..2e7aac0a8d3 100644
--- a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h
+++ b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h
@@ -81,7 +81,6 @@ private:
     vespalib::SyncableThreadExecutor           &_warmupExecutor;
     std::shared_ptr<GidToLidChangeHandler>      _realGidToLidChangeHandler;
     DocumentDBFlushConfig                       _flushConfig;
-    bool                                        _nodeRetired;
 
     // Note: lifetime of indexManager must be handled by caller.
     std::shared_ptr<initializer::InitializerTask>
diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp
index 7ab60270411..97e55c37aff 100644
--- a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp
@@ -1,5 +1,6 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
+#include "storeonlydocsubdb.h"
 #include "docstorevalidator.h"
 #include "document_subdb_initializer.h"
 #include "document_subdb_initializer_result.h"
@@ -7,7 +8,7 @@
 #include "i_document_subdb_owner.h"
 #include "minimal_document_retriever.h"
 #include "reconfig_params.h"
-#include "storeonlydocsubdb.h"
+#include "ibucketstatecalculator.h"
 #include <vespa/searchcore/proton/attribute/attribute_writer.h>
 #include <vespa/searchcore/proton/bucketdb/ibucketdbhandlerinitializer.h>
 #include <vespa/searchcore/proton/common/alloc_config.h>
@@ -111,6 +112,8 @@ StoreOnlyDocSubDB::StoreOnlyDocSubDB(const Config &cfg, const Context &ctx)
       _dmsFlushTarget(),
       _dmsShrinkTarget(),
       _pendingLidsForCommit(std::make_shared<PendingLidTracker>()),
+      _nodeRetired(false),
+      _lastConfiguredCompactionStrategy(),
       _subDbId(cfg._subDbId),
       _subDbType(cfg._subDbType),
       _fileHeaderContext(ctx._fileHeaderContext, _docTypeName, _baseDir),
@@ -280,6 +283,7 @@ StoreOnlyDocSubDB::setupDocumentMetaStore(DocumentMetaStoreInitializerResult::SP
     _dmsShrinkTarget = std::make_shared<ShrinkLidSpaceFlushTarget>("documentmetastore.shrink", Type::GC,
                                                                    Component::ATTRIBUTE, _flushedDocumentMetaStoreSerialNum,
                                                                    _dmsFlushTarget->getLastFlushTime(), dms);
+    _lastConfiguredCompactionStrategy = dms->getConfig().getCompactionStrategy();
 }
 
 DocumentSubDbInitializer::UP
@@ -413,22 +417,68 @@ StoreOnlyDocSubDB::applyConfig(const DocumentDBConfig &newConfigSnapshot, const
     return IReprocessingTask::List();
 }
 
+namespace {
+
+constexpr double RETIRED_DEAD_RATIO = 0.5;
+
+struct UpdateConfig : public search::attribute::IAttributeFunctor {
+    UpdateConfig(search::CompactionStrategy compactionStrategy) noexcept
+        : _compactionStrategy(compactionStrategy)
+    {}
+    void operator()(search::attribute::IAttributeVector &iAttributeVector) override {
+        auto attributeVector = dynamic_cast<search::AttributeVector *>(&iAttributeVector);
+        if (attributeVector != nullptr) {
+            auto cfg = attributeVector->getConfig();
+            cfg.setCompactionStrategy(_compactionStrategy);
+            attributeVector->update_config(cfg);
+        }
+    }
+    search::CompactionStrategy _compactionStrategy;
+};
+
+}
+
+search::CompactionStrategy
+StoreOnlyDocSubDB::computeCompactionStrategy(search::CompactionStrategy strategy) const {
+    return isNodeRetired()
+           ? search::CompactionStrategy(RETIRED_DEAD_RATIO, RETIRED_DEAD_RATIO)
+           : strategy;
+}
+
 void
 StoreOnlyDocSubDB::reconfigure(const search::LogDocumentStore::Config & config, const AllocStrategy& alloc_strategy)
 {
+    _lastConfiguredCompactionStrategy = alloc_strategy.get_compaction_strategy();
     auto cfg = _dms->getConfig();
     GrowStrategy grow = alloc_strategy.get_grow_strategy();
     // Amortize memory spike cost over N docs
     grow.setDocsGrowDelta(grow.getDocsGrowDelta() + alloc_strategy.get_amortize_count());
     cfg.setGrowStrategy(grow);
-    cfg.setCompactionStrategy(alloc_strategy.get_compaction_strategy());
+    cfg.setCompactionStrategy(computeCompactionStrategy(alloc_strategy.get_compaction_strategy()));
     _dms->update_config(cfg); // Update grow and compaction config
     _rSummaryMgr->reconfigure(config);
 }
 
 void
-StoreOnlyDocSubDB::setBucketStateCalculator(const std::shared_ptr<IBucketStateCalculator> &)
-{
+StoreOnlyDocSubDB::setBucketStateCalculator(const std::shared_ptr<IBucketStateCalculator> & calc) {
+    bool wasNodeRetired = isNodeRetired();
+    _nodeRetired = calc->nodeRetired();
+    if (wasNodeRetired != isNodeRetired()) {
+        search::CompactionStrategy compactionStrategy = computeCompactionStrategy(_lastConfiguredCompactionStrategy);
+        auto cfg = _dms->getConfig();
+        cfg.setCompactionStrategy(compactionStrategy);
+        _dms->update_config(cfg);
+        reconfigureAttributesConsideringNodeState();
+    }
+}
+
+void
+StoreOnlyDocSubDB::reconfigureAttributesConsideringNodeState() {
+    search::CompactionStrategy compactionStrategy = computeCompactionStrategy(_lastConfiguredCompactionStrategy);
+    auto attrMan = getAttributeManager();
+    if (attrMan) {
+        attrMan->asyncForEachAttribute(std::make_shared<UpdateConfig>(compactionStrategy));
+    }
 }
 
 proton::IAttributeManager::SP
diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h
index 50e5ffba0e2..7051722f605 100644
--- a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h
+++ b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h
@@ -152,6 +152,8 @@ private:
     DocumentMetaStoreFlushTarget::SP _dmsFlushTarget;
     std::shared_ptr<ShrinkLidSpaceFlushTarget> _dmsShrinkTarget;
     std::shared_ptr<PendingLidTrackerBase>     _pendingLidsForCommit;
+    bool                                       _nodeRetired;
+    search::CompactionStrategy                 _lastConfiguredCompactionStrategy;
 
     IFlushTargetList getFlushTargets() override;
 protected:
@@ -180,9 +182,8 @@ protected:
     StoreOnlyFeedView::Context getStoreOnlyFeedViewContext(const DocumentDBConfig &configSnapshot);
     StoreOnlyFeedView::PersistentParams getFeedViewPersistentParams();
     vespalib::string getSubDbName() const;
-
-    void reconfigure(const search::LogDocumentStore::Config & protonConfig,
-                     const AllocStrategy& alloc_strategy);
+    void reconfigure(const search::LogDocumentStore::Config & protonConfig, const AllocStrategy& alloc_strategy);
+    void reconfigureAttributesConsideringNodeState();
 public:
     StoreOnlyDocSubDB(const Config &cfg, const Context &ctx);
     ~StoreOnlyDocSubDB() override;
@@ -233,6 +234,9 @@ public:
     std::shared_ptr<IDocumentDBReference> getDocumentDBReference() override;
     void tearDownReferences(IDocumentDBReferenceResolver &resolver) override;
     PendingLidTrackerBase & getUncommittedLidsTracker() override { return *_pendingLidsForCommit; }
+    search::CompactionStrategy computeCompactionStrategy(search::CompactionStrategy strategy) const;
+    bool isNodeRetired() const { return _nodeRetired; }
+
 };
 
 }
diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h b/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h
index deb6639c855..abc8eb679dd 100644
--- a/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h
+++ b/searchcore/src/vespa/searchcore/proton/test/mock_attribute_manager.h
@@ -86,8 +86,9 @@ public:
     const std::vector<search::AttributeVector *> &getWritableAttributes() const override {
         return _writables;
     }
-    void asyncForEachAttribute(std::shared_ptr<IConstAttributeFunctor>) const override {
-    }
+    void asyncForEachAttribute(std::shared_ptr<IConstAttributeFunctor>) const override { }
+    void asyncForEachAttribute(std::shared_ptr<IAttributeFunctor>) const override { }
+
     ExclusiveAttributeReadAccessor::UP getExclusiveReadAccessor(const vespalib::string &) const override {
         return ExclusiveAttributeReadAccessor::UP();
     }