Merge pull request #21871 from vespa-engine/toregge/remove-const-type-qualifier-from-member-functions-that-should-only-be-used-by-writer

Remove const type qualifier from member functions that should only be

10 files changed, 52 insertions, 18 deletions

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 eebef46284b..0bcff91007d 100644
--- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp
@@ -24,7 +24,7 @@ MultiValueMappingBase::~MultiValueMappingBase() = default;
 
 MultiValueMappingBase::RefCopyVector
 MultiValueMappingBase::getRefCopy(uint32_t size) const {
-    assert(size <= _indices.size());
+    assert(size <= _indices.get_size());       // Called from writer only
     auto* indices = &_indices.get_elem_ref(0); // Called from writer only
     RefCopyVector result;
     result.reserve(size);
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 9e42e2763e5..8fe64bd62e7 100644
--- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h
+++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.h
@@ -51,15 +51,31 @@ public:
     size_t getTotalValueCnt() const { return _totalValues; }
     RefCopyVector getRefCopy(uint32_t size) const;
 
-    bool isFull() const { return _indices.isFull(); }
+    /*
+     * isFull() should be called from writer only.
+     * Const type qualifier removed to prevent call from reader.
+     */
+    bool isFull() { return _indices.isFull(); }
     void addDoc(uint32_t &docId);
     void shrink(uint32_t docidLimit);
     void reserve(uint32_t lidLimit);
     void clearDocs(uint32_t lidLow, uint32_t lidLimit, std::function<void(uint32_t)> clearDoc);
-    uint32_t size() const { return _indices.size(); }
+    /*
+     * size() should be called from writer only.
+     * Const type qualifier removed to prevent call from reader.
+     */
+    uint32_t size() { return _indices.size(); }
 
-    uint32_t getNumKeys() const { return _indices.size(); }
-    uint32_t getCapacityKeys() const { return _indices.capacity(); }
+    /*
+     * getNumKeys() should be called from writer only.
+     * Const type qualifier removed to prevent call from reader.
+     */
+    uint32_t getNumKeys() { return _indices.size(); }
+    /*
+     * getCapacityKeys() should be called from writer only.
+     * Const type qualifier removed to prevent call from reader.
+     */
+    uint32_t getCapacityKeys() { return _indices.capacity(); }
     virtual void compactWorst(CompactionSpec compaction_spec, const CompactionStrategy& compaction_strategy) = 0;
     bool considerCompact(const CompactionStrategy &compactionStrategy);
 };
diff --git a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp
index 2d8d4c4858b..4464ab02c86 100644
--- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp
@@ -336,7 +336,7 @@ ReferenceAttribute::getUniqueValueCount() const
 ReferenceAttribute::IndicesCopyVector
 ReferenceAttribute::getIndicesCopy(uint32_t size) const
 {
-    assert(size <= _indices.size());
+    assert(size <= _indices.get_size());       // Called from writer only
     auto* indices = &_indices.get_elem_ref(0); // Called from writer only
     IndicesCopyVector result;
     result.reserve(size);
diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp
index b671a23bbe7..38ef186b717 100644
--- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp
@@ -40,7 +40,7 @@ SingleValueEnumAttributeBase::addDoc(bool &incGeneration)
 SingleValueEnumAttributeBase::EnumIndexCopyVector
 SingleValueEnumAttributeBase::getIndicesCopy(uint32_t size) const
 {
-    assert(size <= _enumIndices.size());
+    assert(size <= _enumIndices.get_size());            // Called from writer only
     auto* enum_indices = &_enumIndices.get_elem_ref(0); // Called from writer only
     EnumIndexCopyVector result;
     result.reserve(size);
diff --git a/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp b/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp
index 39986c43ac5..569e1275080 100644
--- a/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp
+++ b/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp
@@ -75,8 +75,16 @@ private:
     }
 
     size_t getKeyCapacity() const override { return sizeof(T)*8; }
-    size_t getCapacity() const override { return _v.capacity(); }
-    size_t getSize() const override { return _v.size(); }
+    /*
+     * getCapacity() should be called from writer only.
+     * Const type qualifier removed to prevent call from readers.
+     */
+    size_t getCapacity() override { return _v.capacity(); }
+    /*
+     * getSize() should be called from writer only.
+     * Const type qualifier removed to prevent call from readers.
+     */
+    size_t getSize() override { return _v.size(); }
     void adjustDocIdLimit(uint32_t docId) override;
     vespalib::RcuVectorBase<T> _v;
 };
