Merge pull request #22208 from vespa-engine/toregge/block-compaction-while-buffers-are-held

Block compaction while buffers are held.

9 files changed, 20 insertions, 7 deletions

diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
index fa0b8977c8a..1ef194f6812 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
@@ -226,7 +226,7 @@ template <typename EntryT>
 std::unique_ptr<IEnumStore::EnumIndexRemapper>
 EnumStoreT<EntryT>::consider_compact_values(const CompactionStrategy& compaction_strategy)
 {
-    if (_compaction_spec.get_values().compact()) {
+    if (!_store.get_data_store().has_held_buffers() && _compaction_spec.get_values().compact()) {
         return compact_worst_values(_compaction_spec.get_values(), compaction_strategy);
     }
     return std::unique_ptr<IEnumStore::EnumIndexRemapper>();
diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h
index b57269d04c6..61798959f3e 100644
--- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h
+++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h
@@ -55,7 +55,10 @@ public:
     void doneLoadFromMultiValue() { _store.setInitializing(false); }
 
     void compactWorst(CompactionSpec compactionSpec, const CompactionStrategy& compaction_strategy) override;
+private:
+    virtual bool has_held_buffers() const noexcept override;
 
+public:
     vespalib::AddressSpace getAddressSpaceUsage() const override;
     vespalib::MemoryUsage getArrayStoreMemoryUsage() const override;
     bool has_free_lists_enabled() const { return _store.has_free_lists_enabled(); }
diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp
index 87ffaa75ace..2a0fb24d70f 100644
--- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp
@@ -50,6 +50,13 @@ MultiValueMapping<EntryT,RefT>::compactWorst(CompactionSpec compaction_spec, con
 }
 
 template <typename EntryT, typename RefT>
+bool
+MultiValueMapping<EntryT,RefT>::has_held_buffers() const noexcept
+{
+    return _store.has_held_buffers();
+}
+
+template <typename EntryT, typename RefT>
 vespalib::MemoryUsage
 MultiValueMapping<EntryT,RefT>::getArrayStoreMemoryUsage() const
 {
diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp
index 0bcff91007d..1e50124017c 100644
--- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp
@@ -89,7 +89,7 @@ MultiValueMappingBase::updateStat(const CompactionStrategy& compaction_strategy)
 bool
 MultiValueMappingBase::considerCompact(const CompactionStrategy &compactionStrategy)
 {
-     if (_compaction_spec.compact()) {
+    if (!has_held_buffers() && _compaction_spec.compact()) {
         compactWorst(_compaction_spec, compactionStrategy);
         return true;
     }
diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h
index 8fe64bd62e7..27f2e8f25e3 100644
--- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h
+++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h
@@ -41,6 +41,8 @@ protected:
     }
 
     EntryRef acquire_entry_ref(uint32_t docId) const noexcept { return _indices.acquire_elem_ref(docId).load_acquire(); }
+
+    virtual bool has_held_buffers() const noexcept = 0;
 public:
     using RefCopyVector = vespalib::Array<EntryRef>;
 
diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp
index 8327db769d7..0e09118a3f3 100644
--- a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp
+++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp
@@ -555,7 +555,7 @@ HnswIndex::compact_link_arrays(CompactionSpec compaction_spec, const CompactionS
 bool
 HnswIndex::consider_compact_level_arrays(const CompactionStrategy& compaction_strategy)
 {
-    if (_compaction_spec.level_arrays().compact()) {
+    if (!_graph.nodes.has_held_buffers() && _compaction_spec.level_arrays().compact()) {
         compact_level_arrays(_compaction_spec.level_arrays(), compaction_strategy);
         return true;
     }
@@ -565,7 +565,7 @@ HnswIndex::consider_compact_level_arrays(const CompactionStrategy& compaction_st
 bool
 HnswIndex::consider_compact_link_arrays(const CompactionStrategy& compaction_strategy)
 {
-    if (_compaction_spec.link_arrays().compact()) {
+    if (!_graph.links.has_held_buffers() && _compaction_spec.link_arrays().compact()) {
         compact_link_arrays(_compaction_spec.link_arrays(), compaction_strategy);
         return true;
     }
diff --git a/vespalib/src/vespa/vespalib/datastore/array_store.h b/vespalib/src/vespa/vespalib/datastore/array_store.h
index ed3af451b04..30e0c3ab91f 100644
--- a/vespalib/src/vespa/vespalib/datastore/array_store.h
+++ b/vespalib/src/vespa/vespalib/datastore/array_store.h
@@ -105,6 +105,7 @@ public:
     const BufferState &bufferState(EntryRef ref) const;
 
     bool has_free_lists_enabled() const { return _store.has_free_lists_enabled(); }
+    bool has_held_buffers() const noexcept { return _store.has_held_buffers(); }
 
     static ArrayStoreConfig optimizedConfigForHugePage(size_t maxSmallArraySize,
                                                        size_t hugePageSize,
diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store.h b/vespalib/src/vespa/vespalib/datastore/unique_store.h
index 81034ab4210..e7c374985a7 100644
--- a/vespalib/src/vespa/vespalib/datastore/unique_store.h
+++ b/vespalib/src/vespa/vespalib/datastore/unique_store.h
@@ -67,7 +67,7 @@ public:
     Allocator& get_allocator() { return _allocator; }
     const Allocator& get_allocator() const { return _allocator; }
     IUniqueStoreDictionary& get_dictionary() { return *_dict; }
-    inline const DataStoreType& get_data_store() const { return _allocator.get_data_store(); }
+    inline const DataStoreType& get_data_store() const noexcept { return _allocator.get_data_store(); }
 
     // Pass on hold list management to underlying store
     void transferHoldLists(generation_t generation);
diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_allocator.h b/vespalib/src/vespa/vespalib/datastore/unique_store_allocator.h
index 025165ee0e0..04df88ab4b9 100644
--- a/vespalib/src/vespa/vespalib/datastore/unique_store_allocator.h
+++ b/vespalib/src/vespa/vespalib/datastore/unique_store_allocator.h
@@ -43,8 +43,8 @@ public:
     const EntryType& get(EntryRef ref) const {
         return get_wrapped(ref).value();
     }
-    DataStoreType& get_data_store() { return _store; }
-    const DataStoreType& get_data_store() const { return _store; }
+    DataStoreType& get_data_store() noexcept { return _store; }
+    const DataStoreType& get_data_store() const noexcept { return _store; }
 };
 
 }