Only keep async variant to simplify what to implement and what fallback there are.

6 files changed, 63 insertions, 130 deletions

diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
index 7793467040e..6e4f38fe564 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
@@ -15,7 +15,6 @@
 #include <vespa/vespalib/util/exceptions.h>
 #include <vespa/vespalib/util/idestructorcallback.h>
 #include <vespa/vespalib/stllike/hash_map.hpp>
-#include <algorithm>
 #include <cassert>
 
 #include <vespa/log/log.h>
@@ -74,11 +73,9 @@ BucketContent::getBucketInfo() const
     uint32_t totalSize = 0;
     uint32_t checksum = 0;
 
-    for (std::vector<BucketEntry>::const_iterator
-             it = _entries.begin(); it != _entries.end(); ++it)
-    {
-        const DocEntry& entry(*it->entry);
-        const GlobalId& gid(it->gid);
+    for (const BucketEntry & bucketEntry : _entries) {
+        const DocEntry& entry(*bucketEntry.entry);
+        const GlobalId& gid(bucketEntry.gid);
 
         GidMapType::const_iterator gidIt(_gidMap.find(gid));
         assert(gidIt != _gidMap.end());
@@ -94,7 +91,7 @@ BucketContent::getBucketInfo() const
         ++unique;
         uniqueSize += entry.getSize();
 
-        checksum ^= computeEntryChecksum(*it);
+        checksum ^= computeEntryChecksum(bucketEntry);
     }
     if (!unique) {
         checksum = 0;
@@ -115,12 +112,6 @@ BucketContent::getBucketInfo() const
 }
 
 namespace {
-struct HasDocId {
-    const DocumentId &_did;
-    HasDocId(const DocumentId &did) : _did(did) {}
-    bool operator()(const DocEntry &entry)
-    { return *entry.getDocumentId() == _did; }
-};
 
 struct TimestampLess {
     bool operator()(const BucketEntry &bucketEntry, Timestamp t)
@@ -129,15 +120,6 @@ struct TimestampLess {
     { return t < bucketEntry.entry->getTimestamp(); }
 };
 
-template <typename Iter>
-typename std::iterator_traits<Iter>::value_type
-dereferenceOrDefaultIfAtEnd(Iter it, Iter end) {
-    if (it == end) {
-        return typename std::iterator_traits<Iter>::value_type();
-    }
-    return *it;
-}
-
 }  // namespace
 
 bool
@@ -442,51 +424,52 @@ DummyPersistence::getBucketInfo(const Bucket& b) const
     return BucketInfoResult(info);
 }
 
-Result
-DummyPersistence::put(const Bucket& b, Timestamp t, Document::SP doc, Context&)
+void
+DummyPersistence::putAsync(const Bucket& b, Timestamp t, Document::SP doc, Context&, OperationComplete::UP onComplete)
 {
     DUMMYPERSISTENCE_VERIFY_INITIALIZED;
     LOG(debug, "put(%s, %" PRIu64 ", %s)",
-        b.toString().c_str(),
-        uint64_t(t),
-        doc->getId().toString().c_str());
+        b.toString().c_str(), uint64_t(t), doc->getId().toString().c_str());
     assert(b.getBucketSpace() == FixedBucketSpaces::default_space());
     BucketContentGuard::UP bc(acquireBucketWithLock(b));
     if (!bc.get()) {
-        return BucketInfoResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
-    }
-
-    DocEntry::SP existing = (*bc)->getEntry(t);
-    if (existing.get()) {
-        if (doc->getId() == *existing->getDocumentId()) {
-            return Result();
+        bc.reset();
+        onComplete->onComplete(std::make_unique<BucketInfoResult>(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found"));
+    } else {
+        DocEntry::SP existing = (*bc)->getEntry(t);
+        if (existing) {
+            bc.reset();
+            if (doc->getId() == *existing->getDocumentId()) {
+                onComplete->onComplete(std::make_unique<Result>());
+            } else {
+                onComplete->onComplete(std::make_unique<Result>(Result::ErrorType::TIMESTAMP_EXISTS,
+                                                                "Timestamp already existed"));
+            }
         } else {
-            return Result(Result::ErrorType::TIMESTAMP_EXISTS,
-                          "Timestamp already existed");
+            LOG(spam, "Inserting document %s", doc->toString(true).c_str());
+            auto entry = std::make_unique<DocEntry>(t, NONE, Document::UP(doc->clone()));
+            (*bc)->insert(std::move(entry));
+            bc.reset();
+            onComplete->onComplete(std::make_unique<Result>());
         }
     }
-
-    LOG(spam, "Inserting document %s", doc->toString(true).c_str());
-
-    auto entry = std::make_unique<DocEntry>(t, NONE, Document::UP(doc->clone()));
-    (*bc)->insert(std::move(entry));
-    return Result();
 }
 
-UpdateResult
-DummyPersistence::update(const Bucket& bucket, Timestamp ts, DocumentUpdateSP upd, Context& context)
+void
+DummyPersistence::updateAsync(const Bucket& bucket, Timestamp ts, DocumentUpdateSP upd, Context& context, OperationComplete::UP onComplete)
 {
     GetResult getResult = get(bucket, document::AllFields(), upd->getId(), context);
 
     if (getResult.hasError()) {
-        return UpdateResult(getResult.getErrorCode(), getResult.getErrorMessage());
+        onComplete->onComplete(std::make_unique<UpdateResult>(getResult.getErrorCode(), getResult.getErrorMessage()));
+        return;
     }
-
     auto docToUpdate = getResult.getDocumentPtr();
     Timestamp updatedTs = getResult.getTimestamp();
     if (!docToUpdate) {
         if (!upd->getCreateIfNonExistent()) {
-            return UpdateResult();
+            onComplete->onComplete(std::make_unique<UpdateResult>());
+            return;
         } else {
             docToUpdate = std::make_shared<document::Document>(upd->getType(), upd->getId());
             updatedTs = ts;
@@ -498,14 +481,14 @@ DummyPersistence::update(const Bucket& bucket, Timestamp ts, DocumentUpdateSP up
     Result putResult = put(bucket, ts, std::move(docToUpdate), context);
 
     if (putResult.hasError()) {
-        return UpdateResult(putResult.getErrorCode(), putResult.getErrorMessage());
+        onComplete->onComplete(std::make_unique<UpdateResult>(putResult.getErrorCode(), putResult.getErrorMessage()));
+    } else {
+        onComplete->onComplete(std::make_unique<UpdateResult>(updatedTs));
     }
-
-    return UpdateResult(updatedTs);
 }
 
-RemoveResult
-DummyPersistence::remove(const Bucket& b, Timestamp t, const DocumentId& did, Context&)
+void
+DummyPersistence::removeAsync(const Bucket& b, Timestamp t, const DocumentId& did, Context &, OperationComplete::UP onComplete)
 {
     DUMMYPERSISTENCE_VERIFY_INITIALIZED;
     LOG(debug, "remove(%s, %" PRIu64 ", %s)",
@@ -515,19 +498,21 @@ DummyPersistence::remove(const Bucket& b, Timestamp t, const DocumentId& did, Co
     assert(b.getBucketSpace() == FixedBucketSpaces::default_space());
 
     BucketContentGuard::UP bc(acquireBucketWithLock(b));
-    if (!bc.get()) {
-        return RemoveResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
-    }
-
-    DocEntry::SP entry((*bc)->getEntry(did));
-    bool foundPut(entry.get() && !entry->isRemove());
-    DocEntry::UP remEntry(new DocEntry(t, REMOVE_ENTRY, did));
+    if ( ! bc ) {
+        bc.reset();
+        onComplete->onComplete(std::make_unique<RemoveResult>(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found"));
+    } else {
+        DocEntry::SP entry((*bc)->getEntry(did));
+        bool foundPut(entry.get() && !entry->isRemove());
+        auto remEntry = std::make_unique<DocEntry>(t, REMOVE_ENTRY, did);
 
-    if ((*bc)->hasTimestamp(t)) {
-        (*bc)->eraseEntry(t);
+        if ((*bc)->hasTimestamp(t)) {
+            (*bc)->eraseEntry(t);
+        }
+        (*bc)->insert(std::move(remEntry));
+        bc.reset();
+        onComplete->onComplete(std::make_unique<RemoveResult>(foundPut));
     }
-    (*bc)->insert(std::move(remEntry));
-    return RemoveResult(foundPut);
 }
 
 GetResult
diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
index a9b611d131c..99d6ba717b7 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
@@ -157,10 +157,10 @@ public:
     Result setClusterState(BucketSpace bucketSpace, const ClusterState& newState) override;
     void setActiveStateAsync(const Bucket&, BucketInfo::ActiveState, OperationComplete::UP) override;
     BucketInfoResult getBucketInfo(const Bucket&) const override;
-    Result put(const Bucket&, Timestamp, DocumentSP, Context&) override;
     GetResult get(const Bucket&, const document::FieldSet&, const DocumentId&, Context&) const override;
-    RemoveResult remove(const Bucket& b, Timestamp t, const DocumentId& did, Context&) override;
-    UpdateResult update(const Bucket&, Timestamp, DocumentUpdateSP, Context&) override;
+    void putAsync(const Bucket&, Timestamp, DocumentSP, Context&, OperationComplete::UP) override;
+    void removeAsync(const Bucket& b, Timestamp t, const DocumentId& did, Context&, OperationComplete::UP) override;
+    void updateAsync(const Bucket&, Timestamp, DocumentUpdateSP, Context&, OperationComplete::UP) override;
 
     CreateIteratorResult
     createIterator(const Bucket &bucket, FieldSetSP fs, const Selection &, IncludedVersions, Context &context) override;
diff --git a/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp b/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp
index 6be0941d731..951dbf97cff 100644
--- a/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp
+++ b/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp
@@ -9,13 +9,6 @@
 
 namespace storage::spi {
 
-RemoveResult
-AbstractPersistenceProvider::removeIfFound(const Bucket& b, Timestamp timestamp,
-                                           const DocumentId& id, Context& context)
-{
-    return remove(b, timestamp, id, context);
-}
-
 void
 AbstractPersistenceProvider::removeIfFoundAsync(const Bucket& b, Timestamp timestamp,
                                                 const DocumentId& id, Context& context, OperationComplete::UP onComplete)
@@ -30,14 +23,4 @@ AbstractPersistenceProvider::getModifiedBuckets(BucketSpace) const
     return BucketIdListResult(list);
 }
 
-void
-AbstractPersistenceProvider::setActiveStateAsync(const Bucket &, BucketInfo::ActiveState, OperationComplete::UP op) {
-    op->onComplete(std::make_unique<Result>());
-}
-
-void
-AbstractPersistenceProvider::deleteBucketAsync(const Bucket &, Context &, OperationComplete::UP op) {
-    op->onComplete(std::make_unique<Result>());
-}
-
 }
diff --git a/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.h b/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.h
index 472abeca161..e287bdc5252 100644
--- a/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.h
+++ b/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.h
@@ -17,11 +17,8 @@ public:
     Result initialize() override { return Result(); };
     Result createBucket(const Bucket&, Context&) override { return Result(); }
     Result removeEntry(const Bucket&, Timestamp, Context&) override { return Result(); }
-    RemoveResult removeIfFound(const Bucket&, Timestamp, const DocumentId&, Context&) override;
     void removeIfFoundAsync(const Bucket&, Timestamp, const DocumentId&, Context&, OperationComplete::UP) override;
     Result setClusterState(BucketSpace, const ClusterState&) override { return Result(); }
-    void setActiveStateAsync(const Bucket &, BucketInfo::ActiveState, OperationComplete::UP ) override;
-    void deleteBucketAsync(const Bucket&, Context&, OperationComplete::UP) override;
     BucketIdListResult getModifiedBuckets(BucketSpace bucketSpace) const override;
 };
 
diff --git a/persistence/src/vespa/persistence/spi/persistenceprovider.cpp b/persistence/src/vespa/persistence/spi/persistenceprovider.cpp
index 7da2ee58aa9..3ea476c33fc 100644
--- a/persistence/src/vespa/persistence/spi/persistenceprovider.cpp
+++ b/persistence/src/vespa/persistence/spi/persistenceprovider.cpp
@@ -32,14 +32,6 @@ PersistenceProvider::put(const Bucket& bucket, Timestamp timestamp, DocumentSP d
     return *future.get();
 }
 
-void
-PersistenceProvider::putAsync(const Bucket &bucket, Timestamp timestamp, DocumentSP doc, Context &context,
-                              OperationComplete::UP onComplete)
-{
-    Result result = put(bucket, timestamp, std::move(doc), context);
-    onComplete->onComplete(std::make_unique<Result>(result));
-}
-
 RemoveResult
 PersistenceProvider::remove(const Bucket& bucket, Timestamp timestamp, const DocumentId & docId, Context& context) {
     auto catcher = std::make_unique<CatchResult>();
@@ -48,14 +40,6 @@ PersistenceProvider::remove(const Bucket& bucket, Timestamp timestamp, const Doc
     return dynamic_cast<const RemoveResult &>(*future.get());
 }
 
-void
-PersistenceProvider::removeAsync(const Bucket &bucket, Timestamp timestamp, const DocumentId & docId, Context &context,
-                                 OperationComplete::UP onComplete)
-{
-    RemoveResult result = remove(bucket, timestamp, docId, context);
-    onComplete->onComplete(std::make_unique<RemoveResult>(result));
-}
-
 RemoveResult
 PersistenceProvider::removeIfFound(const Bucket& bucket, Timestamp timestamp, const DocumentId & docId, Context& context) {
     auto catcher = std::make_unique<CatchResult>();
@@ -64,14 +48,6 @@ PersistenceProvider::removeIfFound(const Bucket& bucket, Timestamp timestamp, co
     return dynamic_cast<const RemoveResult &>(*future.get());
 }
 
-void
-PersistenceProvider::removeIfFoundAsync(const Bucket &bucket, Timestamp timestamp, const DocumentId & docId, Context &context,
-                                        OperationComplete::UP onComplete)
-{
-    RemoveResult result = removeIfFound(bucket, timestamp, docId, context);
-    onComplete->onComplete(std::make_unique<RemoveResult>(result));
-}
-
 UpdateResult
 PersistenceProvider::update(const Bucket& bucket, Timestamp timestamp, DocumentUpdateSP upd, Context& context) {
     auto catcher = std::make_unique<CatchResult>();
@@ -80,12 +56,4 @@ PersistenceProvider::update(const Bucket& bucket, Timestamp timestamp, DocumentU
     return dynamic_cast<const UpdateResult &>(*future.get());
 }
 
-void
-PersistenceProvider::updateAsync(const Bucket &bucket, Timestamp timestamp, DocumentUpdateSP upd, Context &context,
-                                        OperationComplete::UP onComplete)
-{
-    UpdateResult result = update(bucket, timestamp, std::move(upd), context);
-    onComplete->onComplete(std::make_unique<UpdateResult>(result));
-}
-
 }
diff --git a/persistence/src/vespa/persistence/spi/persistenceprovider.h b/persistence/src/vespa/persistence/spi/persistenceprovider.h
index 99e80cc197a..83eb042d855 100644
--- a/persistence/src/vespa/persistence/spi/persistenceprovider.h
+++ b/persistence/src/vespa/persistence/spi/persistenceprovider.h
@@ -57,6 +57,14 @@ struct PersistenceProvider
 
     virtual ~PersistenceProvider();
 
+    // TODO Move to utility class for use in tests only
+    Result deleteBucket(const Bucket&, Context&);
+    Result put(const Bucket&, Timestamp, DocumentSP, Context&);
+    Result setActiveState(const Bucket&, BucketInfo::ActiveState);
+    RemoveResult remove(const Bucket&, Timestamp timestamp, const DocumentId& id, Context&);
+    RemoveResult removeIfFound(const Bucket&, Timestamp timestamp, const DocumentId& id, Context&);
+    UpdateResult update(const Bucket&, Timestamp timestamp, DocumentUpdateSP update, Context&);
+
     /**
      * Initializes the persistence provider. This function is called exactly
      * once when the persistence provider starts. If any error is returned
@@ -86,7 +94,6 @@ struct PersistenceProvider
      * other buckets may be deactivated, so the node must be able to serve
      * the data from its secondary index or get reduced coverage.
      */
-    Result setActiveState(const Bucket&, BucketInfo::ActiveState);
     virtual void setActiveStateAsync(const Bucket &, BucketInfo::ActiveState, OperationComplete::UP ) = 0;
 
     /**
@@ -98,11 +105,8 @@ struct PersistenceProvider
 
     /**
      * Store the given document at the given microsecond time.
-     * An implementation must always implement atleast put or putAsync.
-     * If not an eternal recursion will occur.
      */
-    virtual Result put(const Bucket&, Timestamp, DocumentSP, Context&);
-    virtual void putAsync(const Bucket &, Timestamp , DocumentSP , Context &, OperationComplete::UP );
+    virtual void putAsync(const Bucket &, Timestamp , DocumentSP , Context &, OperationComplete::UP ) = 0;
 
     /**
      * This remove function assumes that there exist something to be removed.
@@ -163,8 +167,7 @@ struct PersistenceProvider
      * @param timestamp The timestamp for the new bucket entry.
      * @param id The ID to remove
      */
-    virtual RemoveResult remove(const Bucket&, Timestamp timestamp, const DocumentId& id, Context&);
-    virtual void removeAsync(const Bucket&, Timestamp timestamp, const DocumentId& id, Context&, OperationComplete::UP);
+    virtual void removeAsync(const Bucket&, Timestamp timestamp, const DocumentId& id, Context&, OperationComplete::UP) = 0;
 
     /**
      * @see remove()
@@ -182,8 +185,7 @@ struct PersistenceProvider
      * @param timestamp The timestamp for the new bucket entry.
      * @param id The ID to remove
      */
-    virtual RemoveResult removeIfFound(const Bucket&, Timestamp timestamp, const DocumentId& id, Context&);
-    virtual void removeIfFoundAsync(const Bucket&, Timestamp timestamp, const DocumentId& id, Context&, OperationComplete::UP);
+    virtual void removeIfFoundAsync(const Bucket&, Timestamp timestamp, const DocumentId& id, Context&, OperationComplete::UP) = 0;
 
     /**
      * Remove any trace of the entry with the given timestamp. (Be it a document
@@ -202,8 +204,7 @@ struct PersistenceProvider
      * @param timestamp The timestamp to use for the new update entry.
      * @param update The document update to apply to the stored document.
      */
-    virtual UpdateResult update(const Bucket&, Timestamp timestamp, DocumentUpdateSP update, Context&);
-    virtual void updateAsync(const Bucket&, Timestamp timestamp, DocumentUpdateSP update, Context&, OperationComplete::UP);
+    virtual void updateAsync(const Bucket&, Timestamp timestamp, DocumentUpdateSP update, Context&, OperationComplete::UP) = 0;
 
     /**
      * Retrieves the latest version of the document specified by the
@@ -342,7 +343,6 @@ struct PersistenceProvider
      * After this operation has succeeded, a restart of the provider should
      * not yield the bucket in getBucketList().
      */
-    Result deleteBucket(const Bucket&, Context&);
     virtual void deleteBucketAsync(const Bucket&, Context&, OperationComplete::UP) = 0;
 
     /**