Merge pull request #18383 from vespa-engine/balder/only-log-when-something-changes

Only log when config actually changes.

3 files changed, 69 insertions, 7 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 f918ebe9179..46de8479d1d 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
@@ -75,6 +75,55 @@ TEST_F("require that strategy is updated when setting new config", Fixture)
     TEST_DO(f.assertStrategyConfig(6, 3, 30));
 }
 
+void
+expectEqual(const MemoryFlush::Config & a, const MemoryFlush::Config & b) {
+    EXPECT_TRUE(a.equal(b));
+    EXPECT_TRUE(a == b);
+    EXPECT_FALSE( a != b);
+    EXPECT_TRUE(b.equal(a));
+    EXPECT_TRUE(b == a);
+    EXPECT_FALSE( b != a);
+}
+
+void
+expectNotEqual(const MemoryFlush::Config & a, const MemoryFlush::Config & b) {
+    EXPECT_FALSE(a.equal(b));
+    EXPECT_FALSE(a == b);
+    EXPECT_TRUE( a != b);
+    EXPECT_FALSE(b.equal(a));
+    EXPECT_FALSE(b == a);
+    EXPECT_TRUE( b != a);
+}
+
+TEST("require that MemoryFlush::Config equal is correct") {
+    MemoryFlush::Config a, b;
+    expectEqual(a, b);;
+    a.maxGlobalMemory = 7;
+    expectNotEqual(a, b);
+    b.maxGlobalMemory = 7;
+    expectEqual(a, b);;
+    a.maxMemoryGain = 8;
+    expectNotEqual(a, b);
+    b.maxMemoryGain = 8;
+    expectEqual(a, b);;
+    a.maxGlobalTlsSize = 9;
+    expectNotEqual(a, b);
+    b.maxGlobalTlsSize = 9;
+    expectEqual(a, b);;
+    a.maxTimeGain = 10us;
+    expectNotEqual(a, b);
+    b.maxTimeGain = 10us;
+    expectEqual(a, b);;
+    a.globalDiskBloatFactor = 11;
+    expectNotEqual(a, b);
+    b.globalDiskBloatFactor = 11;
+    expectEqual(a, b);;
+    a.diskBloatFactor = 12;
+    expectNotEqual(a, b);
+    b.diskBloatFactor = 12;
+    expectEqual(a, b);;
+}
+
 TEST("require that we use configured memory limits") {
     auto cfg = MemoryFlushConfigUpdater::convertConfig(getConfig(6, 3, 30), defaultMemory);
     EXPECT_EQUAL(cfg.maxGlobalMemory, 6u);
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 cf51c7be518..b842820bfe1 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
@@ -74,13 +74,16 @@ MemoryFlushConfigUpdater::updateFlushStrategy(const LockGuard &guard, const char
     MemoryFlush::Config newConfig = convertConfig(_currConfig, _memory);
     considerUseConservativeDiskMode(guard, newConfig);
     considerUseConservativeMemoryMode(guard, newConfig);
-    _flushStrategy->setConfig(newConfig);
-    LOG(info, "Due to %s (conservative-disk=%d, conservative-memory=%d, retired=%d) flush config updated to "
-              "global-disk-bloat(%1.2f), max-tls-size(%" PRIu64 "),"
-              "max-global-memory(%" PRIu64 "), max-memory-gain(%" PRIu64 ")",
-        why, _useConservativeDiskMode, _useConservativeMemoryMode, _nodeRetired,
-        newConfig.globalDiskBloatFactor, newConfig.maxGlobalTlsSize,
-        newConfig.maxGlobalMemory, newConfig.maxMemoryGain);
+    MemoryFlush::Config currentConfig = _flushStrategy->getConfig();
+    if ( currentConfig != newConfig ) {
+        _flushStrategy->setConfig(newConfig);
+        LOG(info, "Due to %s (conservative-disk=%d, conservative-memory=%d, retired=%d) flush config updated to "
+                  "global-disk-bloat(%1.2f), max-tls-size(%" PRIu64 "),"
+                                                                    "max-global-memory(%" PRIu64 "), max-memory-gain(%" PRIu64 ")",
+            why, _useConservativeDiskMode, _useConservativeMemoryMode, _nodeRetired,
+            newConfig.globalDiskBloatFactor, newConfig.maxGlobalTlsSize,
+            newConfig.maxGlobalMemory, newConfig.maxMemoryGain);
+    }
 }
 
 MemoryFlushConfigUpdater::MemoryFlushConfigUpdater(const MemoryFlush::SP &flushStrategy,
diff --git a/searchcore/src/vespa/searchcore/proton/server/memoryflush.h b/searchcore/src/vespa/searchcore/proton/server/memoryflush.h
index 82cf3397ef0..1e8d1b1f08c 100644
--- a/searchcore/src/vespa/searchcore/proton/server/memoryflush.h
+++ b/searchcore/src/vespa/searchcore/proton/server/memoryflush.h
@@ -34,6 +34,16 @@ public:
                uint64_t maxMemoryGain_in,
                double diskBloatFactor_in,
                vespalib::duration maxTimeGain_in);
+        bool operator == (const Config & rhs) const { return equal(rhs); }
+        bool operator != (const Config & rhs) const { return ! equal(rhs); }
+        bool equal(const Config & rhs) const {
+            return (maxGlobalMemory == rhs.maxGlobalMemory) &&
+                   (maxGlobalTlsSize == rhs.maxGlobalTlsSize) &&
+                   (globalDiskBloatFactor == rhs.globalDiskBloatFactor) &&
+                   (maxMemoryGain == rhs.maxMemoryGain) &&
+                   (maxTimeGain == rhs.maxTimeGain) &&
+                   (diskBloatFactor == rhs. diskBloatFactor);
+        }
     };
 
     enum OrderType { DEFAULT, MAXAGE, DISKBLOAT, TLSSIZE, MEMORY };