diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2020-04-27 13:02:42 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2020-04-27 13:26:32 +0000 |
commit | f1a0b4686d58a4b76fd8cbe6fca710a904432e8b (patch) | |
tree | 1770c4a23493aeae6e08b0daaf812b09203d7aa2 /storage | |
parent | e2356a51c4c64745d232c02ed142afd42efe5a93 (diff) |
Remove flush from provider interface.
Diffstat (limited to 'storage')
9 files changed, 10 insertions, 85 deletions
diff --git a/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp b/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp index 05e51993f49..fb80c25bfb7 100644 --- a/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp +++ b/storage/src/tests/persistence/common/persistenceproviderwrapper.cpp @@ -145,15 +145,6 @@ PersistenceProviderWrapper::get(const spi::Bucket& bucket, return _spi.get(bucket, fieldSet, id, context); } -spi::Result -PersistenceProviderWrapper::flush(const spi::Bucket& bucket, - spi::Context& context) -{ - LOG_SPI("flush(" << bucket << ")"); - CHECK_ERROR(spi::Result, FAIL_FLUSH); - return _spi.flush(bucket, context); -} - spi::CreateIteratorResult PersistenceProviderWrapper::createIterator(const spi::Bucket& bucket, const document::FieldSet& fields, diff --git a/storage/src/tests/persistence/common/persistenceproviderwrapper.h b/storage/src/tests/persistence/common/persistenceproviderwrapper.h index 511ced02118..9bd3653e8a1 100644 --- a/storage/src/tests/persistence/common/persistenceproviderwrapper.h +++ b/storage/src/tests/persistence/common/persistenceproviderwrapper.h @@ -36,7 +36,6 @@ public: FAIL_REPLACE_WITH_REMOVE = 1 << 6, FAIL_UPDATE = 1 << 7, FAIL_REVERT = 1 << 8, - FAIL_FLUSH = 1 << 9, FAIL_CREATE_ITERATOR = 1 << 10, FAIL_ITERATE = 1 << 11, FAIL_DESTROY_ITERATOR = 1 << 12, @@ -44,7 +43,7 @@ public: FAIL_SPLIT = 1 << 14, FAIL_JOIN = 1 << 15, FAIL_CREATE_BUCKET = 1 << 16, - FAIL_BUCKET_PERSISTENCE = FAIL_PUT|FAIL_REMOVE|FAIL_UPDATE|FAIL_REVERT|FAIL_FLUSH, + FAIL_BUCKET_PERSISTENCE = FAIL_PUT|FAIL_REMOVE|FAIL_UPDATE|FAIL_REVERT, FAIL_ALL_OPERATIONS = 0xffff, // TODO: add more as needed }; @@ -55,7 +54,7 @@ private: uint32_t _failureMask; public: PersistenceProviderWrapper(spi::PersistenceProvider& spi); - ~PersistenceProviderWrapper(); + ~PersistenceProviderWrapper() override; /** * Explicitly set result to anything != NONE to have all operations @@ -96,7 +95,6 @@ public: spi::GetResult get(const spi::Bucket&, const document::FieldSet&, const spi::DocumentId&, spi::Context&) const override ; - spi::Result flush(const spi::Bucket&, spi::Context&) override; spi::CreateIteratorResult createIterator(const spi::Bucket&, const document::FieldSet&, const spi::Selection&, spi::IncludedVersions versions, spi::Context&) override; diff --git a/storage/src/tests/persistence/mergehandlertest.cpp b/storage/src/tests/persistence/mergehandlertest.cpp index 0b2baab5652..7e7a76b95a8 100644 --- a/storage/src/tests/persistence/mergehandlertest.cpp +++ b/storage/src/tests/persistence/mergehandlertest.cpp @@ -626,8 +626,7 @@ MergeHandlerTest::createDummyGetBucketDiff(int timestampOffset, } TEST_F(MergeHandlerTest, spi_flush_guard) { - PersistenceProviderWrapper providerWrapper( - getPersistenceProvider()); + PersistenceProviderWrapper providerWrapper(getPersistenceProvider()); MergeHandler handler(providerWrapper, getEnv()); providerWrapper.setResult( @@ -635,8 +634,7 @@ TEST_F(MergeHandlerTest, spi_flush_guard) { setUpChain(MIDDLE); // Fail applying unrevertable remove - providerWrapper.setFailureMask( - PersistenceProviderWrapper::FAIL_REMOVE); + providerWrapper.setFailureMask(PersistenceProviderWrapper::FAIL_REMOVE); providerWrapper.clearOperationLog(); try { @@ -645,11 +643,6 @@ TEST_F(MergeHandlerTest, spi_flush_guard) { } catch (const std::runtime_error& e) { EXPECT_TRUE(std::string(e.what()).find("Failed remove") != std::string::npos); } - // Test that we always flush after applying diff locally, even when - // errors are encountered. - const std::vector<std::string>& opLog(providerWrapper.getOperationLog()); - ASSERT_FALSE(opLog.empty()); - EXPECT_EQ("flush(Bucket(0x40000000000004d2, partition 0))", opLog.back()); } TEST_F(MergeHandlerTest, bucket_not_found_in_db) { @@ -901,7 +894,6 @@ TEST_F(MergeHandlerTest, apply_bucket_diff_spi_failures) { { PersistenceProviderWrapper::FAIL_ITERATE, "iterate" }, { PersistenceProviderWrapper::FAIL_PUT, "Failed put" }, { PersistenceProviderWrapper::FAIL_REMOVE, "Failed remove" }, - { PersistenceProviderWrapper::FAIL_FLUSH, "Failed flush" }, }; typedef ExpectedExceptionSpec* ExceptionIterator; @@ -1058,7 +1050,6 @@ TEST_F(MergeHandlerTest, apply_bucket_diff_reply_spi_failures) { { PersistenceProviderWrapper::FAIL_ITERATE, "iterate" }, { PersistenceProviderWrapper::FAIL_PUT, "Failed put" }, { PersistenceProviderWrapper::FAIL_REMOVE, "Failed remove" }, - { PersistenceProviderWrapper::FAIL_FLUSH, "Failed flush" }, }; typedef ExpectedExceptionSpec* ExceptionIterator; diff --git a/storage/src/tests/persistence/persistencetestutils.cpp b/storage/src/tests/persistence/persistencetestutils.cpp index e32fc056413..25c0a36a7f5 100644 --- a/storage/src/tests/persistence/persistencetestutils.cpp +++ b/storage/src/tests/persistence/persistencetestutils.cpp @@ -159,7 +159,6 @@ PersistenceTestUtils::doPutOnDisk( getPersistenceProvider().put(spi::Bucket(b), timestamp, doc, context); - getPersistenceProvider().flush(b, context); return doc; } diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp index e5e358cbb60..d47ed28f636 100644 --- a/storage/src/vespa/storage/persistence/mergehandler.cpp +++ b/storage/src/vespa/storage/persistence/mergehandler.cpp @@ -91,40 +91,6 @@ public: } }; -class FlushGuard -{ - spi::PersistenceProvider& _spi; - spi::Bucket _bucket; - spi::Context& _context; - bool _hasFlushed; -public: - FlushGuard(spi::PersistenceProvider& spi, - const spi::Bucket& bucket, - spi::Context& context) - : _spi(spi), - _bucket(bucket), - _context(context), - _hasFlushed(false) - {} - ~FlushGuard() - { - if (!_hasFlushed) { - LOG(debug, "Auto-flushing %s", _bucket.toString().c_str()); - spi::Result result =_spi.flush(_bucket, _context); - if (result.hasError()) { - LOG(debug, "Flush %s failed: %s", - _bucket.toString().c_str(), - result.toString().c_str()); - } - } - } - void flush() { - LOG(debug, "Flushing %s", _bucket.toString().c_str()); - _hasFlushed = true; - checkResult(_spi.flush(_bucket, _context), _bucket, "flush"); - } -}; - struct IndirectDocEntryTimestampPredicate { bool operator()(const spi::DocEntry::UP& e1, @@ -607,8 +573,6 @@ MergeHandler::applyDiffLocally( std::vector<spi::DocEntry::UP> entries; populateMetaData(bucket, MAX_TIMESTAMP, entries, context); - FlushGuard flushGuard(_spi, bucket, context); - std::shared_ptr<const document::DocumentTypeRepo> repo(_env._component.getTypeRepo()); assert(repo.get() != nullptr); @@ -702,8 +666,6 @@ MergeHandler::applyDiffLocally( LOG(debug, "Merge(%s): Applied %u entries locally from ApplyBucketDiff.", bucket.toString().c_str(), addedCount); - flushGuard.flush(); - spi::BucketInfoResult infoResult(_spi.getBucketInfo(bucket)); if (infoResult.getErrorCode() != spi::Result::ErrorType::NONE) { LOG(warning, "Failed to get bucket info for %s: %s", diff --git a/storage/src/vespa/storage/persistence/persistencethread.cpp b/storage/src/vespa/storage/persistence/persistencethread.cpp index d1da25269cf..aaf62a85e87 100644 --- a/storage/src/vespa/storage/persistence/persistencethread.cpp +++ b/storage/src/vespa/storage/persistence/persistencethread.cpp @@ -607,10 +607,6 @@ PersistenceThread::handleJoinBuckets(api::JoinBucketsCommand& cmd, spi::Context if (!checkForError(result, *tracker)) { return tracker; } - result = _spi.flush(spi::Bucket(destBucket, spi::PartitionId(_env._partition)), context); - if (!checkForError(result, *tracker)) { - return tracker; - } uint64_t lastModified = 0; for (uint32_t i = 0; i < cmd.getSourceBuckets().size(); i++) { document::Bucket srcBucket(destBucket.getBucketSpace(), cmd.getSourceBuckets()[i]); diff --git a/storage/src/vespa/storage/persistence/processallhandler.cpp b/storage/src/vespa/storage/persistence/processallhandler.cpp index 5b94a3da027..f37c6723933 100644 --- a/storage/src/vespa/storage/persistence/processallhandler.cpp +++ b/storage/src/vespa/storage/persistence/processallhandler.cpp @@ -83,9 +83,9 @@ MessageTracker::UP ProcessAllHandler::handleRemoveLocation(api::RemoveLocationCommand& cmd, spi::Context& context) { - MessageTracker::UP tracker(new MessageTracker( + auto tracker = std::make_unique<MessageTracker>( _env._metrics.removeLocation[cmd.getLoadType()], - _env._component.getClock())); + _env._component.getClock()); LOG(debug, "RemoveLocation(%s): using selection '%s'", cmd.getBucketId().toString().c_str(), @@ -99,13 +99,8 @@ ProcessAllHandler::handleRemoveLocation(api::RemoveLocationCommand& cmd, processor, spi::NEWEST_DOCUMENT_ONLY, context); - spi::Result result = _spi.flush(bucket, context); - uint32_t code = _env.convertErrorCode(result); - if (code == 0) { - tracker->setReply(std::make_shared<api::RemoveLocationReply>(cmd, processor._n_removed)); - } else { - tracker->fail(code, result.getErrorMessage()); - } + + tracker->setReply(std::make_shared<api::RemoveLocationReply>(cmd, processor._n_removed)); return tracker; } @@ -114,9 +109,9 @@ MessageTracker::UP ProcessAllHandler::handleStatBucket(api::StatBucketCommand& cmd, spi::Context& context) { - MessageTracker::UP tracker(new MessageTracker( + auto tracker = std::make_unique<MessageTracker>( _env._metrics.statBucket[cmd.getLoadType()], - _env._component.getClock())); + _env._component.getClock()); std::ostringstream ost; ost << "Persistence bucket " << cmd.getBucketId() diff --git a/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp b/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp index fe947bff4d5..76c420e76e6 100644 --- a/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp +++ b/storage/src/vespa/storage/persistence/provider_error_wrapper.cpp @@ -118,12 +118,6 @@ ProviderErrorWrapper::get(const spi::Bucket& bucket, return checkResult(_impl.get(bucket, fieldSet, docId, context)); } -spi::Result -ProviderErrorWrapper::flush(const spi::Bucket& bucket, spi::Context& context) -{ - return checkResult(_impl.flush(bucket, context)); -} - spi::CreateIteratorResult ProviderErrorWrapper::createIterator(const spi::Bucket& bucket, const document::FieldSet& fieldSet, diff --git a/storage/src/vespa/storage/persistence/provider_error_wrapper.h b/storage/src/vespa/storage/persistence/provider_error_wrapper.h index 3b5ace90d13..292eb004223 100644 --- a/storage/src/vespa/storage/persistence/provider_error_wrapper.h +++ b/storage/src/vespa/storage/persistence/provider_error_wrapper.h @@ -52,7 +52,6 @@ public: spi::RemoveResult removeIfFound(const spi::Bucket&, spi::Timestamp, const document::DocumentId&, spi::Context&) override; spi::UpdateResult update(const spi::Bucket&, spi::Timestamp, const spi::DocumentUpdateSP&, spi::Context&) override; spi::GetResult get(const spi::Bucket&, const document::FieldSet&, const document::DocumentId&, spi::Context&) const override; - spi::Result flush(const spi::Bucket&, spi::Context&) override; spi::CreateIteratorResult createIterator(const spi::Bucket&, const document::FieldSet&, const spi::Selection&, spi::IncludedVersions versions, spi::Context&) override; spi::IterateResult iterate(spi::IteratorId, uint64_t maxByteSize, spi::Context&) const override; |