- Remove allowvisitcaching which has been true for many years.

- Improve thread safety.

4 files changed, 25 insertions, 22 deletions

diff --git a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp
index ad0bf60fbe3..414e06bd464 100644
--- a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp
+++ b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp
@@ -528,8 +528,7 @@ VisitCacheStore::VerifyVisitor::~VerifyVisitor() {
 VisitCacheStore::VisitCacheStore(UpdateStrategy strategy) :
     _myDir("visitcache"),
     _repo(makeDocTypeRepoConfig()),
-    _config(DocumentStore::Config(CompressionConfig::LZ4, 1000000, 0)
-                    .allowVisitCaching(true).updateStrategy(strategy),
+    _config(DocumentStore::Config(CompressionConfig::LZ4, 1000000, 0).updateStrategy(strategy),
             LogDataStore::Config().setMaxFileSize(50000).setMaxBucketSpread(3.0)
                     .setFileConfig(WriteableFileChunk::Config(CompressionConfig(), 16_Ki))),
     _fileHeaderContext(),
diff --git a/searchlib/src/vespa/searchlib/docstore/documentstore.cpp b/searchlib/src/vespa/searchlib/docstore/documentstore.cpp
index 0853672e949..08d73719433 100644
--- a/searchlib/src/vespa/searchlib/docstore/documentstore.cpp
+++ b/searchlib/src/vespa/searchlib/docstore/documentstore.cpp
@@ -96,7 +96,10 @@ BackingStore::write(DocumentIdT lid, const Value & value)
 
 void
 BackingStore::reconfigure(const CompressionConfig &compression) {
-    _compression = compression;
+    if (compression != _compression) {
+        // Need proper synchronization
+        _compression = compression;
+    }
 }
 
 using CacheParams = vespalib::CacheParam<
@@ -117,7 +120,6 @@ using docstore::Value;
 bool
 DocumentStore::Config::operator == (const Config &rhs) const {
     return  (_maxCacheBytes == rhs._maxCacheBytes) &&
-            (_allowVisitCaching == rhs._allowVisitCaching) &&
             (_initialCacheEntries == rhs._initialCacheEntries) &&
             (_updateStrategy == rhs._updateStrategy) &&
             (_compression == rhs._compression);
@@ -126,11 +128,11 @@ DocumentStore::Config::operator == (const Config &rhs) const {
 
 DocumentStore::DocumentStore(const Config & config, IDataStore & store)
     : IDocumentStore(),
-      _config(config),
       _backingStore(store),
       _store(std::make_unique<docstore::BackingStore>(_backingStore, config.getCompression())),
       _cache(std::make_unique<docstore::Cache>(*_store, config.getMaxCacheBytes())),
       _visitCache(std::make_unique<docstore::VisitCache>(store, config.getMaxCacheBytes(), config.getCompression())),
+      _updateStrategy(config.updateStrategy()),
       _uncached_lookups(0)
 {
     _cache->reserveElements(config.getInitialCacheEntries());
@@ -142,9 +144,8 @@ void
 DocumentStore::reconfigure(const Config & config) {
     _cache->setCapacityBytes(config.getMaxCacheBytes());
     _store->reconfigure(config.getCompression());
-    _visitCache->reconfigure(_config.getMaxCacheBytes(), config.getCompression());
-
-    _config = config;
+    _visitCache->reconfigure(config.getMaxCacheBytes(), config.getCompression());
+    _updateStrategy.store(config.updateStrategy(), std::memory_order_relaxed);
 }
 
 bool
@@ -152,10 +153,14 @@ DocumentStore::useCache() const {
     return (_cache->capacityBytes() != 0) && (_cache->capacity() != 0);
 }
 
+DocumentStore::Config::UpdateStrategy DocumentStore::updateStrategy() const {
+    return _updateStrategy.load(std::memory_order_relaxed);
+}
+
 void
 DocumentStore::visit(const LidVector & lids, const DocumentTypeRepo &repo, IDocumentVisitor & visitor) const
 {
-    if (useCache() && _config.allowVisitCaching() && visitor.allowVisitCaching()) {
+    if (useCache() && visitor.allowVisitCaching()) {
         docstore::BlobSet blobSet = _visitCache->read(lids).getBlobSet();
         DocumentVisitorAdapter adapter(repo, visitor);
         for (DocumentIdT lid : lids) {
@@ -204,7 +209,7 @@ DocumentStore::write(uint64_t syncToken, DocumentIdT lid, const document::Docume
 void
 DocumentStore::write(uint64_t syncToken, DocumentIdT lid, const vespalib::nbostream & stream) {
     if (useCache()) {
-        switch (_config.updateStrategy()) {
+        switch (updateStrategy()) {
             case Config::UpdateStrategy::INVALIDATE:
                 _backingStore.write(syncToken, lid, stream.peek(), stream.size());
                 _cache->invalidate(lid);
diff --git a/searchlib/src/vespa/searchlib/docstore/documentstore.h b/searchlib/src/vespa/searchlib/docstore/documentstore.h
index 6270108efb8..6d663bfb738 100644
--- a/searchlib/src/vespa/searchlib/docstore/documentstore.h
+++ b/searchlib/src/vespa/searchlib/docstore/documentstore.h
@@ -29,30 +29,25 @@ public:
             _compression(CompressionConfig::LZ4, 9, 70),
             _maxCacheBytes(1000000000),
             _initialCacheEntries(0),
-            _updateStrategy(INVALIDATE),
-            _allowVisitCaching(false)
+            _updateStrategy(INVALIDATE)
         { }
         Config(const CompressionConfig & compression, size_t maxCacheBytes, size_t initialCacheEntries) :
             _compression((maxCacheBytes != 0) ? compression : CompressionConfig::NONE),
             _maxCacheBytes(maxCacheBytes),
             _initialCacheEntries(initialCacheEntries),
-            _updateStrategy(INVALIDATE),
-            _allowVisitCaching(false)
+            _updateStrategy(INVALIDATE)
         { }
         const CompressionConfig & getCompression() const { return _compression; }
         size_t getMaxCacheBytes()   const { return _maxCacheBytes; }
         size_t getInitialCacheEntries() const { return _initialCacheEntries; }
-        bool allowVisitCaching() const { return _allowVisitCaching; }
-        Config & allowVisitCaching(bool allow) { _allowVisitCaching = allow; return *this; }
         Config & updateStrategy(UpdateStrategy strategy) { _updateStrategy = strategy; return *this; }
         UpdateStrategy updateStrategy() const { return _updateStrategy; }
         bool operator == (const Config &) const;
     private:
         CompressionConfig _compression;
-        size_t _maxCacheBytes;
-        size_t _initialCacheEntries;
-        UpdateStrategy _updateStrategy;
-        bool   _allowVisitCaching;
+        size_t            _maxCacheBytes;
+        size_t            _initialCacheEntries;
+        UpdateStrategy    _updateStrategy;
     };
 
     /**
@@ -105,14 +100,15 @@ public:
 
 private:
     bool useCache() const;
+    Config::UpdateStrategy updateStrategy() const;
 
     template <class> class WrapVisitor;
     class WrapVisitorProgress;
-    Config                                   _config;
     IDataStore &                             _backingStore;
     std::unique_ptr<docstore::BackingStore>  _store;
     std::unique_ptr<docstore::Cache>         _cache;
     std::unique_ptr<docstore::VisitCache>    _visitCache;
+    std::atomic<Config::UpdateStrategy>      _updateStrategy;
     mutable std::atomic<uint64_t>            _uncached_lookups;
 };
 
diff --git a/searchlib/src/vespa/searchlib/docstore/visitcache.cpp b/searchlib/src/vespa/searchlib/docstore/visitcache.cpp
index fd6146dae47..041fb62d7de 100644
--- a/searchlib/src/vespa/searchlib/docstore/visitcache.cpp
+++ b/searchlib/src/vespa/searchlib/docstore/visitcache.cpp
@@ -150,7 +150,10 @@ VisitCache::BackingStore::read(const KeySet &key, CompressedBlobSet &blobs) cons
 
 void
 VisitCache::BackingStore::reconfigure(const CompressionConfig &compression) {
-    _compression = compression;
+    if (compression != _compression) {
+        // Need proper synchronization
+        _compression = compression;
+    }
 }