Let removeAsync handle list of documents.

7 files changed, 68 insertions, 28 deletions

diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
index 495522d3cf5..810f2ad2356 100644
--- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
+++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
@@ -3,13 +3,13 @@
 #include <vespa/document/base/testdocman.h>
 #include <vespa/persistence/conformancetest/conformancetest.h>
 #include <vespa/persistence/spi/test.h>
+#include <vespa/persistence/spi/catchresult.h>
 #include <vespa/persistence/spi/resource_usage_listener.h>
 #include <vespa/document/fieldset/fieldsets.h>
 #include <vespa/document/update/documentupdate.h>
 #include <vespa/document/update/assignvalueupdate.h>
 #include <vespa/document/repo/documenttyperepo.h>
 #include <vespa/document/test/make_bucket_space.h>
-#include <vespa/document/util/bytebuffer.h>
 #include <vespa/vdslib/state/state.h>
 #include <vespa/vdslib/state/node.h>
 #include <vespa/vdslib/state/nodestate.h>
@@ -18,7 +18,6 @@
 #include <vespa/vespalib/util/idestructorcallback.h>
 #include <vespa/vespalib/util/size_literals.h>
 #include <vespa/config-stor-distribution.h>
-#include <algorithm>
 #include <limits>
 #include <gtest/gtest.h>
 
@@ -796,6 +795,40 @@ TEST_F(ConformanceTest, testRemove)
     EXPECT_FALSE(getResult.hasDocument());
 }
 
+TEST_F(ConformanceTest, testRemoveMulti)
+{
+    document::TestDocMan testDocMan;
+    _factory->clear();
+    PersistenceProviderUP spi(getSpi(*_factory, testDocMan));
+
+    BucketId bucketId1(8, 0x01);
+    Bucket bucket1(makeSpiBucket(bucketId1));
+    Context context(Priority(0), Trace::TraceLevel(0));
+    spi->createBucket(bucket1, context);
+
+    std::vector<Document::SP> docs;
+    for (size_t i(0); i < 30; i++) {
+        docs.push_back(testDocMan.createRandomDocumentAtLocation(0x01, i));
+    }
+
+    std::vector<PersistenceProvider::TimeStampAndDocumentId> ids;
+    for (size_t i(0); i < docs.size(); i++) {
+        spi->put(bucket1, Timestamp(i), docs[i], context);
+        if (i & 0x1) {
+            ids.emplace_back(Timestamp(i), docs[i]->getId());
+        }
+    }
+
+    auto onDone = std::make_unique<CatchResult>();
+    auto future = onDone->future_result();
+    spi->removeAsync(bucket1, ids, context, std::move(onDone));
+    auto result = future.get();
+    ASSERT_TRUE(result);
+    auto removeResult = dynamic_cast<spi::RemoveResult *>(result.get());
+    ASSERT_TRUE(removeResult != nullptr);
+    EXPECT_EQ(15u, removeResult->num_removed());
+}
+
 TEST_F(ConformanceTest, testRemoveMerge)
 {
     document::TestDocMan testDocMan;
diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
index c7365f39f02..9d9f31b63a3 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
@@ -471,30 +471,33 @@ DummyPersistence::updateAsync(const Bucket& bucket, Timestamp ts, DocumentUpdate
 }
 
 void
-DummyPersistence::removeAsync(const Bucket& b, Timestamp t, const DocumentId& did, Context &, OperationComplete::UP onComplete)
+DummyPersistence::removeAsync(const Bucket& b, std::vector<TimeStampAndDocumentId> ids, Context &, OperationComplete::UP onComplete)
 {
     DUMMYPERSISTENCE_VERIFY_INITIALIZED;
-    LOG(debug, "remove(%s, %" PRIu64 ", %s)",
-        b.toString().c_str(),
-        uint64_t(t),
-        did.toString().c_str());
     assert(b.getBucketSpace() == FixedBucketSpaces::default_space());
-
     BucketContentGuard::UP bc(acquireBucketWithLock(b));
-    while (!bc) {
-        internal_create_bucket(b);
-        bc = acquireBucketWithLock(b);
-    }
-    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);
+
+    uint32_t numRemoves(0);
+    for (const TimeStampAndDocumentId & stampedId : ids) {
+        const DocumentId & id = stampedId.second;
+        Timestamp t = stampedId.first;
+        LOG(debug, "remove(%s, %" PRIu64 ", %s)", b.toString().c_str(), uint64_t(t), id.toString().c_str());
+
+        while (!bc) {
+            internal_create_bucket(b);
+            bc = acquireBucketWithLock(b);
+        }
+        DocEntry::SP entry((*bc)->getEntry(id));
+        numRemoves += (entry.get() && !entry->isRemove()) ? 1 : 0;
+        auto remEntry = std::make_unique<DocEntry>(t, REMOVE_ENTRY, id);
+
+        if ((*bc)->hasTimestamp(t)) {
+            (*bc)->eraseEntry(t);
+        }
+        (*bc)->insert(std::move(remEntry));
     }
-    (*bc)->insert(std::move(remEntry));
     bc.reset();
-    onComplete->onComplete(std::make_unique<RemoveResult>(foundPut));
+    onComplete->onComplete(std::make_unique<RemoveResult>(numRemoves));
 }
 
 GetResult
diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
index 4e0d088d5d6..a7a784d0479 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
@@ -159,7 +159,7 @@ public:
     BucketInfoResult getBucketInfo(const Bucket&) const override;
     GetResult get(const Bucket&, const document::FieldSet&, const DocumentId&, Context&) const 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 removeAsync(const Bucket& b, std::vector<TimeStampAndDocumentId> ids, Context&, OperationComplete::UP) override;
     void updateAsync(const Bucket&, Timestamp, DocumentUpdateSP, Context&, OperationComplete::UP) override;
 
     CreateIteratorResult
diff --git a/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp b/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp
index 951dbf97cff..35654240ec7 100644
--- a/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp
+++ b/persistence/src/vespa/persistence/spi/abstractpersistenceprovider.cpp
@@ -1,10 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "abstractpersistenceprovider.h"
-#include <vespa/document/datatype/documenttype.h>
-#include <vespa/document/update/documentupdate.h>
-#include <vespa/document/fieldset/fieldsets.h>
-#include <vespa/document/fieldvalue/document.h>
+#include <vespa/document/base/documentid.h>
 #include <vespa/vespalib/util/idestructorcallback.h>
 
 namespace storage::spi {
@@ -13,7 +10,9 @@ void
 AbstractPersistenceProvider::removeIfFoundAsync(const Bucket& b, Timestamp timestamp,
                                                 const DocumentId& id, Context& context, OperationComplete::UP onComplete)
 {
-    removeAsync(b, timestamp, id, context, std::move(onComplete));
+    std::vector<TimeStampAndDocumentId> ids;
+    ids.emplace_back(timestamp, id);
+    removeAsync(b, std::move(ids), context, std::move(onComplete));
 }
 
 BucketIdListResult
diff --git a/persistence/src/vespa/persistence/spi/persistenceprovider.cpp b/persistence/src/vespa/persistence/spi/persistenceprovider.cpp
index 31db08a6f4f..e6733dc4150 100644
--- a/persistence/src/vespa/persistence/spi/persistenceprovider.cpp
+++ b/persistence/src/vespa/persistence/spi/persistenceprovider.cpp
@@ -2,6 +2,7 @@
 
 #include "persistenceprovider.h"
 #include "catchresult.h"
+#include <vespa/document/base/documentid.h>
 #include <future>
 
 namespace storage::spi {
@@ -44,7 +45,9 @@ RemoveResult
 PersistenceProvider::remove(const Bucket& bucket, Timestamp timestamp, const DocumentId & docId, Context& context) {
     auto catcher = std::make_unique<CatchResult>();
     auto future = catcher->future_result();
-    removeAsync(bucket, timestamp, docId, context, std::move(catcher));
+    std::vector<TimeStampAndDocumentId> ids;
+    ids.emplace_back(timestamp, docId);
+    removeAsync(bucket, std::move(ids), context, std::move(catcher));
     return dynamic_cast<const RemoveResult &>(*future.get());
 }
 
diff --git a/persistence/src/vespa/persistence/spi/persistenceprovider.h b/persistence/src/vespa/persistence/spi/persistenceprovider.h
index 269175f7d26..8c62e691daf 100644
--- a/persistence/src/vespa/persistence/spi/persistenceprovider.h
+++ b/persistence/src/vespa/persistence/spi/persistenceprovider.h
@@ -54,6 +54,7 @@ struct PersistenceProvider
 {
     using BucketSpace = document::BucketSpace;
     using FieldSetSP = std::shared_ptr<document::FieldSet>;
+    using TimeStampAndDocumentId = std::pair<Timestamp, DocumentId>;
 
     virtual ~PersistenceProvider();
 
@@ -168,7 +169,7 @@ struct PersistenceProvider
      * @param timestamp The timestamp for the new bucket entry.
      * @param id The ID to remove
      */
-    virtual void removeAsync(const Bucket&, Timestamp timestamp, const DocumentId& id, Context&, OperationComplete::UP) = 0;
+    virtual void removeAsync(const Bucket&, std::vector<TimeStampAndDocumentId> ids, Context&, OperationComplete::UP) = 0;
 
     /**
      * @see remove()
diff --git a/persistence/src/vespa/persistence/spi/result.h b/persistence/src/vespa/persistence/spi/result.h
index 73838e99a18..70bd37590a1 100644
--- a/persistence/src/vespa/persistence/spi/result.h
+++ b/persistence/src/vespa/persistence/spi/result.h
@@ -142,6 +142,7 @@ public:
         : _numRemoved(numRemoved) { }
     bool wasFound() const { return _numRemoved > 0; }
     uint32_t num_removed() const { return _numRemoved; }
+    void inc_num_removed(uint32_t add) { _numRemoved += add; }
 
 private:
     uint32_t _numRemoved;