[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-3-0' - wsd/Admin.cpp wsd/Admin.hpp

[Ashod Nakashian]

 wsd/Admin.cpp |   33 ++++++++++++++++++---------------
 wsd/Admin.hpp |    5 ++++-
 2 files changed, 22 insertions(+), 16 deletions(-)

New commits:
commit 25d0e8d17c760d55d4df386c57a28de67379bf08
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Wed Jan 10 22:04:14 2018 -0500

    wsd: limit how frequently we collect admin stats
    
    These stats aren't free and here is little point
    in either being hyper accurate in the timing nor
    to allow admins to set nonsensical values (such
    as a few milliseconds of interval).
    
    We cap the interval to a sensible 50ms and reduce
    some of the logging as well.
    
    Change-Id: I66b4be99cf27d135ca267cb497a7a7d07ff437b6
    Reviewed-on: https://gerrit.libreoffice.org/47737
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>
    (cherry picked from commit 0d7e1536434af962ad37405aa6db135b26ad92c7)
    Reviewed-on: https://gerrit.libreoffice.org/48338
    Reviewed-by: Jan Holesovsky <kendy at collabora.com>
    Tested-by: Jan Holesovsky <kendy at collabora.com>

diff --git a/wsd/Admin.cpp b/wsd/Admin.cpp
index 1c93d822..0b904f82 100644
--- a/wsd/Admin.cpp
+++ b/wsd/Admin.cpp
@@ -225,7 +225,7 @@ void AdminSocketHandler::handleMessage(bool /* fin */, WSOpCode /* code */,
                 {
                     _admin->rescheduleMemTimer(settingVal);
                     model.clearMemStats();
-                    model.notify("settings mem_stats_interval=" + std::to_string(settingVal));
+                    model.notify("settings mem_stats_interval=" + std::to_string(_admin->getMemStatsInterval()));
                 }
             }
             else if (settingName == "cpu_stats_size")
@@ -241,7 +241,7 @@ void AdminSocketHandler::handleMessage(bool /* fin */, WSOpCode /* code */,
                 {
                     _admin->rescheduleCpuTimer(settingVal);
                     model.clearCpuStats();
-                    model.notify("settings cpu_stats_interval=" + std::to_string(settingVal));
+                    model.notify("settings cpu_stats_interval=" + std::to_string(_admin->getCpuStatsInterval()));
                 }
             }
             else if (LOOLProtocol::matchPrefix("limit_", settingName))
@@ -375,9 +375,9 @@ void Admin::pollingThread()
 
         int cpuWait = _cpuStatsTaskIntervalMs -
             std::chrono::duration_cast<std::chrono::milliseconds>(now - lastCPU).count();
-        if (cpuWait <= 0)
+        if (cpuWait <= MinStatsIntervalMs / 2) // Close enough
         {
-            size_t currentJiffies = getTotalCpuUsage();
+            const size_t currentJiffies = getTotalCpuUsage();
             auto cpuPercent = 100 * 1000 * currentJiffies / (sysconf (_SC_CLK_TCK) * _cpuStatsTaskIntervalMs);
             _model.addCpuStats(cpuPercent);
 
@@ -387,7 +387,7 @@ void Admin::pollingThread()
 
         int memWait = _memStatsTaskIntervalMs -
             std::chrono::duration_cast<std::chrono::milliseconds>(now - lastMem).count();
-        if (memWait <= 0)
+        if (memWait <= MinStatsIntervalMs / 2) // Close enough
         {
             const auto totalMem = getTotalMemoryUsage();
             if (totalMem != _lastTotalMemory)
@@ -407,7 +407,7 @@ void Admin::pollingThread()
 
         int netWait = _networkStatsIntervalMs -
             std::chrono::duration_cast<std::chrono::milliseconds>(now - lastNet).count();
-        if(netWait <= 0)
+        if (netWait <= MinStatsIntervalMs / 2) // Close enough
         {
             uint64_t sentCount = _model.getSentBytesTotal();
             uint64_t recvCount = _model.getRecvBytesTotal();
@@ -422,9 +422,10 @@ void Admin::pollingThread()
             lastNet = now;
             netWait += _networkStatsIntervalMs;
         }
+
         // Handle websockets & other work.
-        int timeout = std::min(std::min(cpuWait, memWait), netWait);
-        LOG_TRC("Admin poll for " << timeout << "ms");
+        const int timeout = std::max(std::min(std::min(cpuWait, memWait), netWait), MinStatsIntervalMs);
+        LOG_TRC("Admin poll for " << timeout << "ms.");
         poll(timeout);
     }
 }
@@ -452,15 +453,15 @@ void Admin::rmDoc(const std::string& docKey)
 
 void Admin::rescheduleMemTimer(unsigned interval)
 {
-    _memStatsTaskIntervalMs = interval;
-    LOG_INF("Memory stats interval changed - New interval: " << interval);
+    _memStatsTaskIntervalMs = std::max<int>(interval, MinStatsIntervalMs);
+    LOG_INF("Memory stats interval changed - New interval: " << _memStatsTaskIntervalMs);
     wakeup();
 }
 
 void Admin::rescheduleCpuTimer(unsigned interval)
 {
-    _cpuStatsTaskIntervalMs = interval;
-    LOG_INF("CPU stats interval changed - New interval: " << interval);
+    _cpuStatsTaskIntervalMs = std::max<int>(interval, MinStatsIntervalMs);
+    LOG_INF("CPU stats interval changed - New interval: " << _cpuStatsTaskIntervalMs);
     wakeup();
 }
 
@@ -547,16 +548,18 @@ void Admin::notifyForkit()
 
 void Admin::triggerMemoryCleanup(size_t totalMem)
 {
-    LOG_TRC("Total memory we are consuming (in kB): " << totalMem);
     // Trigger mem cleanup when we are consuming too much memory (as configured by sysadmin)
     const auto memLimit = LOOLWSD::getConfigValue<double>("memproportion", 0.0);
-    LOG_TRC("Mem proportion for LOOL configured : " << memLimit);
     if (memLimit == 0.0 || _totalSysMem == 0)
     {
-        LOG_TRC("Not configured to do memory cleanup. Skipping memory cleanup.");
+        LOG_TRC("Total memory we are consuming (in kB): " << totalMem <<
+                ". Not configured to do memory cleanup. Skipping memory cleanup.");
         return;
     }
 
+    LOG_TRC("Total memory we are consuming (in kB): " << totalMem <<
+            ". Mem proportion for LOOL configured : " << memLimit);
+
     float memToFreePercentage = 0;
     if ( (memToFreePercentage = (totalMem/static_cast<double>(_totalSysMem)) - memLimit/100.) > 0.0 )
     {
diff --git a/wsd/Admin.hpp b/wsd/Admin.hpp
index 3e7e0d83..f8a20d11 100644
--- a/wsd/Admin.hpp
+++ b/wsd/Admin.hpp
@@ -141,8 +141,11 @@ private:
 
     std::atomic<int> _memStatsTaskIntervalMs;
     std::atomic<int> _cpuStatsTaskIntervalMs;
-    DocProcSettings _defDocProcSettings;
     std::atomic<int> _networkStatsIntervalMs;
+    DocProcSettings _defDocProcSettings;
+
+    // Don't update any more frequently than this since it's excessive.
+    static const int MinStatsIntervalMs = 50;
 };
 
 #endif