[Libreoffice-commits] online.git: 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 0d7e1536434af962ad37405aa6db135b26ad92c7
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>

diff --git a/wsd/Admin.cpp b/wsd/Admin.cpp
index 29032057..6fa67198 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