diff --git a/searchlib/src/vespa/searchlib/common/condensedbitvectors.h b/searchlib/src/vespa/searchlib/common/condensedbitvectors.h
index f56890e730f..696400eb7a1 100644
--- a/searchlib/src/vespa/searchlib/common/condensedbitvectors.h
+++ b/searchlib/src/vespa/searchlib/common/condensedbitvectors.h
@@ -24,8 +24,16 @@ public:
     virtual bool get(Key key, uint32_t index) const = 0;
     virtual void clearIndex(uint32_t index) = 0;
     virtual size_t getKeyCapacity() const = 0;
-    virtual size_t getCapacity() const  = 0;
-    virtual size_t getSize() const  = 0;
+    /*
+     * getCapacity() should be called from writer only.
+     * Const type qualifier removed to prevent call from readers.
+     */
+    virtual size_t getCapacity() = 0;
+    /*
+     * getSize() should be called from writer only.
+     * Const type qualifier removed to prevent call from readers.
+     */
+    virtual size_t getSize() = 0;
     virtual void adjustDocIdLimit(uint32_t docId) = 0;
     bool hasKey(Key key) const { return key < getKeyCapacity(); }
     void addKey(Key key) const;
diff --git a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp
index a026b8b8e69..b991773c50f 100644
--- a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp
+++ b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp
@@ -1211,7 +1211,8 @@ LogDataStore::canShrinkLidSpace() const
 bool
 LogDataStore::canShrinkLidSpace(const MonitorGuard &) const
 {
-    return getDocIdLimit() < _lidInfo.size() &&
+    // Update lock is held, allowing call to _lidInfo.get_size()
+    return getDocIdLimit() < _lidInfo.get_size() &&
            _compactLidSpaceGeneration < _genHandler.getFirstUsedGeneration();
 }
 
@@ -1222,7 +1223,8 @@ LogDataStore::getEstimatedShrinkLidSpaceGain() const
     if (!canShrinkLidSpace(guard)) {
         return 0;
     }
-    return (_lidInfo.size() - getDocIdLimit()) * sizeof(uint64_t);
+    // Update lock is held, allowing call to _lidInfo.get_size()
+    return (_lidInfo.get_size() - getDocIdLimit()) * sizeof(uint64_t);
 }
 
 void
diff --git a/searchlib/src/vespa/searchlib/predicate/simple_index.h b/searchlib/src/vespa/searchlib/predicate/simple_index.h
index df3d0686e82..78805820a30 100644
--- a/searchlib/src/vespa/searchlib/predicate/simple_index.h
+++ b/searchlib/src/vespa/searchlib/predicate/simple_index.h
@@ -92,7 +92,7 @@ public:
         : _vector(&vector.acquire_elem_ref(0)),
           _size(size)
     {
-        assert(_size <= vector.size());
+        assert(_size <= vector.get_size()); // Data race: not writer
         linearSeek(1);
     }
 
@@ -165,7 +165,7 @@ private:
     bool shouldCreateVectorPosting(size_t size, double ratio) const;
     bool shouldRemoveVectorPosting(size_t size, double ratio) const;
     size_t getVectorPostingSize(const PostingVector &vector) const {
-        return std::min(vector.size(),
+        return std::min(vector.get_size() /* Data race: not writer */,
                         static_cast<size_t>(_limit_provider.getCommittedDocIdLimit()));
     }
 
diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp
index cc2ecd584e1..8ac24d08dcf 100644
--- a/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp
+++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp
@@ -12,7 +12,7 @@ namespace {
 size_t
 count_valid_link_arrays(const HnswGraph & graph) {
     size_t count(0);
-    size_t num_nodes = graph.node_refs.size();
+    size_t num_nodes = graph.node_refs.get_size(); // Called from writer only
     for (size_t i = 0; i < num_nodes; ++i) {
         auto node_ref = graph.get_node_ref(i);
         if (node_ref.valid()) {
@@ -39,7 +39,7 @@ HnswIndexSaver::HnswIndexSaver(const HnswGraph &graph)
     auto entry = graph.get_entry_node();
     _meta_data.entry_docid = entry.docid;
     _meta_data.entry_level = entry.level;
-    size_t num_nodes = graph.node_refs.size();
+    size_t num_nodes = graph.node_refs.get_size(); // Called from writer only
     assert (num_nodes <= (std::numeric_limits<uint32_t>::max() - 1));
     size_t link_array_count = count_valid_link_arrays(graph);
     assert (link_array_count <= std::numeric_limits<uint32_t>::max());
diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp
index 89f54eacc09..c1cafd547aa 100644
--- a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp
+++ b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp
@@ -268,7 +268,7 @@ TensorAttribute::RefCopyVector
 TensorAttribute::getRefCopy() const
 {
     uint32_t size = getCommittedDocIdLimit();
-    assert(size <= _refVector.size());
+    assert(size <= _refVector.get_size());          // Called from writer only
     auto* ref_vector = &_refVector.get_elem_ref(0); // Called from writer only
     RefCopyVector result;
     result.reserve(size);