From c595d62a06070f9f4463641c40e5b9aa59cbb932 Mon Sep 17 00:00:00 2001
From: Henning Baldersheim <balder@yahoo-inc.com>
Date: Thu, 9 Jul 2020 17:11:51 +0000
Subject: Refactor to avoid deadlock due to trying to aquire a lock you already
 hold. Also reduce number of calls to getDiskBloat/getDiskFootPrint

---
 .../src/vespa/searchlib/docstore/logdatastore.cpp  | 25 +++++++++++-----------
 .../src/vespa/searchlib/docstore/logdatastore.h    |  6 +++---
 2 files changed, 15 insertions(+), 16 deletions(-)

(limited to 'searchlib')

diff --git a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp
index 515d980da3e..4adb3507eeb 100644
--- a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp
+++ b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp
@@ -303,7 +303,7 @@ LogDataStore::compact(uint64_t syncToken)
     const bool doCompact = (_fileChunks.size() > 1);
     if (doCompact) {
         LOG(info, "%s. Will compact", bloatMsg(bloat, usage).c_str());
-        compactWorst(_config.getMaxDiskBloatFactor(), _config.getMaxBucketSpread());
+        compactWorst(_config.getMaxDiskBloatFactor(), _config.getMaxBucketSpread(), isTotalDiskBloatExceeded(usage, bloat));
     }
     flushActiveAndWait(syncToken);
     if (doCompact) {
@@ -314,22 +314,21 @@ LogDataStore::compact(uint64_t syncToken)
 }
 
 bool
-LogDataStore::isTotalDiskBloatExceeded() const {
-    const size_t diskFootPrint = getDiskFootprint();
+LogDataStore::isTotalDiskBloatExceeded(size_t diskFootPrint, size_t bloat) const {
     const size_t maxConfiguredDiskBloat = diskFootPrint * _config.getMaxDiskBloatFactor();
-    return getDiskBloat() > maxConfiguredDiskBloat;
+    return bloat > maxConfiguredDiskBloat;
 }
 
 size_t
 LogDataStore::getMaxCompactGain() const
 {
-    size_t bloat = 0;
-    if ( isTotalDiskBloatExceeded() ) {
-        bloat = getDiskBloat();
+    size_t bloat = getDiskBloat();
+    const size_t diskFootPrint = getDiskFootprint();
+    if ( ! isTotalDiskBloatExceeded(diskFootPrint, bloat) ) {
+        bloat = 0;
     }
 
-    const size_t diskFootPrint = getDiskFootprint();
-    double maxSpread = getMaxBucketSpread();
+    const double maxSpread = getMaxBucketSpread();
     size_t spreadAsBloat = diskFootPrint * (1.0 - 1.0/maxSpread);
     if ( maxSpread < _config.getMaxBucketSpread()) {
         spreadAsBloat = 0;
@@ -381,7 +380,7 @@ LogDataStore::getMaxBucketSpread() const
 }
 
 std::pair<bool, LogDataStore::FileId>
-LogDataStore::findNextToCompact(double bloatLimit, double spreadLimit)
+LogDataStore::findNextToCompact(double bloatLimit, double spreadLimit, bool prioritizeDiskBloat)
 {
     typedef std::multimap<double, FileId, std::greater<double>> CostMap;
     CostMap worstBloat;
@@ -409,7 +408,7 @@ LogDataStore::findNextToCompact(double bloatLimit, double spreadLimit)
         }
     }
     std::pair<bool, FileId> retval(false, FileId(-1));
-    if ( ! worstBloat.empty() && (worstBloat.begin()->first > bloatLimit) && isTotalDiskBloatExceeded()) {
+    if ( ! worstBloat.empty() && (worstBloat.begin()->first > bloatLimit) && prioritizeDiskBloat) {
         retval.first = true;
         retval.second = worstBloat.begin()->second;
     } else if ( ! worstSpread.empty() && (worstSpread.begin()->first > spreadLimit)) {
@@ -423,8 +422,8 @@ LogDataStore::findNextToCompact(double bloatLimit, double spreadLimit)
 }
 
 void
-LogDataStore::compactWorst(double bloatLimit, double spreadLimit) {
-    auto worst = findNextToCompact(bloatLimit, spreadLimit);
+LogDataStore::compactWorst(double bloatLimit, double spreadLimit, bool prioritizeDiskBloat) {
+    auto worst = findNextToCompact(bloatLimit, spreadLimit, prioritizeDiskBloat);
     if (worst.first) {
         compactFile(worst.second);
     }
diff --git a/searchlib/src/vespa/searchlib/docstore/logdatastore.h b/searchlib/src/vespa/searchlib/docstore/logdatastore.h
index a9ba580a314..c709c607f37 100644
--- a/searchlib/src/vespa/searchlib/docstore/logdatastore.h
+++ b/searchlib/src/vespa/searchlib/docstore/logdatastore.h
@@ -187,7 +187,7 @@ private:
     // Implements ISetLid API
     void setLid(const LockGuard & guard, uint32_t lid, const LidInfo & lm) override;
 
-    void compactWorst(double bloatLimit, double spreadLimit);
+    void compactWorst(double bloatLimit, double spreadLimit, bool prioritizeDiskBloat);
     void compactFile(FileId chunkId);
 
     typedef vespalib::RcuVector<uint64_t> LidInfoVector;
@@ -203,7 +203,7 @@ private:
     NameIdSet eraseIncompleteCompactedFiles(NameIdSet partList);
     void internalFlushAll();
 
-    bool isTotalDiskBloatExceeded() const;
+    bool isTotalDiskBloatExceeded(size_t diskFootPrint, size_t bloat) const;
 
     NameIdSet scanDir(const vespalib::string &dir, const vespalib::string &suffix);
     FileId allocateFileId(const LockGuard & guard);
@@ -268,7 +268,7 @@ private:
         return (_fileChunks.empty() ? 0 : _fileChunks.back()->getLastPersistedSerialNum());
     }
     bool shouldCompactToActiveFile(size_t compactedSize) const;
-    std::pair<bool, FileId> findNextToCompact(double bloatLimit, double spreadLimit);
+    std::pair<bool, FileId> findNextToCompact(double bloatLimit, double spreadLimit, bool prioritizeDiskBloat);
     void incGeneration();
     bool canShrinkLidSpace(const vespalib::LockGuard &guard) const;
 
-- 
cgit v1.2.3