Merge pull request #4355 from vespa-engine/toregge/allow-more-disk-bloat-while-node-state-is-retired

Allow more disk bloat while node state is retired.

4 files changed, 51 insertions, 2 deletions

diff --git a/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp b/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp
index a76596aa927..b3a7f5f525e 100644
--- a/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp
+++ b/searchcore/src/tests/proton/server/memory_flush_config_updater/memory_flush_config_updater_test.cpp
@@ -56,9 +56,16 @@ struct Fixture
         EXPECT_EQUAL(expMaxEachMemory, strategy->getConfig().maxMemoryGain);
         EXPECT_EQUAL(expMaxGlobalTlsSize, strategy->getConfig().maxGlobalTlsSize);
     }
+    void assertStrategyDiskConfig(double expGlobalDiskBloatFactor, double expDiskBloatFactor) {
+        EXPECT_APPROX(expGlobalDiskBloatFactor, strategy->getConfig().globalDiskBloatFactor, 0.00001);
+        EXPECT_APPROX(expDiskBloatFactor, strategy->getConfig().diskBloatFactor, 0.00001);
+    }
     void notifyDiskMemUsage(const ResourceUsageState &diskState, const ResourceUsageState &memoryState) {
         updater.notifyDiskMemUsage(DiskMemUsageState(diskState, memoryState));
     }
+    void setNodeRetired(bool nodeRetired) {
+        updater.setNodeRetired(nodeRetired);
+    }
 };
 
 TEST_F("require that strategy is updated when setting new config", Fixture)
@@ -133,4 +140,14 @@ TEST_F("require that we must go below low watermark for memory usage before usin
     TEST_DO(f.assertStrategyConfig(4, 1, 20));
 }
 
+TEST_F("require that more disk bloat is allowed while node state is retired", Fixture)
+{
+    f.notifyDiskMemUsage(ResourceUsageState(0.7, 0.3), belowLimit());
+    TEST_DO(f.assertStrategyDiskConfig(0.2, 0.2));
+    f.setNodeRetired(true);
+    TEST_DO(f.assertStrategyDiskConfig((0.8 - 0.3 / 0.7) * 0.8, 1.0));
+    f.notifyDiskMemUsage(belowLimit(), belowLimit());
+    TEST_DO(f.assertStrategyDiskConfig(0.2, 0.2));
+}
+
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp b/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp
index a5b0df67b77..35d6973e34b 100644
--- a/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.cpp
@@ -20,7 +20,7 @@ shouldUseConservativeMode(const ResourceUsageState &resourceState,
 }
 
 void
-MemoryFlushConfigUpdater::considerUseConservativeDiskMode(const LockGuard &,
+MemoryFlushConfigUpdater::considerUseConservativeDiskMode(const LockGuard &guard,
                                                           MemoryFlush::Config &newConfig)
 {
     if (shouldUseConservativeMode(_currState.diskState(), _useConservativeDiskMode,
@@ -30,6 +30,9 @@ MemoryFlushConfigUpdater::considerUseConservativeDiskMode(const LockGuard &,
         _useConservativeDiskMode = true;
     } else {
         _useConservativeDiskMode = false;
+        if (_nodeRetired) {
+            considerUseRelaxedDiskMode(guard, newConfig);
+        }
     }
 }
 
@@ -49,6 +52,18 @@ MemoryFlushConfigUpdater::considerUseConservativeMemoryMode(const LockGuard &,
 }
 
 void
+MemoryFlushConfigUpdater::considerUseRelaxedDiskMode(const LockGuard &, MemoryFlush::Config &newConfig)
+{
+    double utilization = _currState.diskState().utilization();
+    double bloatMargin = _currConfig.conservative.lowwatermarkfactor - utilization;
+    if (bloatMargin > 0.0) {
+        // Node retired and disk utiliation is below low mater mark factor.
+        newConfig.diskBloatFactor = 1.0;
+        newConfig.globalDiskBloatFactor = std::max(bloatMargin * 0.8, _currConfig.diskbloatfactor);
+    }
+}
+
+void
 MemoryFlushConfigUpdater::updateFlushStrategy(const LockGuard &guard)
 {
     MemoryFlush::Config newConfig = convertConfig(_currConfig, _memory);
@@ -66,7 +81,8 @@ MemoryFlushConfigUpdater::MemoryFlushConfigUpdater(const MemoryFlush::SP &flushS
       _memory(memory),
       _currState(),
       _useConservativeDiskMode(false),
-      _useConservativeMemoryMode(false)
+      _useConservativeMemoryMode(false),
+      _nodeRetired(false)
 {
 }
 
@@ -86,6 +102,14 @@ MemoryFlushConfigUpdater::notifyDiskMemUsage(DiskMemUsageState newState)
     updateFlushStrategy(guard);
 }
 
+void
+MemoryFlushConfigUpdater::setNodeRetired(bool nodeRetired)
+{
+    LockGuard guard(_mutex);
+    _nodeRetired = nodeRetired;
+    updateFlushStrategy(guard);
+}
+
 namespace {
 
 size_t
diff --git a/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.h b/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.h
index e6d8f7610f8..28ee330689d 100644
--- a/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.h
+++ b/searchcore/src/vespa/searchcore/proton/server/memory_flush_config_updater.h
@@ -28,12 +28,15 @@ private:
     DiskMemUsageState _currState;
     bool _useConservativeDiskMode;
     bool _useConservativeMemoryMode;
+    bool _nodeRetired;
 
 
     void considerUseConservativeDiskMode(const LockGuard &guard,
                                          MemoryFlush::Config &newConfig);
     void considerUseConservativeMemoryMode(const LockGuard &guard,
                                            MemoryFlush::Config &newConfig);
+    void considerUseRelaxedDiskMode(const LockGuard &guard,
+                                    MemoryFlush::Config &newConfig);
     void updateFlushStrategy(const LockGuard &guard);
 
 public:
@@ -43,6 +46,7 @@ public:
                              const ProtonConfig::Flush::Memory &config,
                              const HwInfo::Memory &memory);
     void setConfig(const ProtonConfig::Flush::Memory &newConfig);
+    void setNodeRetired(bool nodeRetired);
     virtual void notifyDiskMemUsage(DiskMemUsageState newState) override;
 
     static MemoryFlush::Config convertConfig(const ProtonConfig::Flush::Memory &config,
diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp
index ecdcf1c9c0f..b6b995ee69b 100644
--- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp
@@ -777,7 +777,11 @@ Proton::setClusterState(const storage::spi::ClusterState &calc)
     // about whether node is supposed to be up or not.  Match engine
     // needs to know this in order to stop serving queries.
     bool nodeUp(calc.nodeUp());
+    bool nodeRetired(calc.nodeRetired());
     _matchEngine->setNodeUp(nodeUp);
+    if (_memoryFlushConfigUpdater) {
+        _memoryFlushConfigUpdater->setNodeRetired(nodeRetired);
+    }
 }
 
 namespace {