From 39f3f74d601087495a58628cce89587aae698adb Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 19 Oct 2020 20:39:02 +0000 Subject: - Avoid using PersitenceUtil as a global object that can reach everything. Only wire in what is needed. - GC unused code and make it explicit what is private and what is public. - Remove some traces of diskCount. --- storage/src/tests/common/teststorageapp.cpp | 3 -- storage/src/tests/common/teststorageapp.h | 6 --- .../src/tests/persistence/persistencetestutils.h | 3 +- .../src/vespa/storage/bucketdb/bucketmanager.cpp | 6 +-- .../vespa/storage/common/servicelayercomponent.cpp | 16 ------ .../vespa/storage/common/servicelayercomponent.h | 7 --- .../servicelayercomponentregisterimpl.cpp | 17 +------ .../component/servicelayercomponentregisterimpl.h | 3 -- .../src/vespa/storage/persistence/asynchandler.cpp | 8 +-- .../src/vespa/storage/persistence/asynchandler.h | 5 +- .../persistence/filestorage/filestormanager.cpp | 1 - .../storage/persistence/persistencehandler.cpp | 6 +-- .../vespa/storage/persistence/persistencehandler.h | 1 + .../vespa/storage/persistence/persistenceutil.cpp | 36 ++++++-------- .../vespa/storage/persistence/persistenceutil.h | 58 +++++++++------------- .../vespa/storage/persistence/testandsethelper.cpp | 8 +-- .../vespa/storage/persistence/testandsethelper.h | 9 +++- .../storage/storageserver/servicelayernode.cpp | 1 - 18 files changed, 69 insertions(+), 125 deletions(-) (limited to 'storage') diff --git a/storage/src/tests/common/teststorageapp.cpp b/storage/src/tests/common/teststorageapp.cpp index a2a0cbd1e68..2fc33bc1360 100644 --- a/storage/src/tests/common/teststorageapp.cpp +++ b/storage/src/tests/common/teststorageapp.cpp @@ -141,7 +141,6 @@ TestServiceLayerApp::TestServiceLayerApp(vespalib::stringref configId) _persistenceProvider(), _executor(vespalib::SequencedTaskExecutor::create(1)) { - _compReg.setDiskCount(1); lib::NodeState ns(*_nodeStateUpdater.getReportedNodeState()); ns.setDiskCount(1); _nodeStateUpdater.setReportedNodeState(ns); @@ -155,7 +154,6 @@ TestServiceLayerApp::TestServiceLayerApp(NodeIndex index, _persistenceProvider(), _executor(vespalib::SequencedTaskExecutor::create(1)) { - _compReg.setDiskCount(1); lib::NodeState ns(*_nodeStateUpdater.getReportedNodeState()); ns.setDiskCount(1); _nodeStateUpdater.setReportedNodeState(ns); @@ -166,7 +164,6 @@ TestServiceLayerApp::~TestServiceLayerApp() = default; void TestServiceLayerApp::setupDummyPersistence() { - assert(_compReg.getDiskCount() == 1u); auto provider = std::make_unique(getTypeRepo()); provider->initialize(); setPersistenceProvider(std::move(provider)); diff --git a/storage/src/tests/common/teststorageapp.h b/storage/src/tests/common/teststorageapp.h index ffb16f3646a..0a104b1655c 100644 --- a/storage/src/tests/common/teststorageapp.h +++ b/storage/src/tests/common/teststorageapp.h @@ -104,7 +104,6 @@ private: [[nodiscard]] virtual StorBucketDatabase& content_bucket_db(document::BucketSpace) { abort(); } virtual StorBucketDatabase& getStorageBucketDatabase() { abort(); } virtual BucketDatabase& getBucketDatabase() { abort(); } - virtual uint16_t getDiskCount() const { abort(); } }; class TestServiceLayerApp : public TestStorageApp @@ -134,11 +133,6 @@ public: return _compReg.getBucketSpaceRepo().get(document::FixedBucketSpaces::default_space()).bucketDatabase(); } vespalib::ISequencedTaskExecutor & executor() { return *_executor; } - -private: - // For storage server interface implementation we'll get rid of soon. - // Use getPartitions().size() instead. - uint16_t getDiskCount() const override { return _compReg.getDiskCount(); } }; class TestDistributorApp : public TestStorageApp, diff --git a/storage/src/tests/persistence/persistencetestutils.h b/storage/src/tests/persistence/persistencetestutils.h index 332a30393b1..29f77048b51 100644 --- a/storage/src/tests/persistence/persistencetestutils.h +++ b/storage/src/tests/persistence/persistencetestutils.h @@ -110,7 +110,8 @@ public: MessageTracker::UP createTracker(api::StorageMessage::SP cmd, document::Bucket bucket) { - return MessageTracker::createForTesting(getEnv(), _replySender, NoBucketLock::make(bucket), std::move(cmd)); + return MessageTracker::createForTesting(framework::MilliSecTimer(getEnv()._component.getClock()), getEnv(), + _replySender, NoBucketLock::make(bucket), std::move(cmd)); } api::ReturnCode diff --git a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp index ab22d8440fa..a63067ca156 100644 --- a/storage/src/vespa/storage/bucketdb/bucketmanager.cpp +++ b/storage/src/vespa/storage/bucketdb/bucketmanager.cpp @@ -50,7 +50,7 @@ BucketManager::BucketManager(const config::ConfigUri & configUri, _metrics(std::make_shared(_component.getBucketSpaceRepo())), _simulated_processing_delay(0) { - _metrics->setDisks(_component.getDiskCount()); + _metrics->setDisks(1); _component.registerStatusPage(*this); _component.registerMetric(*_metrics); _component.registerMetricUpdateHook(*this, framework::SecondTime(300)); @@ -231,7 +231,7 @@ BucketManager::updateMetrics(bool updateDocCount) updateDocCount ? "" : ", minusedbits only", _doneInitialized ? "" : ", server is not done initializing"); - uint16_t diskCount = _component.getDiskCount(); + const uint16_t diskCount = 1; assert(diskCount >= 1); if (!updateDocCount || _doneInitialized) { MetricsUpdater total(diskCount); @@ -275,7 +275,7 @@ void BucketManager::update_bucket_db_memory_usage_metrics() { void BucketManager::updateMinUsedBits() { - MetricsUpdater m(_component.getDiskCount()); + MetricsUpdater m(1); _component.getBucketSpaceRepo().for_each_bucket(std::ref(m)); // When going through to get sizes, we also record min bits MinimumUsedBitsTracker& bitTracker(_component.getMinUsedBitsTracker()); diff --git a/storage/src/vespa/storage/common/servicelayercomponent.cpp b/storage/src/vespa/storage/common/servicelayercomponent.cpp index 14e67679e24..b6cd9bc4aae 100644 --- a/storage/src/vespa/storage/common/servicelayercomponent.cpp +++ b/storage/src/vespa/storage/common/servicelayercomponent.cpp @@ -24,20 +24,4 @@ ServiceLayerComponent::getBucketDatabase(BucketSpace bucketSpace) const return _bucketSpaceRepo->get(bucketSpace).bucketDatabase(); } -uint16_t -ServiceLayerComponent::getIdealPartition(const document::Bucket& bucket) const -{ - return _bucketSpaceRepo->get(bucket.getBucketSpace()).getDistribution()->getIdealDisk( - *getStateUpdater().getReportedNodeState(), getIndex(), bucket.getBucketId(), - lib::Distribution::IDEAL_DISK_EVEN_IF_DOWN); -} - -uint16_t -ServiceLayerComponent::getPreferredAvailablePartition( - const document::Bucket& bucket) const -{ - return _bucketSpaceRepo->get(bucket.getBucketSpace()).getDistribution()->getPreferredAvailableDisk( - *getStateUpdater().getReportedNodeState(), getIndex(), bucket.getBucketId()); -} - } // storage diff --git a/storage/src/vespa/storage/common/servicelayercomponent.h b/storage/src/vespa/storage/common/servicelayercomponent.h index a75dacde1d8..1f1e26dbee4 100644 --- a/storage/src/vespa/storage/common/servicelayercomponent.h +++ b/storage/src/vespa/storage/common/servicelayercomponent.h @@ -38,7 +38,6 @@ struct ServiceLayerManagedComponent { virtual ~ServiceLayerManagedComponent() = default; - virtual void setDiskCount(uint16_t count) = 0; virtual void setBucketSpaceRepo(ContentBucketSpaceRepo&) = 0; virtual void setMinUsedBitsTracker(MinimumUsedBitsTracker&) = 0; }; @@ -51,12 +50,10 @@ struct ServiceLayerComponentRegister : public virtual StorageComponentRegister class ServiceLayerComponent : public StorageComponent, private ServiceLayerManagedComponent { - uint16_t _diskCount; ContentBucketSpaceRepo* _bucketSpaceRepo; MinimumUsedBitsTracker* _minUsedBitsTracker; // ServiceLayerManagedComponent implementation - void setDiskCount(uint16_t count) override { _diskCount = count; } void setBucketSpaceRepo(ContentBucketSpaceRepo& repo) override { _bucketSpaceRepo = &repo; } void setMinUsedBitsTracker(MinimumUsedBitsTracker& tracker) override { _minUsedBitsTracker = &tracker; @@ -67,14 +64,12 @@ public: ServiceLayerComponent(ServiceLayerComponentRegister& compReg, vespalib::stringref name) : StorageComponent(compReg, name), - _diskCount(0), _bucketSpaceRepo(nullptr), _minUsedBitsTracker(nullptr) { compReg.registerServiceLayerComponent(*this); } - uint16_t getDiskCount() const { return _diskCount; } const ContentBucketSpaceRepo &getBucketSpaceRepo() const; StorBucketDatabase& getBucketDatabase(document::BucketSpace bucketSpace) const; MinimumUsedBitsTracker& getMinUsedBitsTracker() { @@ -83,8 +78,6 @@ public: const MinimumUsedBitsTracker& getMinUsedBitsTracker() const { return *_minUsedBitsTracker; } - uint16_t getIdealPartition(const document::Bucket&) const; - uint16_t getPreferredAvailablePartition(const document::Bucket&) const; }; } // storage diff --git a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp index 66ec262250c..7a342c1df25 100644 --- a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp +++ b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp @@ -10,8 +10,7 @@ namespace storage { using vespalib::IllegalStateException; ServiceLayerComponentRegisterImpl::ServiceLayerComponentRegisterImpl(bool use_btree_db) - : _diskCount(0), - _bucketSpaceRepo(use_btree_db) + : _bucketSpaceRepo(use_btree_db) { } void @@ -19,24 +18,10 @@ ServiceLayerComponentRegisterImpl::registerServiceLayerComponent(ServiceLayerMan { std::lock_guard lock(_componentLock); _components.push_back(&smc); - smc.setDiskCount(_diskCount); smc.setBucketSpaceRepo(_bucketSpaceRepo); smc.setMinUsedBitsTracker(_minUsedBitsTracker); } -void -ServiceLayerComponentRegisterImpl::setDiskCount(uint16_t count) -{ - std::lock_guard lock(_componentLock); - if (_diskCount != 0) { - throw IllegalStateException("Disk count already set. Cannot be updated live", VESPA_STRLOC); - } - _diskCount = count; - for (uint32_t i=0; i<_components.size(); ++i) { - _components[i]->setDiskCount(count); - } -} - void ServiceLayerComponentRegisterImpl::setDistribution(lib::Distribution::SP distribution) { diff --git a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h index e722110d770..967adf8f787 100644 --- a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h +++ b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h @@ -20,7 +20,6 @@ class ServiceLayerComponentRegisterImpl { std::mutex _componentLock; std::vector _components; - uint16_t _diskCount; ContentBucketSpaceRepo _bucketSpaceRepo; MinimumUsedBitsTracker _minUsedBitsTracker; @@ -29,14 +28,12 @@ public: ServiceLayerComponentRegisterImpl(bool use_btree_db = false); - uint16_t getDiskCount() const { return _diskCount; } ContentBucketSpaceRepo& getBucketSpaceRepo() { return _bucketSpaceRepo; } MinimumUsedBitsTracker& getMinUsedBitsTracker() { return _minUsedBitsTracker; } void registerServiceLayerComponent(ServiceLayerManagedComponent&) override; - void setDiskCount(uint16_t count); void setDistribution(lib::Distribution::SP distribution) override; }; diff --git a/storage/src/vespa/storage/persistence/asynchandler.cpp b/storage/src/vespa/storage/persistence/asynchandler.cpp index 418eeb40ffc..1d1f5caf673 100644 --- a/storage/src/vespa/storage/persistence/asynchandler.cpp +++ b/storage/src/vespa/storage/persistence/asynchandler.cpp @@ -86,10 +86,12 @@ private: } AsyncHandler::AsyncHandler(const PersistenceUtil & env, spi::PersistenceProvider & spi, - vespalib::ISequencedTaskExecutor & executor) + vespalib::ISequencedTaskExecutor & executor, + const document::BucketIdFactory & bucketIdFactory) : _env(env), _spi(spi), - _sequencedExecutor(executor) + _sequencedExecutor(executor), + _bucketIdFactory(bucketIdFactory) {} MessageTracker::UP @@ -188,7 +190,7 @@ bool AsyncHandler::tasConditionMatches(const api::TestAndSetCommand & cmd, MessageTracker & tracker, spi::Context & context, bool missingDocumentImpliesMatch) const { try { - TestAndSetHelper helper(_env, _spi, cmd, missingDocumentImpliesMatch); + TestAndSetHelper helper(_env, _spi, _bucketIdFactory, cmd, missingDocumentImpliesMatch); auto code = helper.retrieveAndMatch(context); if (code.failed()) { diff --git a/storage/src/vespa/storage/persistence/asynchandler.h b/storage/src/vespa/storage/persistence/asynchandler.h index d7e068091c7..1719dcadeb3 100644 --- a/storage/src/vespa/storage/persistence/asynchandler.h +++ b/storage/src/vespa/storage/persistence/asynchandler.h @@ -4,6 +4,7 @@ #include "types.h" #include +namespace document { class BucketIdFactory; } namespace vespalib { class ISequencedTaskExecutor; } namespace storage { @@ -19,7 +20,8 @@ struct PersistenceUtil; */ class AsyncHandler : public Types { public: - AsyncHandler(const PersistenceUtil&, spi::PersistenceProvider&, vespalib::ISequencedTaskExecutor & executor); + AsyncHandler(const PersistenceUtil&, spi::PersistenceProvider&, vespalib::ISequencedTaskExecutor & executor, + const document::BucketIdFactory & bucketIdFactory); MessageTrackerUP handlePut(api::PutCommand& cmd, MessageTrackerUP tracker) const; MessageTrackerUP handleRemove(api::RemoveCommand& cmd, MessageTrackerUP tracker) const; MessageTrackerUP handleUpdate(api::UpdateCommand& cmd, MessageTrackerUP tracker) const; @@ -30,6 +32,7 @@ private: const PersistenceUtil & _env; spi::PersistenceProvider & _spi; vespalib::ISequencedTaskExecutor & _sequencedExecutor; + const document::BucketIdFactory & _bucketIdFactory; }; } // storage diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp index 832e07eaf95..4b35147f9fc 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp @@ -130,7 +130,6 @@ FileStorManager::configure(std::unique_ptrnumThreads; size_t numStripes = std::max(size_t(1u), numThreads / 2); _metrics->initDiskMetrics(1, _component.getLoadTypes()->getMetricLoadTypes(), numStripes, numThreads); diff --git a/storage/src/vespa/storage/persistence/persistencehandler.cpp b/storage/src/vespa/storage/persistence/persistencehandler.cpp index 13ff1df340b..239a133fd02 100644 --- a/storage/src/vespa/storage/persistence/persistencehandler.cpp +++ b/storage/src/vespa/storage/persistence/persistencehandler.cpp @@ -14,13 +14,13 @@ PersistenceHandler::PersistenceHandler(vespalib::ISequencedTaskExecutor & sequen FileStorHandler& filestorHandler, BucketOwnershipNotifier & bucketOwnershipNotifier, FileStorThreadMetrics& metrics) - : + : _clock(component.getClock()), _env(component, filestorHandler, metrics, provider), _processAllHandler(_env, provider), _mergeHandler(_env, provider, cfg.bucketMergeChunkSize, cfg.enableMergeLocalNodeChooseDocsOptimalization, cfg.commonMergeChainOptimalizationMinimumSize), - _asyncHandler(_env, provider, sequencedExecutor), + _asyncHandler(_env, provider, sequencedExecutor, component.getBucketIdFactory()), _splitJoinHandler(_env, provider, bucketOwnershipNotifier, cfg.enableMultibitSplitOptimalization), _simpleHandler(_env, provider) { @@ -141,7 +141,7 @@ PersistenceHandler::processLockedMessage(FileStorHandler::LockedMessage lock) co // Important: we _copy_ the message shared_ptr instead of moving to ensure that `msg` remains // valid even if the tracker is destroyed by an exception in processMessage(). - auto tracker = std::make_unique(_env, _env._fileStorHandler, std::move(lock.first), lock.second); + auto tracker = std::make_unique(framework::MilliSecTimer(_clock), _env, _env._fileStorHandler, std::move(lock.first), lock.second); tracker = processMessage(msg, std::move(tracker)); if (tracker) { tracker->sendReply(); diff --git a/storage/src/vespa/storage/persistence/persistencehandler.h b/storage/src/vespa/storage/persistence/persistencehandler.h index 2cfac865484..ce9ee502fed 100644 --- a/storage/src/vespa/storage/persistence/persistencehandler.h +++ b/storage/src/vespa/storage/persistence/persistencehandler.h @@ -43,6 +43,7 @@ private: MessageTracker::UP processMessage(api::StorageMessage& msg, MessageTracker::UP tracker) const; + const framework::Clock & _clock; PersistenceUtil _env; ProcessAllHandler _processAllHandler; MergeHandler _mergeHandler; diff --git a/storage/src/vespa/storage/persistence/persistenceutil.cpp b/storage/src/vespa/storage/persistence/persistenceutil.cpp index 4ab3cdc5d01..90339108272 100644 --- a/storage/src/vespa/storage/persistence/persistenceutil.cpp +++ b/storage/src/vespa/storage/persistence/persistenceutil.cpp @@ -25,13 +25,15 @@ namespace { const vespalib::duration WARN_ON_SLOW_OPERATIONS = 5s; } -MessageTracker::MessageTracker(const PersistenceUtil & env, +MessageTracker::MessageTracker(const framework::MilliSecTimer & timer, + const PersistenceUtil & env, MessageSender & replySender, FileStorHandler::BucketLockInterface::SP bucketLock, api::StorageMessage::SP msg) - : MessageTracker(env, replySender, true, std::move(bucketLock), std::move(msg)) + : MessageTracker(timer, env, replySender, true, std::move(bucketLock), std::move(msg)) {} -MessageTracker::MessageTracker(const PersistenceUtil & env, +MessageTracker::MessageTracker(const framework::MilliSecTimer & timer, + const PersistenceUtil & env, MessageSender & replySender, bool updateBucketInfo, FileStorHandler::BucketLockInterface::SP bucketLock, @@ -45,12 +47,14 @@ MessageTracker::MessageTracker(const PersistenceUtil & env, _replySender(replySender), _metric(nullptr), _result(api::ReturnCode::OK), - _timer(env._component.getClock()) + _timer(timer) { } MessageTracker::UP -MessageTracker::createForTesting(PersistenceUtil &env, MessageSender &replySender, FileStorHandler::BucketLockInterface::SP bucketLock, api::StorageMessage::SP msg) { - return MessageTracker::UP(new MessageTracker(env, replySender, false, std::move(bucketLock), std::move(msg))); +MessageTracker::createForTesting(const framework::MilliSecTimer & timer, PersistenceUtil &env, MessageSender &replySender, + FileStorHandler::BucketLockInterface::SP bucketLock, api::StorageMessage::SP msg) +{ + return MessageTracker::UP(new MessageTracker(timer, env, replySender, false, std::move(bucketLock), std::move(msg))); } void @@ -161,9 +165,9 @@ PersistenceUtil::PersistenceUtil( spi::PersistenceProvider& provider) : _component(component), _fileStorHandler(fileStorHandler), - _nodeIndex(component.getIndex()), _metrics(metrics), - _bucketFactory(component.getBucketIdFactory()), + _nodeIndex(component.getIndex()), + _bucketIdFactory(component.getBucketIdFactory()), _spi(provider) { } @@ -191,12 +195,6 @@ PersistenceUtil::updateBucketDatabase(const document::Bucket &bucket, const api: } } -uint16_t -PersistenceUtil::getPreferredAvailableDisk(const document::Bucket &bucket) const -{ - return _component.getPreferredAvailablePartition(bucket); -} - PersistenceUtil::LockResult PersistenceUtil::lockAndGetDisk(const document::Bucket &bucket, StorBucketDatabase::Flag flags) @@ -275,19 +273,13 @@ PersistenceUtil::convertErrorCode(const spi::Result& response) return 0; } -void -PersistenceUtil::shutdown(const std::string& reason) -{ - _component.requestShutdown(reason); -} - spi::Bucket PersistenceUtil::getBucket(const document::DocumentId& id, const document::Bucket &bucket) const { - document::BucketId docBucket(_bucketFactory.getBucketId(id)); + document::BucketId docBucket(_bucketIdFactory.getBucketId(id)); docBucket.setUsedBits(bucket.getBucketId().getUsedBits()); if (bucket.getBucketId() != docBucket) { - docBucket = _bucketFactory.getBucketId(id); + docBucket = _bucketIdFactory.getBucketId(id); throw vespalib::IllegalStateException("Document " + id.toString() + " (bucket " + docBucket.toString() + ") does not belong in " + "bucket " + bucket.getBucketId().toString() + ".", VESPA_STRLOC); diff --git a/storage/src/vespa/storage/persistence/persistenceutil.h b/storage/src/vespa/storage/persistence/persistenceutil.h index ffce25b1e49..6a3d08a7e95 100644 --- a/storage/src/vespa/storage/persistence/persistenceutil.h +++ b/storage/src/vespa/storage/persistence/persistenceutil.h @@ -11,13 +11,13 @@ namespace storage { -struct PersistenceUtil; +class PersistenceUtil; class MessageTracker : protected Types { public: typedef std::unique_ptr UP; - MessageTracker(const PersistenceUtil & env, MessageSender & replySender, + MessageTracker(const framework::MilliSecTimer & timer, const PersistenceUtil & env, MessageSender & replySender, FileStorHandler::BucketLockInterface::SP bucketLock, api::StorageMessage::SP msg); ~MessageTracker(); @@ -70,11 +70,11 @@ public: bool checkForError(const spi::Result& response); static MessageTracker::UP - createForTesting(PersistenceUtil & env, MessageSender & replySender, + createForTesting(const framework::MilliSecTimer & timer, PersistenceUtil & env, MessageSender & replySender, FileStorHandler::BucketLockInterface::SP bucketLock, api::StorageMessage::SP msg); private: - MessageTracker(const PersistenceUtil & env, MessageSender & replySender, bool updateBucketInfo, + MessageTracker(const framework::MilliSecTimer & timer, const PersistenceUtil & env, MessageSender & replySender, bool updateBucketInfo, FileStorHandler::BucketLockInterface::SP bucketLock, api::StorageMessage::SP msg); [[nodiscard]] bool count_result_as_failure() const noexcept; @@ -92,13 +92,15 @@ private: framework::MilliSecTimer _timer; }; -struct PersistenceUtil { - ServiceLayerComponent &_component; - FileStorHandler &_fileStorHandler; - uint16_t _nodeIndex; - FileStorThreadMetrics &_metrics; // Needs a better solution for speed and thread safety - const document::BucketIdFactory &_bucketFactory; - spi::PersistenceProvider &_spi; +class PersistenceUtil { +public: + /** Lock the given bucket in the file stor handler. */ + struct LockResult { + std::shared_ptr lock; + LockResult() : lock() {} + + bool bucketExisted() const { return bool(lock); } + }; PersistenceUtil( ServiceLayerComponent&, @@ -111,34 +113,22 @@ struct PersistenceUtil { StorBucketDatabase& getBucketDatabase(document::BucketSpace bucketSpace) const { return _component.getBucketDatabase(bucketSpace); } - + spi::Bucket getBucket(const document::DocumentId& id, const document::Bucket &bucket) const; + void setBucketInfo(MessageTracker& tracker, const document::Bucket &bucket) const; void updateBucketDatabase(const document::Bucket &bucket, const api::BucketInfo& info) const; - - uint16_t getPreferredAvailableDisk(const document::Bucket &bucket) const; - - /** Lock the given bucket in the file stor handler. */ - struct LockResult { - std::shared_ptr lock; - LockResult() : lock() {} - - bool bucketExisted() const { return bool(lock); } - }; - - LockResult lockAndGetDisk( - const document::Bucket &bucket, - StorBucketDatabase::Flag flags = StorBucketDatabase::NONE); - + LockResult lockAndGetDisk(const document::Bucket &bucket, StorBucketDatabase::Flag flags = StorBucketDatabase::NONE); api::BucketInfo getBucketInfo(const document::Bucket &bucket) const; static api::BucketInfo convertBucketInfo(const spi::BucketInfo&); - - void setBucketInfo(MessageTracker& tracker, const document::Bucket &bucket) const; - - spi::Bucket getBucket(const document::DocumentId& id, const document::Bucket &bucket) const; - static uint32_t convertErrorCode(const spi::Result& response); - - void shutdown(const std::string& reason); +public: + ServiceLayerComponent &_component; + FileStorHandler &_fileStorHandler; + FileStorThreadMetrics &_metrics; // Needs a better solution for speed and thread safety + uint16_t _nodeIndex; +private: + const document::BucketIdFactory &_bucketIdFactory; + spi::PersistenceProvider &_spi; }; } // storage diff --git a/storage/src/vespa/storage/persistence/testandsethelper.cpp b/storage/src/vespa/storage/persistence/testandsethelper.cpp index fa0cfd7ea7e..77eddd3bd0d 100644 --- a/storage/src/vespa/storage/persistence/testandsethelper.cpp +++ b/storage/src/vespa/storage/persistence/testandsethelper.cpp @@ -24,8 +24,9 @@ void TestAndSetHelper::resolveDocumentType(const document::DocumentTypeRepo & do } } -void TestAndSetHelper::parseDocumentSelection(const document::DocumentTypeRepo & documentTypeRepo) { - document::select::Parser parser(documentTypeRepo, _env._bucketFactory); +void TestAndSetHelper::parseDocumentSelection(const document::DocumentTypeRepo & documentTypeRepo, + const document::BucketIdFactory & bucketIdFactory) { + document::select::Parser parser(documentTypeRepo, bucketIdFactory); try { _docSelectionUp = parser.parse(_cmd.getCondition().getSelection()); @@ -39,6 +40,7 @@ spi::GetResult TestAndSetHelper::retrieveDocument(const document::FieldSet & fie } TestAndSetHelper::TestAndSetHelper(const PersistenceUtil & env, const spi::PersistenceProvider & spi, + const document::BucketIdFactory & bucketFactory, const api::TestAndSetCommand & cmd, bool missingDocumentImpliesMatch) : _env(env), _spi(spi), @@ -49,7 +51,7 @@ TestAndSetHelper::TestAndSetHelper(const PersistenceUtil & env, const spi::Persi { const auto _repo = _env._component.getTypeRepo()->documentTypeRepo; resolveDocumentType(*_repo); - parseDocumentSelection(*_repo); + parseDocumentSelection(*_repo, bucketFactory); } TestAndSetHelper::~TestAndSetHelper() = default; diff --git a/storage/src/vespa/storage/persistence/testandsethelper.h b/storage/src/vespa/storage/persistence/testandsethelper.h index 44ecee88964..ca88f8b5337 100644 --- a/storage/src/vespa/storage/persistence/testandsethelper.h +++ b/storage/src/vespa/storage/persistence/testandsethelper.h @@ -8,7 +8,10 @@ #include namespace document::select { class Node; } -namespace document { class FieldSet; } +namespace document { + class FieldSet; + class BucketIdFactory; +} namespace storage { @@ -42,11 +45,13 @@ class TestAndSetHelper { bool _missingDocumentImpliesMatch; void resolveDocumentType(const document::DocumentTypeRepo & documentTypeRepo); - void parseDocumentSelection(const document::DocumentTypeRepo & documentTypeRepo); + void parseDocumentSelection(const document::DocumentTypeRepo & documentTypeRepo, + const document::BucketIdFactory & bucketIdFactory); spi::GetResult retrieveDocument(const document::FieldSet & fieldSet, spi::Context & context); public: TestAndSetHelper(const PersistenceUtil & env, const spi::PersistenceProvider & _spi, + const document::BucketIdFactory & bucketIdFactory, const api::TestAndSetCommand & cmd, bool missingDocumentImpliesMatch = false); ~TestAndSetHelper(); api::ReturnCode retrieveAndMatch(spi::Context & context); diff --git a/storage/src/vespa/storage/storageserver/servicelayernode.cpp b/storage/src/vespa/storage/storageserver/servicelayernode.cpp index e7ce6737ac8..1c8fdd4178d 100644 --- a/storage/src/vespa/storage/storageserver/servicelayernode.cpp +++ b/storage/src/vespa/storage/storageserver/servicelayernode.cpp @@ -88,7 +88,6 @@ ServiceLayerNode::subscribeToConfigs() << " disks but persistence provider states it has 1 disk."; throw vespalib::IllegalStateException(ost.str(), VESPA_STRLOC); } - _context.getComponentRegister().setDiskCount(1u); } void -- cgit v1.2.3