Remove disabled StreamSocket::checkRemoval criteria `MinBytesPerSec…

…` minimum throughput PR #9916 added `MinBytesPerSec` minimum throughput criteria for checkRemoval to limit connections, i.e. cleaning up unresponsiveness or lagging sockets. Due to its unreliable nature per request, the criteria has been disabled per default but remained in the code test w/ UT UnitTimeoutSocket. Turns out that even the UT may fail on certain test setups. We decided to remove this criteria altogether, as it will nor be used. Signed-off-by: Sven Göthel <[email protected]> Change-Id: I9212cdf8b87c1c7b0df5761db956cbaa4fd44f58
CollaboraOnline · Oct 25, 2024 · 105e6ea · 105e6ea
1 parent fcd34d2
commit 105e6ea
Show file tree

Hide file tree

Showing 9 changed files with 2 additions and 214 deletions.
diff --git a/net/NetUtil.hpp b/net/NetUtil.hpp
@@ -41,8 +41,6 @@ class Defaults
 
     /// Maximum total connections (9999 or MAX_CONNECTIONS). Zero disables metric.
     size_t MaxConnections;
-    /// Socket minimum bits per seconds throughput (0). Zero disables metric.
-    double MinBytesPerSec;
 
     /// Socket poll timeout in us (64s), useful to increase for debugging.
     std::chrono::microseconds SocketPollTimeout;
@@ -53,7 +51,6 @@ class Defaults
         , WSPingPeriod(std::chrono::microseconds(3000000))
         , HTTPTimeout(std::chrono::microseconds(30000000))
         , MaxConnections(9999)
-        , MinBytesPerSec(0.0)
         , SocketPollTimeout(std::chrono::microseconds(64000000))
     {
     }

