Do not hold the bucket guard longer than necessary.

5 files changed, 31 insertions, 32 deletions

diff --git a/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp b/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp
index 9c175da5794..4b4c287b97c 100644
--- a/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp
+++ b/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp
@@ -267,7 +267,11 @@ putGid(DocumentMetaStore &dms, const GlobalId &gid, uint32_t lid, Timestamp time
     EXPECT_TRUE(dms.put(gid, bid, timestamp, docSize, lid, 0u).ok());
 }
 
-TEST(DocumentMetaStoreTest, removed_documents_are_bucketized_to_bucket_0)
+TEST(DocumentMetaStoreTest, control_meta_data_sizeof) {
+    EXPECT_EQ(24u, sizeof(RawDocumentMetaData));
+    EXPECT_EQ(40u, sizeof(search::DocumentMetaData));
+}
+ TEST(DocumentMetaStoreTest, removed_documents_are_bucketized_to_bucket_0)
 {
     DocumentMetaStore dms(createBucketDB());
     dms.constructFreeList();
diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp
index 6ada497be27..78b0780cfa9 100644
--- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp
+++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp
@@ -559,8 +559,8 @@ DocumentMetaStore::updateMetaData(DocId lid,
     return true;
 }
 
-void
-DocumentMetaStore::remove(DocId lid, uint64_t prepare_serial_num, bucketdb::Guard &bucketGuard)
+RawDocumentMetaData
+DocumentMetaStore::removeInternal(DocId lid, uint64_t prepare_serial_num)
 {
     const GlobalId & gid = getRawGid(lid);
     KeyComp comp(gid, _metaDataStore);
@@ -578,11 +578,7 @@ DocumentMetaStore::remove(DocId lid, uint64_t prepare_serial_num, bucketdb::Guar
     }
     _gidToLidMap.remove(itr);
     _lidAlloc.unregisterLid(lid);
-    RawDocumentMetaData &oldMetaData = _metaDataStore[lid];
-    bucketGuard->remove(oldMetaData.getGid(),
-                        oldMetaData.getBucketId().stripUnused(),
-                        oldMetaData.getTimestamp(), oldMetaData.getDocSize(),
-                        _subDbType);
+    return _metaDataStore[lid];
 }
 
 bool
@@ -591,8 +587,9 @@ DocumentMetaStore::remove(DocId lid, uint64_t prepare_serial_num)
     if (!validLid(lid)) {
         return false;
     }
-    bucketdb::Guard bucketGuard = _bucketDB->takeGuard();
-    remove(lid, prepare_serial_num, bucketGuard);
+    RawDocumentMetaData meta = removeInternal(lid, prepare_serial_num);
+    _bucketDB->takeGuard()->remove(meta.getGid(), meta.getBucketId().stripUnused(),
+                                   meta.getTimestamp(), meta.getDocSize(), _subDbType);
     incGeneration();
     if (_op_listener) {
         _op_listener->notify_remove();
@@ -638,13 +635,21 @@ DocumentMetaStore::move(DocId fromLid, DocId toLid, uint64_t prepare_serial_num)
 void
 DocumentMetaStore::removeBatch(const std::vector<DocId> &lidsToRemove, const uint32_t docIdLimit)
 {
-    bucketdb::Guard bucketGuard = _bucketDB->takeGuard();
+    std::vector<RawDocumentMetaData> removed;
+    removed.reserve(lidsToRemove.size());
     for (const auto &lid : lidsToRemove) {
         assert(lid > 0 && lid < docIdLimit);
         (void) docIdLimit;
 
         assert(validLid(lid));
-        remove(lid, 0u, bucketGuard);
+        removed.push_back(removeInternal(lid, 0u));
+    }
+    {
+        bucketdb::Guard bucketGuard = _bucketDB->takeGuard();
+        for (const auto &meta: removed) {
+            bucketGuard->remove(meta.getGid(), meta.getBucketId().stripUnused(),
+                                meta.getTimestamp(), meta.getDocSize(), _subDbType);
+        }
     }
     incGeneration();
     if (_op_listener) {
@@ -691,8 +696,7 @@ DocumentMetaStore::getLid(const GlobalId &gid, DocId &lid) const
     GlobalId value(gid);
     KeyComp comp(value, _metaDataStore);
     auto find_key = GidToLidMapKey::make_find_key(gid);
-    TreeType::ConstIterator itr =
-        _gidToLidMap.getFrozenView().find(find_key, comp);
+    TreeType::ConstIterator itr = _gidToLidMap.getFrozenView().find(find_key, comp);
     if (!itr.valid()) {
         return false;
     }
@@ -718,11 +722,7 @@ DocumentMetaStore::getMetaData(const GlobalId &gid) const
     const RawDocumentMetaData &raw = getRawMetaData(lid);
     Timestamp timestamp(raw.getTimestamp());
     std::atomic_thread_fence(std::memory_order_acquire);
-    return search::DocumentMetaData(lid,
-                                    timestamp,
-                                    raw.getBucketId(),
-                                    raw.getGid(),
-                                    _subDbType == SubDbType::REMOVED);
+    return search::DocumentMetaData(lid, timestamp, raw.getBucketId(), raw.getGid(), _subDbType == SubDbType::REMOVED);
 }
 
 void
@@ -740,10 +740,7 @@ DocumentMetaStore::getMetaData(const BucketId &bucketId,
                 continue; // Wrong bucket (due to overlapping buckets)
             Timestamp timestamp(rawData.getTimestamp());
             std::atomic_thread_fence(std::memory_order_acquire);
-            result.push_back(search::DocumentMetaData(lid, timestamp,
-                            rawData.getBucketId(),
-                            rawData.getGid(),
-                            _subDbType == SubDbType::REMOVED));
+            result.emplace_back(lid, timestamp, rawData.getBucketId(), rawData.getGid(),_subDbType == SubDbType::REMOVED);
         }
     }
 }
diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h
index 7c2db260a8a..b99136a6611 100644
--- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h
+++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h
@@ -126,7 +126,7 @@ private:
 
     VESPA_DLL_LOCAL DocId readNextDoc(documentmetastore::Reader & reader, TreeType::Builder & treeBuilder);
 
-    void remove(DocId lid, uint64_t cached_iterator_sequence_id, bucketdb::Guard &bucketGuard);
+    RawDocumentMetaData removeInternal(DocId lid, uint64_t cached_iterator_sequence_id);
 
 public:
     typedef TreeType::Iterator Iterator;
diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/raw_document_meta_data.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/raw_document_meta_data.h
index f83cb7bc062..e5f7dcc7192 100644
--- a/searchcore/src/vespa/searchcore/proton/documentmetastore/raw_document_meta_data.h
+++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/raw_document_meta_data.h
@@ -23,7 +23,7 @@ struct RawDocumentMetaData
     uint16_t  _docSizeHigh;
     Timestamp _timestamp;
 
-    RawDocumentMetaData()
+    RawDocumentMetaData() noexcept
         : _gid(),
           _bucketUsedBits(BucketId::minNumBits),
           _docSizeLow(0),
@@ -31,7 +31,7 @@ struct RawDocumentMetaData
           _timestamp()
     { }
 
-    RawDocumentMetaData(const GlobalId &gid, const BucketId &bucketId, const Timestamp &timestamp, uint32_t docSize)
+    RawDocumentMetaData(const GlobalId &gid, const BucketId &bucketId, const Timestamp &timestamp, uint32_t docSize) noexcept
         : _gid(gid),
           _bucketUsedBits(bucketId.getUsedBits()),
           _docSizeLow(docSize),
@@ -49,10 +49,10 @@ struct RawDocumentMetaData
         }
     }
 
