From 0bf5d6deef76b7220a6e314ad81fb4a1ba82b2f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sven=20G=C3=B6thel?= Date: Fri, 25 Oct 2024 11:48:57 +0200 Subject: [PATCH] Remove disabled `StreamSocket::checkRemoval` criteria `MinBytesPerSec` minimum throughput MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Change-Id: I9212cdf8b87c1c7b0df5761db956cbaa4fd44f58 --- net/NetUtil.hpp | 3 - net/Socket.cpp | 11 +- net/Socket.hpp | 3 - test/Makefile.am | 3 - test/UnitTimeoutConnections.cpp | 1 - test/UnitTimeoutNone.cpp | 1 - test/UnitTimeoutSocket.cpp | 192 -------------------------------- test/UnitTimeoutWSPing.cpp | 1 - wsd/COOLWSD.cpp | 1 - 9 files changed, 2 insertions(+), 214 deletions(-) delete mode 100644 test/UnitTimeoutSocket.cpp diff --git a/net/NetUtil.hpp b/net/NetUtil.hpp index f2a873f765e8b..62458ae181c07 100644 --- 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 index 762939551ab94..b03f7ac3822db 100644 --- 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(now - getCreationTime()); const auto durLast = std::chrono::duration_cast(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::epsilon() && - bytesPerSecIn > std::numeric_limits::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 index 16d2e48696fe4..95de312c074de 100644 --- 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 index d5a779f64a752..f3339adf7fe87 100644 --- 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 index c55fb62b25554..5d6f1839ce8bf 100644 --- 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 index 8b02cc4f93aea..8ce994e75b521 100644 --- 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 deleted file mode 100644 index 01ed3a69d19b3..0000000000000 --- a/test/UnitTimeoutSocket.cpp +++ /dev/null @@ -1,192 +0,0 @@ -/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ -/* - * Copyright the Collabora Online contributors. - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. - */ - -#include - -#include -#include - -#include -#include - -#include -#include - -#include -#include -#include -#include -#include - -#include "UnitTimeoutBase.hpp" - -/// Test suite class for Socket max-duration timeout limit using HTTP and WS sessions. -class UnitTimeoutSocket : public UnitTimeoutBase0 -{ - TestResult testHttp(); - TestResult testWSPing(); - TestResult testWSDChatPing(); - - void configNet(net::Defaults& defaults) override - { - // defaults.WSPingTimeout = std::chrono::microseconds(2000000); - // defaults.WSPingPeriod = std::chrono::microseconds(3000000); - // defaults.HTTPTimeout = std::chrono::microseconds(30000000); - // defaults.HTTPTimeout = std::chrono::microseconds(1); - // defaults.MaxConnections = 9999; - // defaults.MinBytesPerSec = 0.0; - defaults.MinBytesPerSec = 100000.0; // 100kBps - // defaults.SocketPollTimeout = std::chrono::microseconds(64000000); - } - -public: - UnitTimeoutSocket() - : UnitTimeoutBase0("UnitTimeoutSocket") - { - } - - void invokeWSDTest() override; -}; - -UnitBase::TestResult UnitTimeoutSocket::testHttp() -{ - setTestname(__func__); - TST_LOG("Starting Test: " << testname); - - const std::string documentURL = "/favicon.ico"; - - // Keep alive socket, avoid forced socket disconnect via dtor - TerminatingPoll socketPoller(testname); - socketPoller.runOnClientThread(); - - const int sockCount = 4; - // Reused http session, keep-alive - std::vector> sessions; - - try - { - for(int sockIdx = 0; sockIdx < sockCount; ++sockIdx) { - sessions.push_back( http::Session::create(helpers::getTestServerURI()) ); - TST_LOG("Test: " << testname << "[" << sockIdx << "]: `" << documentURL << "`"); - http::Request request(documentURL, http::Request::VERB_GET); - const std::shared_ptr response = - sessions[sockIdx]->syncRequest(request, socketPoller); - TST_LOG("Response: " << response->header().toString()); - TST_LOG("Response size: " << testname << "[" << sockIdx << "]: `" << documentURL << "`: " << response->header().getContentLength()); - LOK_ASSERT_EQUAL(http::StatusCode::OK, response->statusCode()); - LOK_ASSERT_EQUAL(true, sessions[sockIdx]->isConnected()); - LOK_ASSERT(http::Header::ConnectionToken::None == - response->header().getConnectionToken()); - LOK_ASSERT(0 < response->header().getContentLength()); - } - } - catch (const Poco::Exception& exc) - { - LOK_ASSERT_FAIL(exc.displayText()); - } - LOK_ASSERT_EQUAL(true, pollDisconnected(std::chrono::microseconds(1000000), sessions, &socketPoller)); - - TST_LOG("Clearing Sessions: " << testname); - sessions.clear(); - TST_LOG("Clearing Poller: " << testname); - socketPoller.closeAllSockets(); - TST_LOG("Ending Test: " << testname); - return TestResult::Ok; -} - -/// Test the native WebSocket control-frame ping/pong facility -> No Timeout! -UnitBase::TestResult UnitTimeoutSocket::testWSPing() -{ - setTestname(__func__); - TST_LOG("Starting Test: " << testname); - - std::string documentPath, documentURL; - helpers::getDocumentPathAndURL("hello.odt", documentPath, documentURL, testname); - - std::shared_ptr session = http::WebSocketSession::create(helpers::getTestServerURI()); - http::Request req(documentURL); - session->asyncRequest(req, socketPoll()); - - // wsd/ClientSession.cpp:709 sendTextFrameAndLogError("error: cmd=" + tokens[0] + " kind=nodocloaded"); - constexpr const bool loadDoc = true; // Required for WSD chat -> wsd/ClientSession.cpp:709, common/Session.hpp:160 - if( loadDoc ) { - session->sendMessage("load url=" + documentURL); - } - - LOK_ASSERT_EQUAL(true, session->isConnected()); - - assertMessage(*session, "progress:", "find"); - assertMessage(*session, "progress:", "connect"); - assertMessage(*session, "progress:", "ready"); - - LOK_ASSERT_EQUAL(true, pollDisconnected(std::chrono::microseconds(1000000), *session)); - - TST_LOG("Ending Test: " << testname); - return TestResult::Ok; -} - -/// Tests the WSD chat ping/pong facility, where client sends the ping. -/// See: https://github.com/CollaboraOnline/online/blob/master/wsd/protocol.txt/ -UnitBase::TestResult UnitTimeoutSocket::testWSDChatPing() -{ - setTestname(__func__); - TST_LOG("Starting Test: " << testname); - - std::string documentPath, documentURL; - helpers::getDocumentPathAndURL("hello.odt", documentPath, documentURL, testname); - - std::shared_ptr session = http::WebSocketSession::create(helpers::getTestServerURI()); - http::Request req(documentURL); - session->asyncRequest(req, socketPoll()); - - // wsd/ClientSession.cpp:709 sendTextFrameAndLogError("error: cmd=" + tokens[0] + " kind=nodocloaded"); - constexpr const bool loadDoc = true; // Required for WSD chat -> wsd/ClientSession.cpp:709, common/Session.hpp:160 - if( loadDoc ) { - session->sendMessage("load url=" + documentURL); - } - - LOK_ASSERT_EQUAL(true, session->isConnected()); - - assertMessage(*session, "progress:", "find"); - assertMessage(*session, "progress:", "connect"); - assertMessage(*session, "progress:", "ready"); - - session->sendMessage("ping"); - assertMessage(*session, "", "pong"); - - LOK_ASSERT_EQUAL(true, pollDisconnected(std::chrono::microseconds(1000000), *session)); - - TST_LOG("Ending Test: " << testname); - return TestResult::Ok; -} - -void UnitTimeoutSocket::invokeWSDTest() -{ - UnitBase::TestResult result = TestResult::Ok; - - result = testHttp(); - if (result != TestResult::Ok) - exitTest(result); - - result = testWSPing(); - if (result != TestResult::Ok) - exitTest(result); - - result = testWSDChatPing(); - if (result != TestResult::Ok) - exitTest(result); - - exitTest(TestResult::Ok); -} - -UnitBase* unit_create_wsd(void) { return new UnitTimeoutSocket(); } - -/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/test/UnitTimeoutWSPing.cpp b/test/UnitTimeoutWSPing.cpp index e6bfb1a6831fa..0f2fe34378bd2 100644 --- 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 index 1845030552352..89e09ad131042 100644 --- 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 << "]"); }