diff --git a/net/Socket.cpp b/net/Socket.cpp
@@ -1502,24 +1502,17 @@ bool StreamSocket::checkRemoval(std::chrono::steady_clock::time_point now)
     if( isIPType() )
     {
         // Forced removal on outside-facing IPv[46] network connections only
-        const auto durTotal =
-            std::chrono::duration_cast<std::chrono::milliseconds>(now - getCreationTime());
         const auto durLast =
             std::chrono::duration_cast<std::chrono::milliseconds>(now - getLastSeenTime());
-        const double bytesPerSecIn = durTotal.count() > 0 ? (double)bytesRcvd() / ((double)durTotal.count() / 1000.0) : 0.0;
         /// TO Criteria: Violate maximum idle (_pollTimeout default 64s)
         const bool isIDLE = _pollTimeout > std::chrono::microseconds::zero() &&
                            durLast > _pollTimeout;
-        /// TO Criteria: Violate minimum bytes-per-sec throughput? (_minBytesPerSec default 0, disabled)
-        const bool isMinThroughput = _minBytesPerSec > std::numeric_limits<double>::epsilon() &&
-                                    bytesPerSecIn > std::numeric_limits<double>::epsilon() &&
-                                    bytesPerSecIn < _minBytesPerSec;
         /// TO Criteria: Shall terminate?
         const bool isTermination = SigUtil::getTerminationFlag();
-        if (isIDLE || isMinThroughput || isTermination )
+        if (isIDLE || isTermination )
         {
             LOG_WRN("CheckRemoval: Timeout: {IDLE " << isIDLE
-                    << ", MinThroughput " << isMinThroughput << ", Termination " << isTermination << "}, "
+                    << ", Termination " << isTermination << "}, "
                     << getStatsString(now) << ", "
                     << *this);
             if (_socketHandler)

diff --git a/net/Socket.hpp b/net/Socket.hpp
@@ -1069,7 +1069,6 @@ class StreamSocket : public Socket,
         Socket(fd, type, creationTime),
         _pollTimeout( net::Defaults::get().SocketPollTimeout ),
         _httpTimeout( net::Defaults::get().HTTPTimeout ),
-        _minBytesPerSec( net::Defaults::get().MinBytesPerSec ),
         _hostname(std::move(host)),
         _wsState(WSState::HTTP),
         _isLocalHost(hostType == LocalHost),
@@ -1739,8 +1738,6 @@ class StreamSocket : public Socket,
     const std::chrono::microseconds _pollTimeout;
     /// defaults to 30s, see net::Defaults::HTTPTimeout
     const std::chrono::microseconds _httpTimeout;
-    /// defaults to 0 (disabled), see net::Defaults::MinBytesPerSec
-    const double _minBytesPerSec;
 
     /// The hostname (or IP) of the peer we are connecting to.
     const std::string _hostname;

diff --git a/test/Makefile.am b/test/Makefile.am
@@ -91,7 +91,6 @@ all_la_unit_tests = \
 	unit-join-disconnect.la \
 	unit-initial-load-fail.la \
 	unit-timeout.la \
-	unit-timeout_socket.la \
 	unit-timeout_wsping.la \
 	unit-timeout_conn.la \
 	unit-timeout_none.la \
@@ -227,8 +226,6 @@ unit_join_disconnect_la_SOURCES = UnitJoinDisconnect.cpp
 unit_join_disconnect_la_LIBADD = $(CPPUNIT_LIBS)
 unit_timeout_la_SOURCES = UnitTimeout.cpp
 unit_timeout_la_LIBADD = $(CPPUNIT_LIBS)
-unit_timeout_socket_la_SOURCES = UnitTimeoutSocket.cpp
-unit_timeout_socket_la_LIBADD = $(CPPUNIT_LIBS)
 unit_timeout_wsping_la_SOURCES = UnitTimeoutWSPing.cpp
 unit_timeout_wsping_la_LIBADD = $(CPPUNIT_LIBS)
 unit_timeout_conn_la_SOURCES = UnitTimeoutConnections.cpp

diff --git a/test/UnitTimeoutConnections.cpp b/test/UnitTimeoutConnections.cpp
@@ -39,7 +39,6 @@ class UnitTimeoutConnections : public UnitTimeoutBase1
         // defaults.HTTPTimeout = std::chrono::microseconds(30000000);
         // defaults.MaxConnections = 9999;
         defaults.MaxConnections = ConnectionLimit;
-        // defaults.MinBytesPerSec = 0.0;
         // defaults.SocketPollTimeout = std::chrono::microseconds(64000000);
     }
 

diff --git a/test/UnitTimeoutNone.cpp b/test/UnitTimeoutNone.cpp
@@ -39,7 +39,6 @@ class UnitTimeoutNone : public UnitTimeoutBase1
         // defaults.HTTPTimeout = std::chrono::microseconds(30000000);
         // defaults.MaxConnections = 9999;
         // defaults.MaxConnections = ConnectionLimit;
-        // defaults.MinBytesPerSec = 0.0;
         // defaults.SocketPollTimeout = std::chrono::microseconds(64000000);
     }
 

diff --git a/test/UnitTimeoutSocket.cpp b/test/UnitTimeoutSocket.cpp
diff --git a/test/UnitTimeoutWSPing.cpp b/test/UnitTimeoutWSPing.cpp
@@ -40,7 +40,6 @@ class UnitTimeoutWSPing : public UnitTimeoutBase0
         defaults.WSPingPeriod = std::chrono::microseconds(10000);
         // defaults.HTTPTimeout = std::chrono::microseconds(30000000);
         // defaults.MaxConnections = 9999;
-        // defaults.MinBytesPerSec = 0.0;
         // defaults.SocketPollTimeout = std::chrono::microseconds(64000000);
     }
 

diff --git a/wsd/COOLWSD.cpp b/wsd/COOLWSD.cpp
@@ -2771,7 +2771,6 @@ void COOLWSD::innerInitialize(Poco::Util::Application& self)
         LOG_DBG("net::Defaults: WSPing[Timeout "
                 << netDefaults.WSPingTimeout << ", Period " << netDefaults.WSPingPeriod << "], HTTP[Timeout "
                 << netDefaults.HTTPTimeout << "], Socket[MaxConnections " << netDefaults.MaxConnections
-                << ", MinBytesPerSec " << netDefaults.MinBytesPerSec
                 << "], SocketPoll[Timeout " << netDefaults.SocketPollTimeout << "]");
     }