From caeec93d81f88de278a7b324a6e023ee4449ee7b Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 12 Nov 2021 08:59:43 +0000 Subject: When node is retired we can relax compaction strategy as we have peaked in memory usage and will go reduce from here. --- .../proton/attribute/attributemanager.cpp | 11 +++++++ .../searchcore/proton/attribute/attributemanager.h | 1 + .../proton/attribute/filter_attribute_manager.cpp | 16 ++++++++++ .../proton/attribute/filter_attribute_manager.h | 1 + .../proton/attribute/i_attribute_manager.h | 1 + .../proton/server/searchabledocsubdb.cpp | 8 ++--- .../searchcore/proton/server/searchabledocsubdb.h | 1 - .../searchcore/proton/server/storeonlydocsubdb.cpp | 36 ++++++++++++++++++++-- .../searchcore/proton/server/storeonlydocsubdb.h | 6 ++-- .../proton/test/mock_attribute_manager.h | 5 +-- 10 files changed, 72 insertions(+), 14 deletions(-) diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp index 4b6e67212e1..b4fc20d0c44 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp @@ -575,6 +575,7 @@ AttributeManager::asyncForEachAttribute(std::shared_ptr { for (const auto &attr : _attributes) { if (attr.second.isExtra()) { + // TODO Why skip extra ? continue; } AttributeVector::SP attrsp = attr.second.getAttribute(); @@ -583,6 +584,16 @@ AttributeManager::asyncForEachAttribute(std::shared_ptr } } +void +AttributeManager::asyncForEachAttribute(std::shared_ptr func) const +{ + for (const auto &attr : _attributes) { + AttributeVector::SP attrsp = attr.second.getAttribute(); + _attributeFieldWriter.execute(_attributeFieldWriter.getExecutorIdFromName(attrsp->getNamePrefix()), + [attrsp, func]() { (*func)(*attrsp); }); + } +} + void AttributeManager::asyncForAttribute(const vespalib::string &name, std::unique_ptr func) const { auto itr = _attributes.find(name); 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 &getWritableAttributes() const override; void asyncForEachAttribute(std::shared_ptr func) const override; + void asyncForEachAttribute(std::shared_ptr func) const override; void asyncForAttribute(const vespalib::string &name, std::unique_ptr 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 @@ -205,6 +205,22 @@ FilterAttributeManager::asyncForEachAttribute(std::shared_ptr func) const +{ + // Run by document db master thread + std::vector 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 func) const { AttributeGuard::UP attr = _mgr->getAttribute(name); 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 & getWritableAttributes() const override; void asyncForEachAttribute(std::shared_ptr func) const override; + void asyncForEachAttribute(std::shared_ptr func) const override; ExclusiveAttributeReadAccessor::UP getExclusiveReadAccessor(const vespalib::string &name) const override; void setImportedAttributes(std::unique_ptr 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 &getWritableAttributes() const = 0; virtual void asyncForEachAttribute(std::shared_ptr func) const = 0; + virtual void asyncForEachAttribute(std::shared_ptr func) const = 0; virtual ExclusiveAttributeReadAccessor::UP getExclusiveReadAccessor(const vespalib::string &name) const = 0; 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 #include #include @@ -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()), - _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 &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 _realGidToLidChangeHandler; DocumentDBFlushConfig _flushConfig; - bool _nodeRetired; // Note: lifetime of indexManager must be handled by caller. std::shared_ptr diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp index 7ab60270411..1dbda17855b 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 #include #include @@ -111,6 +112,7 @@ StoreOnlyDocSubDB::StoreOnlyDocSubDB(const Config &cfg, const Context &ctx) _dmsFlushTarget(), _dmsShrinkTarget(), _pendingLidsForCommit(std::make_shared()), + _nodeRetired(false), _subDbId(cfg._subDbId), _subDbType(cfg._subDbType), _fileHeaderContext(ctx._fileHeaderContext, _docTypeName, _baseDir), @@ -426,9 +428,37 @@ StoreOnlyDocSubDB::reconfigure(const search::LogDocumentStore::Config & config, _rSummaryMgr->reconfigure(config); } +namespace { + +constexpr double RETIRED_DEAD_RATIO = 0.5; + +struct UpdateConfig : public search::attribute::IAttributeFunctor { + void operator()(search::attribute::IAttributeVector &iAttributeVector) override { + auto attributeVector = dynamic_cast(&iAttributeVector); + if (attributeVector != nullptr) { + auto cfg = attributeVector->getConfig(); + cfg.setCompactionStrategy(search::CompactionStrategy(RETIRED_DEAD_RATIO, RETIRED_DEAD_RATIO)); + attributeVector->update_config(cfg); + } + } +}; + +} + void -StoreOnlyDocSubDB::setBucketStateCalculator(const std::shared_ptr &) -{ +StoreOnlyDocSubDB::setBucketStateCalculator(const std::shared_ptr & calc) +{ + _nodeRetired = calc->nodeRetired(); + if (isNodeRetired()) { + auto cfg = _dms->getConfig(); + cfg.setCompactionStrategy(search::CompactionStrategy(RETIRED_DEAD_RATIO, RETIRED_DEAD_RATIO)); + _dms->update_config(cfg); + + auto attrMan = getAttributeManager(); + if (attrMan) { + attrMan->asyncForEachAttribute(std::make_shared()); + } + } } 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..3dbf6dfdf07 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h @@ -152,6 +152,7 @@ private: DocumentMetaStoreFlushTarget::SP _dmsFlushTarget; std::shared_ptr _dmsShrinkTarget; std::shared_ptr _pendingLidsForCommit; + bool _nodeRetired; IFlushTargetList getFlushTargets() override; protected: @@ -180,9 +181,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); + bool isNodeRetired() const { return _nodeRetired; } + void reconfigure(const search::LogDocumentStore::Config & protonConfig, const AllocStrategy& alloc_strategy); public: StoreOnlyDocSubDB(const Config &cfg, const Context &ctx); ~StoreOnlyDocSubDB() override; 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 &getWritableAttributes() const override { return _writables; } - void asyncForEachAttribute(std::shared_ptr) const override { - } + void asyncForEachAttribute(std::shared_ptr) const override { } + void asyncForEachAttribute(std::shared_ptr) const override { } + ExclusiveAttributeReadAccessor::UP getExclusiveReadAccessor(const vespalib::string &) const override { return ExclusiveAttributeReadAccessor::UP(); } -- cgit v1.2.3 From 8a0fc71d26790955ef72c166fd46af00439c1272 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 12 Nov 2021 10:18:21 +0000 Subject: Consider retirement on every cluster change and reconfig. --- .../proton/attribute/attributemanager.cpp | 6 +++- .../searchcore/proton/server/storeonlydocsubdb.cpp | 41 ++++++++++++++-------- .../searchcore/proton/server/storeonlydocsubdb.h | 1 + 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp index b4fc20d0c44..032be9e1dc8 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp @@ -575,7 +575,7 @@ AttributeManager::asyncForEachAttribute(std::shared_ptr { for (const auto &attr : _attributes) { if (attr.second.isExtra()) { - // TODO Why skip extra ? + // We must skip extra attributes as they must be handled in other threads. (DocumentMetaStore) continue; } AttributeVector::SP attrsp = attr.second.getAttribute(); @@ -588,6 +588,10 @@ void AttributeManager::asyncForEachAttribute(std::shared_ptr 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(); _attributeFieldWriter.execute(_attributeFieldWriter.getExecutorIdFromName(attrsp->getNamePrefix()), [attrsp, func]() { (*func)(*attrsp); }); diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp index 1dbda17855b..5b1b547fcfc 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp @@ -113,6 +113,7 @@ StoreOnlyDocSubDB::StoreOnlyDocSubDB(const Config &cfg, const Context &ctx) _dmsShrinkTarget(), _pendingLidsForCommit(std::make_shared()), _nodeRetired(false), + _lastConfiguredCompactionStrategy(), _subDbId(cfg._subDbId), _subDbType(cfg._subDbType), _fileHeaderContext(ctx._fileHeaderContext, _docTypeName, _baseDir), @@ -282,6 +283,7 @@ StoreOnlyDocSubDB::setupDocumentMetaStore(DocumentMetaStoreInitializerResult::SP _dmsShrinkTarget = std::make_shared("documentmetastore.shrink", Type::GC, Component::ATTRIBUTE, _flushedDocumentMetaStoreSerialNum, _dmsFlushTarget->getLastFlushTime(), dms); + _lastConfiguredCompactionStrategy = dms->getConfig().getCompactionStrategy(); } DocumentSubDbInitializer::UP @@ -415,19 +417,6 @@ StoreOnlyDocSubDB::applyConfig(const DocumentDBConfig &newConfigSnapshot, const return IReprocessingTask::List(); } -void -StoreOnlyDocSubDB::reconfigure(const search::LogDocumentStore::Config & config, const AllocStrategy& alloc_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()); - _dms->update_config(cfg); // Update grow and compaction config - _rSummaryMgr->reconfigure(config); -} - namespace { constexpr double RETIRED_DEAD_RATIO = 0.5; @@ -443,15 +432,37 @@ struct UpdateConfig : public search::attribute::IAttributeFunctor { } }; +search::CompactionStrategy +computeCompactionStrategy(search::CompactionStrategy strategy, bool isNodeRetired) { + 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(computeCompactionStrategy(alloc_strategy.get_compaction_strategy(), isNodeRetired())); + _dms->update_config(cfg); // Update grow and compaction config + _rSummaryMgr->reconfigure(config); } void StoreOnlyDocSubDB::setBucketStateCalculator(const std::shared_ptr & calc) { + bool wasNodeRetired = isNodeRetired(); _nodeRetired = calc->nodeRetired(); - if (isNodeRetired()) { + if (wasNodeRetired != isNodeRetired()) { auto cfg = _dms->getConfig(); - cfg.setCompactionStrategy(search::CompactionStrategy(RETIRED_DEAD_RATIO, RETIRED_DEAD_RATIO)); + cfg.setCompactionStrategy(computeCompactionStrategy(_lastConfiguredCompactionStrategy, isNodeRetired())); _dms->update_config(cfg); auto attrMan = getAttributeManager(); diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h index 3dbf6dfdf07..ddb6522493f 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h @@ -153,6 +153,7 @@ private: std::shared_ptr _dmsShrinkTarget; std::shared_ptr _pendingLidsForCommit; bool _nodeRetired; + search::CompactionStrategy _lastConfiguredCompactionStrategy; IFlushTargetList getFlushTargets() override; protected: -- cgit v1.2.3 From f370d1e0a6a9fae3d0cb2b5240562f61703f25f7 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 12 Nov 2021 13:20:07 +0000 Subject: Correct config for retired nodes both after reconfig and retired -> up for both attributes and document meta store. --- .../proton/server/fast_access_doc_subdb.cpp | 5 +-- .../searchcore/proton/server/storeonlydocsubdb.cpp | 37 ++++++++++++++-------- .../searchcore/proton/server/storeonlydocsubdb.h | 2 ++ 3 files changed, 28 insertions(+), 16 deletions(-) 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 #include #include -#include #include #include #include #include #include -#include #include #include #include @@ -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/storeonlydocsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp index 5b1b547fcfc..97e55c37aff 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.cpp @@ -422,23 +422,27 @@ 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(&iAttributeVector); if (attributeVector != nullptr) { auto cfg = attributeVector->getConfig(); - cfg.setCompactionStrategy(search::CompactionStrategy(RETIRED_DEAD_RATIO, RETIRED_DEAD_RATIO)); + cfg.setCompactionStrategy(_compactionStrategy); attributeVector->update_config(cfg); } } + search::CompactionStrategy _compactionStrategy; }; -search::CompactionStrategy -computeCompactionStrategy(search::CompactionStrategy strategy, bool isNodeRetired) { - return isNodeRetired - ? search::CompactionStrategy(RETIRED_DEAD_RATIO, RETIRED_DEAD_RATIO) - : strategy; } +search::CompactionStrategy +StoreOnlyDocSubDB::computeCompactionStrategy(search::CompactionStrategy strategy) const { + return isNodeRetired() + ? search::CompactionStrategy(RETIRED_DEAD_RATIO, RETIRED_DEAD_RATIO) + : strategy; } void @@ -450,25 +454,30 @@ StoreOnlyDocSubDB::reconfigure(const search::LogDocumentStore::Config & config, // Amortize memory spike cost over N docs grow.setDocsGrowDelta(grow.getDocsGrowDelta() + alloc_strategy.get_amortize_count()); cfg.setGrowStrategy(grow); - cfg.setCompactionStrategy(computeCompactionStrategy(alloc_strategy.get_compaction_strategy(), isNodeRetired())); + 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 & calc) -{ +StoreOnlyDocSubDB::setBucketStateCalculator(const std::shared_ptr & calc) { bool wasNodeRetired = isNodeRetired(); _nodeRetired = calc->nodeRetired(); if (wasNodeRetired != isNodeRetired()) { + search::CompactionStrategy compactionStrategy = computeCompactionStrategy(_lastConfiguredCompactionStrategy); auto cfg = _dms->getConfig(); - cfg.setCompactionStrategy(computeCompactionStrategy(_lastConfiguredCompactionStrategy, isNodeRetired())); + cfg.setCompactionStrategy(compactionStrategy); _dms->update_config(cfg); + reconfigureAttributesConsideringNodeState(); + } +} - auto attrMan = getAttributeManager(); - if (attrMan) { - attrMan->asyncForEachAttribute(std::make_shared()); - } +void +StoreOnlyDocSubDB::reconfigureAttributesConsideringNodeState() { + search::CompactionStrategy compactionStrategy = computeCompactionStrategy(_lastConfiguredCompactionStrategy); + auto attrMan = getAttributeManager(); + if (attrMan) { + attrMan->asyncForEachAttribute(std::make_shared(compactionStrategy)); } } diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h index ddb6522493f..10ef88d0eaa 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h @@ -184,6 +184,7 @@ protected: vespalib::string getSubDbName() const; bool isNodeRetired() const { return _nodeRetired; } void reconfigure(const search::LogDocumentStore::Config & protonConfig, const AllocStrategy& alloc_strategy); + void reconfigureAttributesConsideringNodeState(); public: StoreOnlyDocSubDB(const Config &cfg, const Context &ctx); ~StoreOnlyDocSubDB() override; @@ -234,6 +235,7 @@ public: std::shared_ptr getDocumentDBReference() override; void tearDownReferences(IDocumentDBReferenceResolver &resolver) override; PendingLidTrackerBase & getUncommittedLidsTracker() override { return *_pendingLidsForCommit; } + search::CompactionStrategy computeCompactionStrategy(search::CompactionStrategy strategy) const; }; } -- cgit v1.2.3 From 854ecccb1ea9fbe2d1e1be4e9bbe234b6c1e60f7 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 12 Nov 2021 14:04:32 +0000 Subject: Test that up -> retired -> up is reflectd correctly in document subdbs. --- .../document_subdbs/document_subdbs_test.cpp | 25 +++++++++++++++++++++- .../searchcore/proton/server/storeonlydocsubdb.h | 3 ++- 2 files changed, 26 insertions(+), 2 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..bd356973841 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(_baseSchema, Traits::ConfigDir::dir())), _baseDir(BASE_DIR + "/" + SUB_NAME, BASE_DIR), _subDb(_cfg._cfg, _ctx._ctx), _tmpFeedView() @@ -557,6 +557,29 @@ 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(); + 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); +} + template void requireThatReconfiguredAttributesAreAccessibleViaFeedView(Fixture &f) diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h index 10ef88d0eaa..7051722f605 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlydocsubdb.h @@ -182,7 +182,6 @@ protected: StoreOnlyFeedView::Context getStoreOnlyFeedViewContext(const DocumentDBConfig &configSnapshot); StoreOnlyFeedView::PersistentParams getFeedViewPersistentParams(); vespalib::string getSubDbName() const; - bool isNodeRetired() const { return _nodeRetired; } void reconfigure(const search::LogDocumentStore::Config & protonConfig, const AllocStrategy& alloc_strategy); void reconfigureAttributesConsideringNodeState(); public: @@ -236,6 +235,8 @@ public: void tearDownReferences(IDocumentDBReferenceResolver &resolver) override; PendingLidTrackerBase & getUncommittedLidsTracker() override { return *_pendingLidsForCommit; } search::CompactionStrategy computeCompactionStrategy(search::CompactionStrategy strategy) const; + bool isNodeRetired() const { return _nodeRetired; } + }; } -- cgit v1.2.3 From e834424c31fd2dc6c3268cf3b374c5731108cd6b Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 12 Nov 2021 20:23:03 +0000 Subject: Add test that compaction config is reflected properly in document metastore and attributes. --- .../document_subdbs/document_subdbs_test.cpp | 31 ++++++++++++++++++++++ 1 file changed, 31 insertions(+) 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 bd356973841..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 @@ -580,6 +580,37 @@ TEST_F("require that subdb reflect retirement", FastAccessFixture) 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(f._subDb.getDocumentMetaStoreContext().get()).getConfig().getCompactionStrategy()); + + auto calc = std::make_shared(); + 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(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(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(f._subDb.getDocumentMetaStoreContext().get()).getConfig().getCompactionStrategy()); + +} + template void requireThatReconfiguredAttributesAreAccessibleViaFeedView(Fixture &f) -- cgit v1.2.3