Revert "Geirst/commit attributes once when handling delete bucket operations"

2 files changed, 16 insertions, 77 deletions

diff --git a/searchcore/src/tests/proton/attribute/attribute_test.cpp b/searchcore/src/tests/proton/attribute/attribute_test.cpp
index cb1df2df0c5..ce0205af29f 100644
--- a/searchcore/src/tests/proton/attribute/attribute_test.cpp
+++ b/searchcore/src/tests/proton/attribute/attribute_test.cpp
@@ -41,7 +41,6 @@ LOG_SETUP("attribute_test");
 #include <vespa/vespalib/io/fileutil.h>
 #include <vespa/vespalib/test/insertion_operators.h>
 #include <vespa/vespalib/testkit/testapp.h>
-#include <vespa/searchcommon/attribute/iattributevector.h>
 
 namespace vespa { namespace config { namespace search {}}}
 
@@ -56,7 +55,6 @@ using proton::ImportedAttributesRepo;
 using proton::test::AttributeUtils;
 using search::TuneFileAttributes;
 using search::attribute::BitVectorSearchCache;
-using search::attribute::IAttributeVector;
 using search::attribute::ImportedAttributeVector;
 using search::attribute::ReferenceAttribute;
 using search::index::DummyFileHeaderContext;
@@ -71,11 +69,10 @@ using vespalib::tensor::Tensor;
 using vespalib::tensor::TensorCells;
 using vespalib::tensor::TensorDimensions;
 
-using AVConfig = search::attribute::Config;
-using AVBasicType = search::attribute::BasicType;
-using AVCollectionType = search::attribute::CollectionType;
-using Int32AttributeVector = SingleValueNumericAttribute<IntegerAttributeTemplate<int32_t> >;
-using LidVector = LidVectorContext::LidVector;
+typedef search::attribute::Config AVConfig;
+typedef search::attribute::BasicType AVBasicType;
+typedef search::attribute::CollectionType AVCollectionType;
+typedef SingleValueNumericAttribute<IntegerAttributeTemplate<int32_t> > Int32AttributeVector;
 
 namespace
 {
@@ -159,9 +156,6 @@ struct Fixture
     void remove(SerialNum serialNum, DocumentIdT lid, bool immediateCommit = true) {
         _aw->remove(serialNum, lid, immediateCommit, emptyCallback);
     }
-    void remove(const LidVector &lidVector, SerialNum serialNum, bool immediateCommit = true) {
-        _aw->remove(lidVector, serialNum, immediateCommit, emptyCallback);
-    }
     void commit(SerialNum serialNum) {
         _aw->forceCommit(serialNum, emptyCallback);
     }
@@ -299,23 +293,23 @@ TEST_F("require that attribute writer handles predicate put", Fixture)
     EXPECT_TRUE(it.valid());
 }
 
-void
-assertUndefined(const IAttributeVector &attr, uint32_t docId)
-{
-    EXPECT_TRUE(search::attribute::isUndefined<int32_t>(attr.getInt(docId)));
-}
-
 TEST_F("require that attribute writer handles remove", Fixture)
 {
     AttributeVector::SP a1 = f.addAttribute("a1");
     AttributeVector::SP a2 = f.addAttribute("a2");
+    Schema s;
+    s.addAttributeField(Schema::AttributeField("a1", schema::DataType::INT32, CollectionType::SINGLE));
+    s.addAttributeField(Schema::AttributeField("a2", schema::DataType::INT32, CollectionType::SINGLE));
+
+    DocBuilder idb(s);
+
     fillAttribute(a1, 1, 10, 1);
     fillAttribute(a2, 1, 20, 1);
 
     f.remove(2, 0);
 
-    TEST_DO(assertUndefined(*a1, 0));
-    TEST_DO(assertUndefined(*a2, 0));
+    EXPECT_TRUE(search::attribute::isUndefined<int32_t>(a1->getInt(0)));
+    EXPECT_TRUE(search::attribute::isUndefined<int32_t>(a2->getInt(0)));
 
     f.remove(2, 0); // same sync token as previous
     try {
@@ -327,24 +321,6 @@ TEST_F("require that attribute writer handles remove", Fixture)
     }
 }
 
