Merge pull request #4113 from vespa-engine/toregge/use-std-mutex-in-searchcore-persistence-engine

Use std::mutex in searchore PersistenceEngine.

2 files changed, 35 insertions, 34 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp
index 6cdec1ec0f9..6f2fbabfb1c 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp
@@ -20,7 +20,6 @@ using storage::spi::PartitionState;
 using storage::spi::PartitionStateList;
 using storage::spi::Result;
 using vespalib::IllegalStateException;
-using vespalib::LockGuard;
 using vespalib::Sequence;
 using vespalib::make_string;
 
@@ -30,7 +29,7 @@ namespace {
 
 class ResultHandlerBase {
 protected:
-    vespalib::Lock           _lock;
+    std::mutex               _lock;
     vespalib::CountDownLatch _latch;
 public:
     ResultHandlerBase(uint32_t waitCnt);
@@ -55,7 +54,7 @@ public:
     ~GenericResultHandler();
     void handle(const Result &result) override {
         if (result.hasError()) {
-            vespalib::LockGuard guard(_lock);
+            std::lock_guard<std::mutex> guard(_lock);
             if (_result.hasError()) {
                 _result = TransportLatch::mergeErrorResults(_result, result);
             } else {
@@ -109,7 +108,7 @@ public:
     ~SynchronizedBucketIdListResultHandler() override;
     void handle(const BucketIdListResult &result) override {
         {
-            vespalib::LockGuard guard(_lock);
+            std::lock_guard<std::mutex> guard(_lock);
             BucketIdListResultHandler::handle(result);
         }
         _latch.countDown();
@@ -161,21 +160,21 @@ BucketInfoResultHandler::~BucketInfoResultHandler() = default;
 PersistenceEngine::HandlerSnapshot::UP
 PersistenceEngine::getHandlerSnapshot() const
 {
-    LockGuard guard(_lock);
+    std::lock_guard<std::mutex> guard(_lock);
     return _handlers.getHandlerSnapshot();
 }
 
 PersistenceEngine::HandlerSnapshot::UP
 PersistenceEngine::getHandlerSnapshot(document::BucketSpace bucketSpace) const
 {
-    LockGuard guard(_lock);
+    std::lock_guard<std::mutex> guard(_lock);
     return _handlers.getHandlerSnapshot(bucketSpace);
 }
 
 PersistenceEngine::HandlerSnapshot::UP
 PersistenceEngine::getHandlerSnapshot(document::BucketSpace bucketSpace, const DocumentId &id) const
 {
-    LockGuard guard(_lock);
+    std::lock_guard<std::mutex> guard(_lock);
     return _handlers.getHandlerSnapshot(bucketSpace, id);
 }
 
@@ -207,7 +206,7 @@ IPersistenceHandler::SP
 PersistenceEngine::putHandler(document::BucketSpace bucketSpace, const DocTypeName &docType,
                               const IPersistenceHandler::SP &handler)
 {
-    LockGuard guard(_lock);
+    std::lock_guard<std::mutex> guard(_lock);
     return _handlers.putHandler(bucketSpace, docType, handler);
 }
 
@@ -215,7 +214,7 @@ PersistenceEngine::putHandler(document::BucketSpace bucketSpace, const DocTypeNa
 IPersistenceHandler::SP
 PersistenceEngine::getHandler(document::BucketSpace bucketSpace, const DocTypeName &docType) const
 {
-    LockGuard guard(_lock);
+    std::lock_guard<std::mutex> guard(_lock);
     return _handlers.getHandler(bucketSpace, docType);
 }
 
@@ -224,7 +223,7 @@ IPersistenceHandler::SP
 PersistenceEngine::removeHandler(document::BucketSpace bucketSpace, const DocTypeName &docType)
 {
     // TODO: Grab bucket list and treat them as modified
-    LockGuard guard(_lock);
+    std::lock_guard<std::mutex> guard(_lock);
     return _handlers.removeHandler(bucketSpace, docType);
 }
 
@@ -449,7 +448,7 @@ PersistenceEngine::createIterator(const Bucket &bucket, const document::FieldSet
     }
     entry->handler_sequence = HandlerSnapshot::release(std::move(*snapshot));
 
-    LockGuard guard(_iterators_lock);
+    std::lock_guard<std::mutex> guard(_iterators_lock);
     static IteratorId id_counter(0);
     IteratorId id(++id_counter);
     _iterators[id] = entry.release();
@@ -461,28 +460,31 @@ PersistenceEngine::IterateResult
 PersistenceEngine::iterate(IteratorId id, uint64_t maxByteSize, Context&) const
 {
     std::shared_lock<std::shared_timed_mutex> rguard(_rwMutex);
-    LockGuard guard(_iterators_lock);
-    auto it = _iterators.find(id);
-    if (it == _iterators.end()) {
-        return IterateResult(Result::PERMANENT_ERROR, make_string("Unknown iterator with id %" PRIu64, id.getValue()));
-    }
-    if (it->second->in_use) {
-        return IterateResult(Result::TRANSIENT_ERROR, make_string("Iterator with id %" PRIu64 " is already in use", id.getValue()));
+    IteratorEntry *iteratorEntry;
+    {
+        std::lock_guard<std::mutex> guard(_iterators_lock);
+        auto it = _iterators.find(id);
+        if (it == _iterators.end()) {
+            return IterateResult(Result::PERMANENT_ERROR, make_string("Unknown iterator with id %" PRIu64, id.getValue()));
+        }
+        iteratorEntry = it->second;
+        if (iteratorEntry->in_use) {
+            return IterateResult(Result::TRANSIENT_ERROR, make_string("Iterator with id %" PRIu64 " is already in use", id.getValue()));
+        }
+        iteratorEntry->in_use = true;
     }
-    it->second->in_use = true;
-    guard.unlock();
 
-    DocumentIterator &iterator = it->second->it;
+    DocumentIterator &iterator = iteratorEntry->it;
     try {
         IterateResult result = iterator.iterate(maxByteSize);
-        LockGuard guard2(_iterators_lock);
-        it->second->in_use = false;
+        std::lock_guard<std::mutex> guard(_iterators_lock);
+        iteratorEntry->in_use = false;
         return result;
     } catch (const std::exception & e) {
         IterateResult result(Result::PERMANENT_ERROR, make_string("Caught exception during visitor iterator.iterate() = '%s'", e.what()));
         LOG(warning, "Caught exception during visitor iterator.iterate() = '%s'", e.what());
-        LockGuard guard2(_iterators_lock);
-        it->second->in_use = false;
+        std::lock_guard<std::mutex> guard(_iterators_lock);
+        iteratorEntry->in_use = false;
         return result;
     }
 }
@@ -492,7 +494,7 @@ Result
 PersistenceEngine::destroyIterator(IteratorId id, Context&)
 {
     std::shared_lock<std::shared_timed_mutex> rguard(_rwMutex);
-    LockGuard guard(_iterators_lock);
+    std::lock_guard<std::mutex> guard(_iterators_lock);
     auto it = _iterators.find(id);
     if (it == _iterators.end()) {
         return Result();
@@ -545,7 +547,7 @@ PersistenceEngine::getModifiedBuckets(BucketSpace bucketSpace) const
     typedef BucketIdListResultV MBV;
     MBV extraModifiedBuckets;
     {
-        LockGuard guard(_lock);
+        std::lock_guard<std::mutex> guard(_lock);
         extraModifiedBuckets.swap(_extraModifiedBuckets[bucketSpace]);
     }
     HandlerSnapshot::UP snap = getHandlerSnapshot(bucketSpace);
@@ -613,7 +615,7 @@ PersistenceEngine::destroyIterators()
     for (;;) {
         IteratorId id;
         {
-            LockGuard guard(_iterators_lock);
+            std::lock_guard<std::mutex> guard(_iterators_lock);
             if (_iterators.empty())
                 break;
             id = _iterators.begin()->first;
@@ -632,7 +634,7 @@ PersistenceEngine::saveClusterState(const ClusterState &calc)
 {
     auto clusterState = std::make_shared<ClusterState>(calc);
     {
-        LockGuard guard(_lock);
+        std::lock_guard<std::mutex> guard(_lock);
         clusterState.swap(_clusterState);
     }
 }
@@ -640,7 +642,7 @@ PersistenceEngine::saveClusterState(const ClusterState &calc)
 PersistenceEngine::ClusterState::SP
 PersistenceEngine::savedClusterState() const
 {
-    LockGuard guard(_lock);
+    std::lock_guard<std::mutex> guard(_lock);
     return _clusterState;
 }
 
@@ -663,7 +665,7 @@ PersistenceEngine::grabExtraModifiedBuckets(BucketSpace bucketSpace, IPersistenc
     BucketIdListResultHandler resultHandler;
     handler.handleListBuckets(resultHandler);
     auto result = std::make_shared<BucketIdListResult>(resultHandler.getResult());
-    LockGuard guard(_lock);
+    std::lock_guard<std::mutex> guard(_lock);
     _extraModifiedBuckets[bucketSpace].push_back(result);
 }
 
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h
index b2abc7911d7..b3e70e9f433 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h
@@ -8,7 +8,6 @@
 #include <vespa/persistence/spi/abstractpersistenceprovider.h>
 #include <vespa/searchcore/proton/common/handlermap.hpp>
 #include <vespa/searchcore/proton/persistenceengine/ipersistencehandler.h>
-#include <vespa/vespalib/util/sync.h>
 #include <mutex>
 #include <shared_mutex>
 
@@ -71,9 +70,9 @@ private:
     const ssize_t                           _defaultSerializedSize;
     const bool                              _ignoreMaxBytes;
     PersistenceHandlerMap                   _handlers;
-    vespalib::Lock                          _lock;
+    mutable std::mutex                      _lock;
     Iterators                               _iterators;
-    vespalib::Lock                          _iterators_lock;
+    mutable std::mutex                      _iterators_lock;
     IPersistenceEngineOwner                &_owner;
     const IResourceWriteFilter             &_writeFilter;
     ClusterState::SP                        _clusterState;