Remove flush from provider interface.

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;