From 9850da39b38672510e0ff755355e327d06a264b5 Mon Sep 17 00:00:00 2001 From: Trond Norbye Date: Fri, 18 Jan 2019 09:02:49 +0100 Subject: [PATCH] MB-32704: Remove per-port setting of max connections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *Problem* Memcached used to have a “per port” setting of the maximum number of client which may be connected to the port. The motivation behind that was that we wanted to keep a pool of connections available to make sure that ns_server could connect to the system (via the 11209 port). Later on when we added support for SSL we didn’t have time to look at the overall model, we just copied the “per port” setting into the new SSL connection. This leads into the following “problem” with the current configuration: I can have 5000 connections to 11209 (plain, ipv4/6) I can have 30 000 connections to 11210 (plain, ipv4/6) I can have 30 000 connections to 11207 (SSL, ipv4/6) In a deployment which use a mix of SSL and plain clients one may use 60k clients connected to the system (30k of each type), but deployments which only use PLAIN connections may only use 30k connections (trying to use 30 001 would fail, even if we don’t use a single SSL connection). *Solution* The configuration file provided to memcached contains two new toplevel keys in the configuration: max_connections - A number containing the maximum number of connections allowed to memcached (65k if we use the values in the example above) system_connections - A number reserved for users authenticated as system users. (5k if we use the values in the example above) memcached continues to accept all sockets until we reach max_connections, at that time it'll accept the socket and immediately close the socket. To make sure that the normal client's don't occupy all of the connections to the server, memcached performs checks in the validation phase to determine if connections needs to be closed by using the following logic: 1. If the connection is represent a system-internal user, leave the connection alone and continue to execute the command. 2. If we've exceeded the number of "user" connections (60k in the example above) and used more than half of the system connections the connection is disconnected if it is authenticated or the commad being executed isn't one of Hello, SaslListMech, SaslAuth or SaslStep. Change-Id: I3aec178b48f45fa055efb465ca9cea59fd71a895 Reviewed-on: http://review.couchbase.org/103761 Reviewed-by: Jim Walker Tested-by: Build Bot --- daemon/connection.cc | 15 +++- daemon/connection.h | 4 +- daemon/listening_port.cc | 2 - daemon/listening_port.h | 6 -- daemon/memcached.cc | 77 +++++++++++--------- daemon/network_interface.cc | 6 -- daemon/network_interface.h | 1 - daemon/protocol/mcbp/stats_context.cc | 11 +-- daemon/server_socket.cc | 30 ++------ daemon/settings.cc | 50 +++++++++---- daemon/settings.h | 47 ++++++------ daemon/statemachine.cc | 69 +++++++++++++----- daemon/stats.h | 3 + tests/config_parse_test/config_parse_test.cc | 56 +++++++++++++- tests/testapp/testapp.cc | 5 +- 15 files changed, 236 insertions(+), 146 deletions(-) diff --git a/daemon/connection.cc b/daemon/connection.cc index d00927502d..240b4b81e8 100644 --- a/daemon/connection.cc +++ b/daemon/connection.cc @@ -244,7 +244,7 @@ void Connection::restartAuthentication() { externalAuthManager->logoff(username); } sasl_conn.reset(); - internal = false; + setInternal(false); authenticated = false; username = ""; } @@ -1273,6 +1273,9 @@ Connection::Connection(SOCKET sfd, event_base* b, const ListeningPort& ifc) } Connection::~Connection() { + if (internal) { + --stats.system_conns; + } if (authenticated && domain == cb::sasl::Domain::External) { externalAuthManager->logoff(username); } @@ -1322,6 +1325,16 @@ bool Connection::shouldDelete() { return getState() == StateMachine::State ::destroyed; } +void Connection::setInternal(bool internal) { + if (Connection::internal) { + --stats.system_conns; + } + Connection::internal = internal; + if (internal) { + ++stats.system_conns; + } +} + size_t Connection::getNumberOfCookies() const { size_t ret = 0; for (const auto& cookie : cookies) { diff --git a/daemon/connection.h b/daemon/connection.h index ec15fe8ea9..86c7953e00 100644 --- a/daemon/connection.h +++ b/daemon/connection.h @@ -173,9 +173,7 @@ class Connection : public dcp_message_producers { * An internal user is a user which is used by one of the components * in Couchbase (like ns_server, indexer etc). */ - void setInternal(bool internal) { - Connection::internal = internal; - } + void setInternal(bool internal); /** * Update the username to reflect what the user used from the SASL diff --git a/daemon/listening_port.cc b/daemon/listening_port.cc index 7de1b964f9..c3cfd0f1e7 100644 --- a/daemon/listening_port.cc +++ b/daemon/listening_port.cc @@ -19,8 +19,6 @@ ListeningPort::ListeningPort(in_port_t port, std::string host, bool tcp_nodelay) : port(port), - curr_conns(1), - maxconns(0), host(std::move(host)), ipv6(false), ipv4(false), diff --git a/daemon/listening_port.h b/daemon/listening_port.h index f0e537413c..180c6e2170 100644 --- a/daemon/listening_port.h +++ b/daemon/listening_port.h @@ -41,12 +41,6 @@ class ListeningPort { */ const in_port_t port; - /** The current number of connections connected to this port */ - int curr_conns; - - /** The maximum number of connections allowed for this port */ - int maxconns; - /** The hostname this port is bound to ("*" means all interfaces) */ const std::string host; diff --git a/daemon/memcached.cc b/daemon/memcached.cc index eed820b384..afae3c8553 100644 --- a/daemon/memcached.cc +++ b/daemon/memcached.cc @@ -132,6 +132,11 @@ std::unique_ptr executorPool; /* Mutex for global stats */ std::mutex stats_mutex; +/// The maximum number of file handles we may have. During startup +/// we'll try to increase the allowed number of file handles to the +/// limit specified for the current user. +static size_t max_file_handles; + /* * forward declarations */ @@ -430,6 +435,33 @@ static size_t get_number_of_worker_threads() { return ret; } +/// We might not support as many connections as requested if +/// we don't have enough file descriptors available +static void recalculate_max_connections() { + const auto maxconn = settings.getMaxConnections(); + const auto system = (3 * (settings.getNumWorkerThreads() + 2)) + 1024; + const uint64_t maxfiles = maxconn + system; + + if (max_file_handles < maxfiles) { + const auto newmax = max_file_handles - system; + settings.setMaxConnections(newmax, false); + LOG_WARNING( + "max_connections is set higher than the available number of " + "file descriptors available. Reduce max_connections to: {}", + newmax); + + if (newmax > settings.getSystemConnections()) { + LOG_WARNING( + "system_connections > max_connections. Reduce " + "system_connections to {}", + settings.getSystemConnections(), + newmax, + newmax / 2); + settings.setSystemConnections(newmax / 2); + } + } +} + static void breakpad_changed_listener(const std::string&, Settings &s) { cb::breakpad::initialize(s.getBreakpadSettings()); } @@ -473,12 +505,10 @@ static void interfaces_changed_listener(const std::string&, Settings &s) { for (const auto& ifc : s.getInterfaces()) { auto* port = get_listening_port_instance(ifc.port); if (port != nullptr) { - port->maxconns = ifc.maxconn; port->tcp_nodelay = ifc.tcp_nodelay; port->setSslSettings(ifc.ssl.key, ifc.ssl.cert); } } - s.calculateMaxconns(); } #ifdef HAVE_LIBNUMA @@ -516,6 +546,10 @@ static void settings_init() { // Set up the listener functions settings.addChangeListener("breakpad", breakpad_changed_listener); + settings.addChangeListener("max_connections", + [](const std::string&, Settings& s) -> void { + recalculate_max_connections(); + }); settings.addChangeListener("ssl_minimum_protocol", ssl_minimum_protocol_changed_listener); settings.addChangeListener("ssl_cipher_list", @@ -978,9 +1012,6 @@ static void add_listening_port(const NetworkInterface *interf, in_port_t port, s if (descr == nullptr) { ListeningPort newport(port, interf->host, interf->tcp_nodelay); - newport.curr_conns = 1; - newport.maxconns = interf->maxconn; - newport.setSslSettings(interf->ssl.key, interf->ssl.cert); if (family == AF_INET) { @@ -998,7 +1029,6 @@ static void add_listening_port(const NetworkInterface *interf, in_port_t port, s } else if (family == AF_INET6) { descr->ipv6 = true; } - ++descr->curr_conns; } } @@ -2119,32 +2149,6 @@ void delete_all_buckets() { } while (!done); } -static void set_max_filehandles() { - const uint64_t maxfiles = settings.getMaxconns() + - (3 * (settings.getNumWorkerThreads() + 2)) + - 1024; - - auto limit = cb::io::maximizeFileDescriptors(maxfiles); - - if (limit < maxfiles) { - LOG_WARNING( - "Failed to set the number of file descriptors " - "to {} due to system resource restrictions. " - "This may cause the system to misbehave once you reach a " - "high connection count as the system won't be able open " - "new files on the system. The maximum number of file " - "descriptors is currently set to {}. The system " - "is configured to allow {} number of client connections, " - "and in addition to that the overhead of the worker " - "threads is {}. Finally the backed database needs to " - "open files to persist data.", - int(maxfiles), - int(limit), - settings.getMaxconns(), - (3 * (settings.getNumWorkerThreads() + 2))); - } -} - /** * The log function used from SASL * @@ -2197,6 +2201,10 @@ extern "C" int memcached_main(int argc, char **argv) { // allocation). const std::string numa_status = configure_numa_policy(); #endif + + max_file_handles = cb::io::maximizeFileDescriptors( + std::numeric_limits::max()); + std::unique_ptr parent_monitor; try { @@ -2302,10 +2310,7 @@ extern "C" int memcached_main(int argc, char **argv) { /* inform interested parties of initial verbosity level */ perform_callbacks(ON_LOG_LEVEL, nullptr, nullptr); - set_max_filehandles(); - - /* Aggregate the maximum number of connections */ - settings.calculateMaxconns(); + recalculate_max_connections(); if (getenv("MEMCACHED_CRASH_TEST")) { // The crash tests wants the system to generate a crash. diff --git a/daemon/network_interface.cc b/daemon/network_interface.cc index 6b7796eb7f..34f9cccbaf 100644 --- a/daemon/network_interface.cc +++ b/daemon/network_interface.cc @@ -24,11 +24,6 @@ #include #include -static void handle_interface_maxconn(NetworkInterface& ifc, - nlohmann::json::const_iterator it) { - ifc.maxconn = gsl::narrow(cb::jsonGet(it)); -} - static void handle_interface_port(NetworkInterface& ifc, nlohmann::json::const_iterator it) { ifc.port = in_port_t(cb::jsonGet(it)); @@ -133,7 +128,6 @@ NetworkInterface::NetworkInterface(const nlohmann::json& json) { }; std::vector handlers = { - {"maxconn", handle_interface_maxconn}, {"port", handle_interface_port}, {"host", handle_interface_host}, {"ipv4", handle_interface_ipv4}, diff --git a/daemon/network_interface.h b/daemon/network_interface.h index 56ca2da5db..9a5ae3d1ed 100644 --- a/daemon/network_interface.h +++ b/daemon/network_interface.h @@ -39,7 +39,6 @@ class NetworkInterface { std::string key; std::string cert; } ssl; - int maxconn = 1000; in_port_t port = 11211; Protocol ipv6 = Protocol::Optional; Protocol ipv4 = Protocol::Optional; diff --git a/daemon/protocol/mcbp/stats_context.cc b/daemon/protocol/mcbp/stats_context.cc index 5dae2c89ad..25bb1a4e3b 100644 --- a/daemon/protocol/mcbp/stats_context.cc +++ b/daemon/protocol/mcbp/stats_context.cc @@ -177,13 +177,10 @@ static ENGINE_ERROR_CODE server_stats(ADD_STAT add_stat_callback, stats.daemon_conns); add_stat(cookie, add_stat_callback, "curr_connections", stats.curr_conns.load(std::memory_order_relaxed)); - for (auto& instance : stats.listening_ports) { - std::string key = - "max_conns_on_port_" + std::to_string(instance.port); - add_stat(cookie, add_stat_callback, key.c_str(), instance.maxconns); - key = "curr_conns_on_port_" + std::to_string(instance.port); - add_stat(cookie, add_stat_callback, key.c_str(), instance.curr_conns); - } + add_stat(cookie, + add_stat_callback, + "system_connections", + stats.system_conns.load(std::memory_order_relaxed)); add_stat(cookie, add_stat_callback, "total_connections", stats.total_conns); add_stat(cookie, add_stat_callback, "connection_structures", stats.conn_structs); diff --git a/daemon/server_socket.cc b/daemon/server_socket.cc index 56cb840e6e..20b6f8eefe 100644 --- a/daemon/server_socket.cc +++ b/daemon/server_socket.cc @@ -128,41 +128,21 @@ void ServerSocket::acceptNewClient() { return; } - int port_conns; - ListeningPort* port_instance; - int curr_conns = stats.curr_conns.fetch_add(1, std::memory_order_relaxed); - { - std::lock_guard guard(stats_mutex); - port_instance = get_listening_port_instance(listen_port); - cb_assert(port_instance); - port_conns = ++port_instance->curr_conns; - } + size_t curr_conns = + stats.curr_conns.fetch_add(1, std::memory_order_relaxed); - if (curr_conns >= settings.getMaxconns() || - port_conns >= port_instance->maxconns) { - { - std::lock_guard guard(stats_mutex); - --port_instance->curr_conns; - } + if (curr_conns >= settings.getMaxConnections()) { stats.rejected_conns++; LOG_WARNING( - "Too many open connections. Current/Limit for port " - "{}: {}/{}; total: {}/{}", - port_instance->port, - port_conns, - port_instance->maxconns, + R"(Too many open connections. total: {}/{})", curr_conns, - settings.getMaxconns()); + settings.getMaxConnections()); safe_close(client); return; } if (cb::net::set_socket_noblocking(client) == -1) { - { - std::lock_guard guard(stats_mutex); - --port_instance->curr_conns; - } LOG_WARNING("Failed to make socket non-blocking. closing it"); safe_close(client); return; diff --git a/daemon/settings.cc b/daemon/settings.cc index 4b8c54c59e..4b3afa3361 100644 --- a/daemon/settings.cc +++ b/daemon/settings.cc @@ -55,9 +55,7 @@ Settings::Settings() reqs_per_event_low_priority(0), default_reqs_per_event(00), max_packet_size(0), - topkeys_size(0), - maxconns(0) { - + topkeys_size(0) { verbose.store(0); connection_idle_time.reset(); dedupe_nmvb_maps.store(false); @@ -435,6 +433,22 @@ static void handle_max_packet_size(Settings& s, const nlohmann::json& obj) { 1024); } +static void handle_max_connections(Settings& s, const nlohmann::json& obj) { + if (!obj.is_number_unsigned()) { + cb::throwJsonTypeError( + R"("max_connections" must be a positive number)"); + } + s.setMaxConnections(obj.get()); +} + +static void handle_system_connections(Settings& s, const nlohmann::json& obj) { + if (!obj.is_number_unsigned()) { + cb::throwJsonTypeError( + R"("system_connections" must be a positive number)"); + } + s.setSystemConnections(obj.get()); +} + /** * Handle the "sasl_mechanisms" tag in the settings * @@ -606,6 +620,8 @@ void Settings::reconfigure(const nlohmann::json& json) { {"ssl_minimum_protocol", handle_ssl_minimum_protocol}, {"breakpad", handle_breakpad}, {"max_packet_size", handle_max_packet_size}, + {"max_connections", handle_max_connections}, + {"system_connections", handle_system_connections}, {"sasl_mechanisms", handle_sasl_mechanisms}, {"ssl_sasl_mechanisms", handle_ssl_sasl_mechanisms}, {"stdin_listener", handle_stdin_listener}, @@ -876,6 +892,24 @@ void Settings::updateSettings(const Settings& other, bool apply) { } } + if (other.has.max_connections) { + if (other.max_connections != max_connections) { + LOG_INFO(R"(Change max connections from {} to {})", + max_connections, + other.max_connections); + setMaxConnections(other.max_connections); + } + } + + if (other.has.system_connections) { + if (other.system_connections != system_connections) { + LOG_INFO(R"(Change max connections from {} to {})", + system_connections, + other.system_connections); + setSystemConnections(other.system_connections); + } + } + if (other.has.xattr_enabled) { if (other.xattr_enabled != xattr_enabled) { LOG_INFO("{} XATTR", @@ -906,16 +940,6 @@ void Settings::updateSettings(const Settings& other, bool apply) { continue; } - if (i2.maxconn != i1.maxconn) { - LOG_INFO("Change max connections for {}:{} from {} to {}", - i1.host, - i1.port, - i1.maxconn, - i2.maxconn); - i1.maxconn = i2.maxconn; - changed = true; - } - if (i2.tcp_nodelay != i1.tcp_nodelay) { LOG_INFO("{} TCP NODELAY for {}:{}", i2.tcp_nodelay ? "Enable" : "Disable", diff --git a/daemon/settings.h b/daemon/settings.h index 5e980763a1..3980539dbd 100644 --- a/daemon/settings.h +++ b/daemon/settings.h @@ -300,25 +300,28 @@ class Settings { notify_changed("root"); } - /** - * Get the aggregated number of max connections allowed - * - * @return the sum of maxconns specified for all interfaces - */ - int getMaxconns() const { - return maxconns; + size_t getMaxConnections() const { + return max_connections; } - /** - * Calculate the aggregated count of all connections - */ - void calculateMaxconns() { - maxconns = 0; - for (auto& ifc : interfaces) { - maxconns += ifc.maxconn; + void setMaxConnections(size_t max_connections, bool notify = true) { + Settings::max_connections = max_connections; + has.max_connections = true; + if (notify) { + notify_changed("max_connections"); } } + size_t getSystemConnections() const { + return system_connections; + } + + void setSystemConnections(size_t system_connections) { + Settings::system_connections = system_connections; + has.system_connections = true; + notify_changed("system_connections"); + } + /** * Set the number of request to handle per notification from the * event library @@ -926,6 +929,12 @@ class Settings { std::atomic active_external_users_push_interval{ std::chrono::minutes(5)}; + /// The maximum number of connections allowed + size_t max_connections = 60000; + + /// The pool of connections reserved for system usage + size_t system_connections = 5000; + public: /** * Flags for each of the above config options, indicating if they were @@ -970,6 +979,8 @@ class Settings { bool scramsha_fallback_salt; bool external_auth_service; bool active_external_users_push_interval = false; + bool max_connections = false; + bool system_connections = false; } has; protected: @@ -989,14 +1000,6 @@ class Settings { std::map > change_listeners; void notify_changed(const std::string& key); - - /************************************************************************* - * These settings are not exposed to the user, and are either derived from - * the above, or not directly configurable: - */ -protected: - int maxconns; /* Total number of permitted connections. Derived - from sum of all individual interfaces */ }; extern Settings settings; diff --git a/daemon/statemachine.cc b/daemon/statemachine.cc index 44d1358e92..7c03404686 100644 --- a/daemon/statemachine.cc +++ b/daemon/statemachine.cc @@ -27,6 +27,7 @@ #include "mcbp.h" #include "mcbp_executors.h" #include "sasl_tasks.h" +#include "settings.h" #include #include @@ -397,12 +398,61 @@ bool StateMachine::conn_validate() { // We should not be receiving a server command. // Audit and log audit_invalid_packet(cookie, cookie.getPacket()); - LOG_WARNING("{}: Received a server command. Closing connection"); + LOG_WARNING("{}: Received a server command. Closing connection", + connection.getId()); connection.setState(StateMachine::State::closing); return true; } } // We don't currently have any validators for response packets + if (!connection.isInternal()) { + // Disconnect the client if we're starting to run out of connections + const auto sys = settings.getSystemConnections(); + const auto max = settings.getMaxConnections(); + const auto user = max - sys; + + if (stats.curr_conns > user && stats.system_conns < (sys / 2)) { + // The logic is as follows: + // As long as we've got at least 50% of the system reserved + // connections free, we'll just let the client use the socket. + // If we're beyond that point, we'll disconnect this client + // if one of the following is true: + // * The connection is authenticated + // * The command isn't one of: + // * HELLO + // * SASL_LIST_MECH + // * SASL_AUTH + // * SASL_STEP + + bool authentication = false; + if (header.isRequest()) { + const auto& request = header.getRequest(); + if (cb::mcbp::is_client_magic(request.getMagic())) { + auto opcode = request.getClientOpcode(); + using cb::mcbp::ClientOpcode; + authentication = opcode == ClientOpcode::Hello || + opcode == ClientOpcode::SaslListMechs || + opcode == ClientOpcode::SaslAuth || + opcode == ClientOpcode::SaslStep; + } + } + + if (!authentication || connection.isAuthenticated()) { + LOG_WARNING( + "{}: Shutting down client ({}) as we're running out of" + " connections: System: {}/{} User: {}/{}", + connection.getId(), + connection.getDescription(), + stats.system_conns, + sys, + stats.curr_conns, + max); + setCurrentState(State::closing); + return true; + } + } + } + connection.setState(StateMachine::State::execute); return true; } @@ -554,23 +604,6 @@ bool StateMachine::conn_send_data() { } bool StateMachine::conn_immediate_close() { - { - std::lock_guard guard(stats_mutex); - auto* port_instance = - get_listening_port_instance(connection.getParentPort()); - if (port_instance) { - --port_instance->curr_conns; - } else { - // There isn't any point of throwing an exception here, as it would - // just put us back in the "closing path" of the connection. - // Instead just log it - LOG_WARNING( - "{}: conn_immediate_close: Failed to map parent port: {}", - connection.getId(), - connection.toJSON().dump()); - } - } - disassociate_bucket(connection); // Do the final cleanup of the connection: diff --git a/daemon/stats.h b/daemon/stats.h index dff6bd8ca4..f0b1f50f21 100644 --- a/daemon/stats.h +++ b/daemon/stats.h @@ -200,6 +200,9 @@ struct stats { /** The current number of connections to the server */ std::atomic curr_conns; + /// The number of system connections + std::atomic system_conns; + /** The total number of connections to the server since start (or reset) */ Couchbase::RelaxedAtomic total_conns; diff --git a/tests/config_parse_test/config_parse_test.cc b/tests/config_parse_test/config_parse_test.cc index 4d15c6202c..ed3dd5ffc4 100644 --- a/tests/config_parse_test/config_parse_test.cc +++ b/tests/config_parse_test/config_parse_test.cc @@ -288,7 +288,6 @@ TEST_F(SettingsTest, Interfaces) { EXPECT_EQ(0, ifc0.port); EXPECT_EQ(NetworkInterface::Protocol::Optional, ifc0.ipv4); EXPECT_EQ(NetworkInterface::Protocol::Optional, ifc0.ipv6); - EXPECT_EQ(10, ifc0.maxconn); EXPECT_EQ("*", ifc0.host); } catch (std::exception& exception) { FAIL() << exception.what(); @@ -836,6 +835,28 @@ TEST_F(SettingsTest, max_packet_size) { } } +TEST_F(SettingsTest, max_connections) { + nonNumericValuesShouldFail("max_connections"); + + nlohmann::json obj; + const size_t maxconn = 100; + obj["max_connections"] = maxconn; + Settings settings(obj); + EXPECT_EQ(maxconn, settings.getMaxConnections()); + EXPECT_TRUE(settings.has.max_connections); +} + +TEST_F(SettingsTest, system_connections) { + nonNumericValuesShouldFail("system_connections"); + + nlohmann::json obj; + const size_t maxconn = 100; + obj["system_connections"] = maxconn; + Settings settings(obj); + EXPECT_EQ(maxconn, settings.getSystemConnections()); + EXPECT_TRUE(settings.has.system_connections); +} + TEST_F(SettingsTest, SaslMechanisms) { nonStringValuesShouldFail("sasl_mechanisms"); @@ -1072,7 +1093,6 @@ TEST(SettingsUpdateTest, InterfaceSomeValuesMayChange) { settings.addInterface(ifc); - ifc.maxconn = 10; ifc.tcp_nodelay = false; ifc.ssl.key.assign("/opt/couchbase/security/key.pem"); ifc.ssl.cert.assign("/opt/couchbase/security/cert.pem"); @@ -1080,13 +1100,11 @@ TEST(SettingsUpdateTest, InterfaceSomeValuesMayChange) { updated.addInterface(ifc); EXPECT_NO_THROW(settings.updateSettings(updated, false)); - EXPECT_NE(ifc.maxconn, settings.getInterfaces()[0].maxconn); EXPECT_NE(ifc.tcp_nodelay, settings.getInterfaces()[0].tcp_nodelay); EXPECT_NE(ifc.ssl.key, settings.getInterfaces()[0].ssl.key); EXPECT_NE(ifc.ssl.cert, settings.getInterfaces()[0].ssl.cert); EXPECT_NO_THROW(settings.updateSettings(updated)); - EXPECT_EQ(ifc.maxconn, settings.getInterfaces()[0].maxconn); EXPECT_EQ(ifc.tcp_nodelay, settings.getInterfaces()[0].tcp_nodelay); EXPECT_EQ(ifc.ssl.key, settings.getInterfaces()[0].ssl.key); EXPECT_EQ(ifc.ssl.cert, settings.getInterfaces()[0].ssl.cert); @@ -1177,6 +1195,36 @@ TEST(SettingsUpdateTest, UpdatingLoggerSettingsShouldFail) { std::invalid_argument); } +TEST(SettingsUpdateTest, MaxConnectionsIsDynamic) { + Settings updated; + Settings settings; + settings.setMaxConnections(10); + // setting it to the same value should work + updated.setMaxConnections(10); + settings.updateSettings(updated, false); + + // changing it should work + updated.setMaxConnections(1000); + ; + settings.updateSettings(updated, true); + EXPECT_EQ(1000, settings.getMaxConnections()); +} + +TEST(SettingsUpdateTest, SystemConnectionsIsDynamic) { + Settings updated; + Settings settings; + settings.setSystemConnections(10); + // setting it to the same value should work + updated.setSystemConnections(10); + settings.updateSettings(updated, false); + + // changing it should work + updated.setSystemConnections(1000); + ; + settings.updateSettings(updated, true); + EXPECT_EQ(1000, settings.getSystemConnections()); +} + TEST(SettingsUpdateTest, DefaultReqIsDynamic) { Settings updated; Settings settings; diff --git a/tests/testapp/testapp.cc b/tests/testapp/testapp.cc index a024dc002c..bdc0ff620d 100644 --- a/tests/testapp/testapp.cc +++ b/tests/testapp/testapp.cc @@ -412,16 +412,17 @@ nlohmann::json TestappTest::generate_config(uint16_t ssl_port) { ret["verbosity"] = memcached_verbose - 1; } + ret["max_connections"] = Testapp::MAX_CONNECTIONS; + ret["system_connections"] = Testapp::MAX_CONNECTIONS / 4; + ret["stdin_listener"] = false; ret["interfaces"][0]["port"] = 0; ret["interfaces"][0]["ipv4"] = "optional"; ret["interfaces"][0]["ipv6"] = "optional"; - ret["interfaces"][0]["maxconn"] = Testapp::MAX_CONNECTIONS; ret["interfaces"][0]["host"] = "*"; ret["interfaces"][1]["port"] = ssl_port; ret["interfaces"][1]["ipv4"] = "optional"; ret["interfaces"][1]["ipv6"] = "optional"; - ret["interfaces"][1]["maxconn"] = Testapp::MAX_CONNECTIONS; ret["interfaces"][1]["host"] = "*"; ret["interfaces"][1]["ssl"]["key"] = pem_path; ret["interfaces"][1]["ssl"]["cert"] = cert_path;