-TEST_F("require that attribute writer handles batch remove", Fixture)
-{
-    AttributeVector::SP a1 = f.addAttribute("a1");
-    AttributeVector::SP a2 = f.addAttribute("a2");
-    fillAttribute(a1, 4, 22, 1);
-    fillAttribute(a2, 4, 33, 1);
-
-    LidVector lidsToRemove = {1,3};
-    f.remove(lidsToRemove, 2);
-
-    TEST_DO(assertUndefined(*a1, 1));
-    EXPECT_EQUAL(22, a1->getInt(2));
-    TEST_DO(assertUndefined(*a1, 3));
-    TEST_DO(assertUndefined(*a2, 1));
-    EXPECT_EQUAL(33, a2->getInt(2));
-    TEST_DO(assertUndefined(*a2, 3));
-}
-
 void verifyAttributeContent(const AttributeVector & v, uint32_t lid, vespalib::stringref expected)
 {
     attribute::ConstCharContent sbuf;
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp
index 816a434e56a..7413d0f369f 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp
@@ -19,8 +19,6 @@ using search::attribute::ImportedAttributeVector;
 
 namespace proton {
 
-using LidVector = LidVectorContext::LidVector;
-
 AttributeWriter::WriteContext::WriteContext(uint32_t executorId)
     : _executorId(executorId),
       _fieldPaths(),
@@ -274,47 +272,13 @@ RemoveTask::run()
     const auto &attributes = _wc.getAttributes();
     for (auto &attrp : attributes) {
         AttributeVector &attr = *attrp;
-        if (attr.getStatus().getLastSyncToken() < _serialNum) {
+        // Must use <= due to batch remove
+        if (attr.getStatus().getLastSyncToken() <= _serialNum) {
             applyRemoveToAttribute(_serialNum, _lid, _immediateCommit, attr, _onWriteDone);
         }
     }
 }
 
-class BatchRemoveTask : public vespalib::Executor::Task
-{
-private:
-    const AttributeWriter::WriteContext &_writeCtx;
-    const SerialNum _serialNum;
-    const LidVector _lidsToRemove;
-    const bool _immediateCommit;
-    std::remove_reference_t<AttributeWriter::OnWriteDoneType> _onWriteDone;
-public:
-    BatchRemoveTask(const AttributeWriter::WriteContext &writeCtx,
-                    SerialNum serialNum,
-                    const LidVector &lidsToRemove,
-                    bool immediateCommit,
-                    AttributeWriter::OnWriteDoneType onWriteDone)
-        : _writeCtx(writeCtx),
-          _serialNum(serialNum),
-          _lidsToRemove(lidsToRemove),
-          _immediateCommit(immediateCommit),
-          _onWriteDone(onWriteDone)
-    {}
-    virtual ~BatchRemoveTask() override {}
-    virtual void run() override {
-        for (auto attr : _writeCtx.getAttributes()) {
-            if (attr->getStatus().getLastSyncToken() < _serialNum) {
-                for (auto lidToRemove : _lidsToRemove) {
-                    applyRemoveToAttribute(_serialNum, lidToRemove, false, *attr, _onWriteDone);
-                }
-                if (_immediateCommit) {
-                    attr->commit(_serialNum, _serialNum);
-                }
-            }
-        }
-    }
-};
-
 class CommitTask : public vespalib::Executor::Task
 {
     const AttributeWriter::WriteContext  &_wc;
@@ -455,9 +419,8 @@ void
 AttributeWriter::remove(const LidVector &lidsToRemove, SerialNum serialNum,
                         bool immediateCommit, OnWriteDoneType onWriteDone)
 {
-    for (const auto &writeCtx : _writeContexts) {
-        auto removeTask = std::make_unique<BatchRemoveTask>(writeCtx, serialNum, lidsToRemove, immediateCommit, onWriteDone);
-        _attributeFieldWriter.executeTask(writeCtx.getExecutorId(), std::move(removeTask));
+    for (const auto &lid : lidsToRemove) {
+        internalRemove(serialNum, lid, immediateCommit, onWriteDone);
     }
 }