-    bool operator<(const GlobalId &rhs) const { return _gid < rhs; }
-    bool operator==(const GlobalId &rhs) const { return _gid == rhs; }
-    bool operator<(const RawDocumentMetaData &rhs) const { return _gid < rhs._gid; }
-    bool operator==(const RawDocumentMetaData &rhs) const { return _gid == rhs._gid; }
+    bool operator<(const GlobalId &rhs) const noexcept { return _gid < rhs; }
+    bool operator==(const GlobalId &rhs) const noexcept { return _gid == rhs; }
+    bool operator<(const RawDocumentMetaData &rhs) const noexcept { return _gid < rhs._gid; }
+    bool operator==(const RawDocumentMetaData &rhs) const noexcept { return _gid == rhs._gid; }
 
     const GlobalId &getGid() const { return _gid; }
     GlobalId &getGid() { return _gid; }
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp
index b8b4fd109c4..e7977c7380b 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentbucketmover.cpp
@@ -6,10 +6,8 @@
 #include "maintenancedocumentsubdb.h"
 #include <vespa/searchcore/proton/documentmetastore/i_document_meta_store.h>
 #include <vespa/searchcore/proton/feedoperation/moveoperation.h>
-#include <vespa/searchcore/proton/persistenceengine/i_document_retriever.h>
 #include <vespa/searchcore/proton/bucketdb/bucket_db_owner.h>
 #include <vespa/document/fieldvalue/document.h>
-#include <vespa/vespalib/util/destructor_callbacks.h>
 
 using document::BucketId;
 using document::Document;