From bbc768d6ef92f166717d65e67689f6aa15c98952 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Fri, 14 Aug 2020 16:08:20 +0000 Subject: Change api so that we can drop clone from the interface and prepare for using a true repo. --- storage/src/tests/common/teststorageapp.cpp | 8 +++----- storage/src/tests/common/teststorageapp.h | 5 +++-- .../persistence/common/persistenceproviderwrapper.cpp | 6 ++---- .../persistence/common/persistenceproviderwrapper.h | 5 +++-- .../persistence/filestorage/mergeblockingtest.cpp | 4 +--- .../persistence/filestorage/operationabortingtest.cpp | 15 ++++++++------- .../src/vespa/storage/persistence/bucketprocessor.cpp | 10 +++++----- storage/src/vespa/storage/persistence/mergehandler.cpp | 18 ++++++++---------- .../vespa/storage/persistence/persistencethread.cpp | 6 +++--- .../storage/persistence/provider_error_wrapper.cpp | 4 ++-- .../vespa/storage/persistence/provider_error_wrapper.h | 5 +++-- 11 files changed, 41 insertions(+), 45 deletions(-) (limited to 'storage/src') diff --git a/storage/src/tests/common/teststorageapp.cpp b/storage/src/tests/common/teststorageapp.cpp index 9fcf1049e1b..1847de0e84f 100644 --- a/storage/src/tests/common/teststorageapp.cpp +++ b/storage/src/tests/common/teststorageapp.cpp @@ -171,19 +171,17 @@ TestServiceLayerApp::TestServiceLayerApp(DiskCount dc, NodeIndex index, assert(dc > 0); } -TestServiceLayerApp::~TestServiceLayerApp() {} +TestServiceLayerApp::~TestServiceLayerApp() = default; void TestServiceLayerApp::setupDummyPersistence() { - spi::PersistenceProvider::UP provider(new spi::dummy::DummyPersistence( - getTypeRepo(), _compReg.getDiskCount())); + auto provider = std::make_unique(getTypeRepo(), _compReg.getDiskCount()); setPersistenceProvider(std::move(provider)); } void -TestServiceLayerApp::setPersistenceProvider( - spi::PersistenceProvider::UP provider) +TestServiceLayerApp::setPersistenceProvider(PersistenceProviderUP provider) { _partitions = provider->getPartitionStates().getList(); assert(spi::PartitionId(_compReg.getDiskCount()) == _partitions.size()); diff --git a/storage/src/tests/common/teststorageapp.h b/storage/src/tests/common/teststorageapp.h index e567206c371..218e7352f04 100644 --- a/storage/src/tests/common/teststorageapp.h +++ b/storage/src/tests/common/teststorageapp.h @@ -107,8 +107,9 @@ private: class TestServiceLayerApp : public TestStorageApp { + using PersistenceProviderUP = std::unique_ptr; ServiceLayerComponentRegisterImpl& _compReg; - spi::PersistenceProvider::UP _persistenceProvider; + PersistenceProviderUP _persistenceProvider; spi::PartitionStateList _partitions; public: @@ -118,7 +119,7 @@ public: ~TestServiceLayerApp(); void setupDummyPersistence(); - void setPersistenceProvider(spi::PersistenceProvider::UP); + void setPersistenceProvider(PersistenceProviderUP); ServiceLayerComponentRegisterImpl& getComponentRegister() { return _compReg; } diff --git a/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp b/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp index dd9ce6e6cba..67a1c41a9ef 100644 --- a/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp +++ b/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp @@ -147,11 +147,9 @@ PersistenceProviderWrapper::get(const spi::Bucket& bucket, } spi::CreateIteratorResult -PersistenceProviderWrapper::createIterator(const spi::Bucket& bucket, - const document::FieldSet& fields, - const spi::Selection& sel, +PersistenceProviderWrapper::createIterator(const spi::Bucket &bucket, FieldSetSP fields, const spi::Selection &sel, spi::IncludedVersions versions, - spi::Context& context) + spi::Context &context) { // TODO: proper printing of FieldSet and Selection diff --git a/storage/src/tests/persistence/common/persistenceproviderwrapper.h b/storage/src/tests/persistence/common/persistenceproviderwrapper.h index 21e5d8016aa..75712750d68 100644 --- a/storage/src/tests/persistence/common/persistenceproviderwrapper.h +++ b/storage/src/tests/persistence/common/persistenceproviderwrapper.h @@ -100,8 +100,9 @@ public: spi::UpdateResult update(const spi::Bucket&, spi::Timestamp, spi::DocumentUpdateSP, spi::Context&) override; spi::GetResult get(const spi::Bucket&, const document::FieldSet&, const spi::DocumentId&, spi::Context&) const override; - spi::CreateIteratorResult createIterator(const spi::Bucket&, const document::FieldSet&, const spi::Selection&, - spi::IncludedVersions versions, spi::Context&) override; + spi::CreateIteratorResult + createIterator(const spi::Bucket &bucket, FieldSetSP, const spi::Selection &, spi::IncludedVersions versions, + spi::Context &context) override; spi::IterateResult iterate(spi::IteratorId, uint64_t maxByteSize, spi::Context&) const override; spi::Result destroyIterator(spi::IteratorId, spi::Context&) override; diff --git a/storage/src/tests/persistence/filestorage/mergeblockingtest.cpp b/storage/src/tests/persistence/filestorage/mergeblockingtest.cpp index d9582cec585..c73ae7e506c 100644 --- a/storage/src/tests/persistence/filestorage/mergeblockingtest.cpp +++ b/storage/src/tests/persistence/filestorage/mergeblockingtest.cpp @@ -15,9 +15,7 @@ namespace storage { struct MergeBlockingTest : public FileStorTestFixture { void setupDisks() { FileStorTestFixture::setupPersistenceThreads(1); - _node->setPersistenceProvider( - spi::PersistenceProvider::UP( - new spi::dummy::DummyPersistence(_node->getTypeRepo(), 1))); + _node->setPersistenceProvider(std::make_unique(_node->getTypeRepo(), 1)); } void SetUp() override; diff --git a/storage/src/tests/persistence/filestorage/operationabortingtest.cpp b/storage/src/tests/persistence/filestorage/operationabortingtest.cpp index 93c484368de..7810a595012 100644 --- a/storage/src/tests/persistence/filestorage/operationabortingtest.cpp +++ b/storage/src/tests/persistence/filestorage/operationabortingtest.cpp @@ -77,18 +77,19 @@ spi::LoadType defaultLoadType(0, "default"); } struct OperationAbortingTest : FileStorTestFixture { - spi::PersistenceProvider::UP _dummyProvider; - BlockingMockProvider* _blockingProvider; + std::unique_ptr _dummyProvider; + BlockingMockProvider * _blockingProvider; std::unique_ptr _queueBarrier; std::unique_ptr _completionBarrier; void setupProviderAndBarriers(uint32_t queueBarrierThreads) { FileStorTestFixture::setupPersistenceThreads(1); - _dummyProvider.reset(new spi::dummy::DummyPersistence(_node->getTypeRepo(), 1)); - _queueBarrier.reset(new vespalib::Barrier(queueBarrierThreads)); - _completionBarrier.reset(new vespalib::Barrier(2)); - _blockingProvider = new BlockingMockProvider(*_dummyProvider, *_queueBarrier, *_completionBarrier); - _node->setPersistenceProvider(spi::PersistenceProvider::UP(_blockingProvider)); + _dummyProvider = std::make_unique(_node->getTypeRepo(), 1); + _queueBarrier = std::make_unique(queueBarrierThreads); + _completionBarrier = std::make_unique(2); + auto blockingProvider = std::make_unique(*_dummyProvider, *_queueBarrier, *_completionBarrier); + _blockingProvider = blockingProvider.get(); + _node->setPersistenceProvider(std::move(blockingProvider)); } void validateReplies(DummyStorageLink& link, size_t repliesTotal, diff --git a/storage/src/vespa/storage/persistence/bucketprocessor.cpp b/storage/src/vespa/storage/persistence/bucketprocessor.cpp index c88b08612d7..ea09fcfc348 100644 --- a/storage/src/vespa/storage/persistence/bucketprocessor.cpp +++ b/storage/src/vespa/storage/persistence/bucketprocessor.cpp @@ -47,11 +47,11 @@ BucketProcessor::iterateAll(spi::PersistenceProvider& provider, spi::Selection sel = spi::Selection(spi::DocumentSelection(documentSelection)); spi::CreateIteratorResult createIterResult(provider.createIterator( - bucket, - document::AllFields(), - sel, - versions, - context)); + bucket, + std::make_shared(), + sel, + versions, + context)); if (createIterResult.getErrorCode() != spi::Result::ErrorType::NONE) { vespalib::asciistream ss; diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp index 70894858887..b275150fe37 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.cpp +++ b/storage/src/vespa/storage/persistence/mergehandler.cpp @@ -124,11 +124,11 @@ MergeHandler::populateMetaData( spi::Selection sel(docSel); sel.setToTimestamp(spi::Timestamp(maxTimestamp.getTime())); spi::CreateIteratorResult createIterResult(_spi.createIterator( - bucket, - document::NoFields(), - sel, - spi::ALL_VERSIONS, - context)); + bucket, + std::make_shared(), + sel, + spi::ALL_VERSIONS, + context)); if (createIterResult.getErrorCode() != spi::Result::ErrorType::NONE) { std::ostringstream ss; @@ -376,8 +376,7 @@ MergeHandler::fetchLocalData( "remaining size to fill is %u", bucket.toString().c_str(), alreadyFilled, _maxChunkSize, remainingSize); if (remainingSize == 0) { - LOG(debug, - "Diff already at max chunk size, not fetching any local data"); + LOG(debug, "Diff already at max chunk size, not fetching any local data"); return; } @@ -387,7 +386,7 @@ MergeHandler::fetchLocalData( sel.setTimestampSubset(slots); spi::CreateIteratorResult createIterResult( _spi.createIterator(bucket, - document::AllFields(), + std::make_shared(), sel, spi::NEWEST_DOCUMENT_OR_REMOVE, context)); @@ -409,8 +408,7 @@ MergeHandler::fetchLocalData( bool fetchedAllLocalData = false; bool chunkLimitReached = false; while (true) { - spi::IterateResult result( - _spi.iterate(iteratorId, remainingSize, context)); + spi::IterateResult result(_spi.iterate(iteratorId, remainingSize, context)); if (result.getErrorCode() != spi::Result::ErrorType::NONE) { std::ostringstream ss; ss << "Failed to iterate for " diff --git a/storage/src/vespa/storage/persistence/persistencethread.cpp b/storage/src/vespa/storage/persistence/persistencethread.cpp index 53e455ea204..a9b13bab0f6 100644 --- a/storage/src/vespa/storage/persistence/persistencethread.cpp +++ b/storage/src/vespa/storage/persistence/persistencethread.cpp @@ -455,11 +455,11 @@ MessageTracker::UP PersistenceThread::handleCreateIterator(CreateIteratorCommand& cmd, MessageTracker::UP tracker) { tracker->setMetric(_env._metrics.createIterator); - document::FieldSet::UP fieldSet = document::FieldSetRepo::parse(*_env._component.getTypeRepo(), cmd.getFields()); + document::FieldSet::SP fieldSet = document::FieldSetRepo::parse(*_env._component.getTypeRepo(), cmd.getFields()); tracker->context().setReadConsistency(cmd.getReadConsistency()); spi::CreateIteratorResult result(_spi.createIterator( - spi::Bucket(cmd.getBucket(), spi::PartitionId(_env._partition)), - *fieldSet, cmd.getSelection(), cmd.getIncludedVersions(), tracker->context())); + spi::Bucket(cmd.getBucket(), spi::PartitionId(_env._partition)), + std::move(fieldSet), cmd.getSelection(), cmd.getIncludedVersions(), tracker->context())); if (tracker->checkForError(result)) { tracker->setReply(std::make_shared(cmd, spi::IteratorId(result.getIteratorId()))); } diff --git a/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp b/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp index a5564282d17..0884d807eda 100644 --- a/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp +++ b/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp @@ -111,8 +111,8 @@ ProviderErrorWrapper::get(const spi::Bucket& bucket, const document::FieldSet& f } spi::CreateIteratorResult -ProviderErrorWrapper::createIterator(const spi::Bucket& bucket, const document::FieldSet& fieldSet, - const spi::Selection& selection, spi::IncludedVersions versions, spi::Context& context) +ProviderErrorWrapper::createIterator(const spi::Bucket &bucket, FieldSetSP fieldSet, const spi::Selection &selection, + spi::IncludedVersions versions, spi::Context &context) { return checkResult(_impl.createIterator(bucket, fieldSet, selection, versions, context)); } diff --git a/storage/src/vespa/storage/persistence/provider_error_wrapper.h b/storage/src/vespa/storage/persistence/provider_error_wrapper.h index 602877e0b02..54abf0e96fb 100644 --- a/storage/src/vespa/storage/persistence/provider_error_wrapper.h +++ b/storage/src/vespa/storage/persistence/provider_error_wrapper.h @@ -52,8 +52,9 @@ public: spi::RemoveResult removeIfFound(const spi::Bucket&, spi::Timestamp, const document::DocumentId&, spi::Context&) override; spi::UpdateResult update(const spi::Bucket&, spi::Timestamp, spi::DocumentUpdateSP, spi::Context&) override; spi::GetResult get(const spi::Bucket&, const document::FieldSet&, const document::DocumentId&, spi::Context&) const override; - spi::CreateIteratorResult createIterator(const spi::Bucket&, const document::FieldSet&, const spi::Selection&, - spi::IncludedVersions versions, spi::Context&) override; + spi::CreateIteratorResult + createIterator(const spi::Bucket &bucket, FieldSetSP, const spi::Selection &, spi::IncludedVersions versions, + spi::Context &context) override; spi::IterateResult iterate(spi::IteratorId, uint64_t maxByteSize, spi::Context&) const override; spi::Result destroyIterator(spi::IteratorId, spi::Context&) override; spi::Result createBucket(const spi::Bucket&, spi::Context&) override; -- cgit v1.2.3