DPE-3706 Fix max_connections calculation and expose experimental max_connections field (#429)

* fix max_connections calculation

* Add experimental max-connections configuration

* use config and allow restart for single unit

* missing comment

* clearer variable name

* change calculation logic

* libpatch bump

Author: paulomach

config.yaml

@@ -30,3 +30,11 @@ options:
   mysql-interface-database:
     description: "The database name for the legacy 'mysql' relation"
     type: "string"
+# Experimental features
+  experimental-max-connections:
+    type: int
+    description: |
+      Maximum number of connections allowed to the MySQL server.
+      When set max-connections value take precedence over the memory utilizations
+      againts innodb_buffer_pool_size.
+      This is an experimental feature and may be removed in future releases.

lib/charms/mysql/v0/mysql.py

@@ -130,7 +130,7 @@ def wait_until_mysql_connection(self) -> None:
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 59
+LIBPATCH = 60
 
 UNIT_TEARDOWN_LOCKNAME = "unit-teardown"
 UNIT_ADD_LOCKNAME = "unit-add"
@@ -141,7 +141,8 @@ def wait_until_mysql_connection(self) -> None:
 BYTES_1MiB = 1048576  # 1 mebibyte
 RECOVERY_CHECK_TIME = 10  # seconds
 GET_MEMBER_STATE_TIME = 10  # seconds
-MIN_MAX_CONNECTIONS = 100
+MAX_CONNECTIONS_FLOOR = 10
+MIM_MEM_BUFFERS = 200 * BYTES_1MiB
 
 SECRET_INTERNAL_LABEL = "secret-id"
 SECRET_DELETED_LABEL = "None"
@@ -868,36 +869,60 @@ def render_mysqld_configuration(
         *,
         profile: str,
         memory_limit: Optional[int] = None,
+        experimental_max_connections: Optional[int] = None,
         snap_common: str = "",
     ) -> tuple[str, dict]:
         """Render mysqld ini configuration file.
 
         Args:
             profile: profile to use for the configuration (testing, production)
             memory_limit: memory limit to use for the configuration in bytes
+            experimental_max_connections: explicit max connections to use for the configuration
             snap_common: snap common directory (for log files locations in vm)
 
         Returns: a tuple with mysqld ini file string content and a the config dict
         """
+        max_connections = None
         performance_schema_instrument = ""
         if profile == "testing":
             innodb_buffer_pool_size = 20 * BYTES_1MiB
             innodb_buffer_pool_chunk_size = 1 * BYTES_1MiB
             group_replication_message_cache_size = 128 * BYTES_1MiB
-            max_connections = MIN_MAX_CONNECTIONS
+            max_connections = 100
             performance_schema_instrument = "'memory/%=OFF'"
         else:
             available_memory = self.get_available_memory()
             if memory_limit:
                 # when memory limit is set, we need to use the minimum
                 # between the available memory and the limit
                 available_memory = min(available_memory, memory_limit)
+
+            if experimental_max_connections:
+                # when set, we use the experimental max connections
+                # and it takes precedence over buffers usage
+                max_connections = experimental_max_connections
+                # we reserve 200MiB for memory buffers
+                # even when there's some overcommittment
+                available_memory = max(
+                    available_memory - max_connections * 12 * BYTES_1MiB, 200 * BYTES_1MiB
+                )
+
             (
                 innodb_buffer_pool_size,
                 innodb_buffer_pool_chunk_size,
                 group_replication_message_cache_size,
             ) = self.get_innodb_buffer_pool_parameters(available_memory)
-            max_connections = max(self.get_max_connections(available_memory), MIN_MAX_CONNECTIONS)
+
+            # constrain max_connections based on the available memory
+            # after innodb_buffer_pool_size calculation
+            available_memory -= innodb_buffer_pool_size + (
+                group_replication_message_cache_size or 0
+            )
+            if not max_connections:
+                max_connections = max(
+                    self.get_max_connections(available_memory), MAX_CONNECTIONS_FLOOR
+                )
+
             if available_memory < 2 * BYTES_1GiB:
                 # disable memory instruments if we have less than 2GiB of RAM
                 performance_schema_instrument = "'memory/%=OFF'"

src/charm.py

@@ -254,6 +254,7 @@ def _on_config_changed(self, event: EventBase) -> None:
             profile=self.config.profile,
             snap_common=CHARMED_MYSQL_COMMON_DIRECTORY,
             memory_limit=memory_limit_bytes,
+            experimental_max_connections=self.config.experimental_max_connections,
         )
 
         changed_config = compare_dictionaries(previous_config, new_config_dict)
@@ -612,7 +613,9 @@ def workload_initialise(self) -> None:
         Raised errors must be treated on handlers.
         """
         self._mysql.write_mysqld_config(
-            profile=self.config.profile, memory_limit=self.config.profile_limit_memory
+            profile=self.config.profile,
+            memory_limit=self.config.profile_limit_memory,
+            experimental_max_connections=self.config.experimental_max_connections,
         )
         self._mysql.setup_logrotate_and_cron()
         self._mysql.reset_root_password_and_start_mysqld()
@@ -799,19 +802,22 @@ def _restart(self, event: EventBase) -> None:
             logger.debug("Deferring restart until all units are in the relation")
             event.defer()
             return
-        if self._mysql.is_unit_primary(self.unit_label):
+        if self.peers.units and self._mysql.is_unit_primary(self.unit_label):
             restart_states = {
                 self.restart_peers.data[unit].get("state", "unset") for unit in self.peers.units
             }
-            if restart_states != {"release"}:
+            if restart_states == {"unset"}:
+                logger.debug("Restarting leader")
+            elif restart_states != {"release"}:
                 # Wait other units restart first to minimize primary switchover
                 logger.debug("Primary is waiting for other units to restart")
                 event.defer()
                 return
 
         self.unit.status = MaintenanceStatus("restarting MySQL")
         self._mysql.restart_mysqld()
-        self.unit.status = ActiveStatus(self.active_status_message)
+        sleep(10)
+        self._on_update_status(None)
 
 
 if __name__ == "__main__":

src/config.py

@@ -10,6 +10,7 @@
 from typing import Optional
 
 from charms.data_platform_libs.v0.data_models import BaseConfigModel
+from charms.mysql.v0.mysql import MAX_CONNECTIONS_FLOOR
 from pydantic import validator
 
 logger = logging.getLogger(__name__)
@@ -61,6 +62,7 @@ class CharmConfig(BaseConfigModel):
     profile_limit_memory: Optional[int]
     mysql_interface_user: Optional[str]
     mysql_interface_database: Optional[str]
+    experimental_max_connections: Optional[int]
 
     @validator("profile")
     @classmethod
@@ -103,3 +105,14 @@ def profile_limit_memory_validator(cls, value: int) -> Optional[int]:
             raise ValueError("`profile-limit-memory` limited to 7 digits (9999999MB)")
 
         return value
+
+    @validator("experimental_max_connections")
+    @classmethod
+    def experimental_max_connections_validator(cls, value: int) -> Optional[int]:
+        """Check experimental max connections."""
+        if value < MAX_CONNECTIONS_FLOOR:
+            raise ValueError(
+                f"experimental-max-connections must be greater than {MAX_CONNECTIONS_FLOOR}"
+            )
+
+        return value

src/mysql_vm_helpers.py

@@ -233,12 +233,18 @@ def get_available_memory(self) -> int:
             logger.error("Failed to query system memory")
             raise MySQLGetAvailableMemoryError
 
-    def write_mysqld_config(self, profile: str, memory_limit: Optional[int]) -> None:
+    def write_mysqld_config(
+        self,
+        profile: str,
+        memory_limit: Optional[int],
+        experimental_max_connections: Optional[int] = None,
+    ) -> None:
         """Create custom mysql config file.
 
         Args:
             profile: profile to use for the mysql config
             memory_limit: memory limit to use for the mysql config in MB
+            experimental_max_connections: experimental max connections to use for the mysql config
 
         Raises: MySQLCreateCustomMySQLDConfigError if there is an error creating the
             custom mysqld config
@@ -252,6 +258,7 @@ def write_mysqld_config(self, profile: str, memory_limit: Optional[int]) -> None
                 profile=profile,
                 snap_common=CHARMED_MYSQL_COMMON_DIRECTORY,
                 memory_limit=memory_limit,
+                experimental_max_connections=experimental_max_connections,
             )
         except (MySQLGetAvailableMemoryError, MySQLGetAutoTunningParametersError):
             logger.exception("Failed to get available memory or auto tuning parameters")

tests/unit/test_mysql.py

@@ -1805,6 +1805,67 @@ def test_hold_if_recovering(self, mock_get_member_state):
         self.mysql.hold_if_recovering()
         self.assertEqual(mock_get_member_state.call_count, 1)
 
+    @patch("charms.mysql.v0.mysql.MySQLBase.get_available_memory")
+    def test_render_mysqld_configuration(self, _get_available_memory):
+        """Test render_mysqld_configuration."""
+        # 32GB of memory, production profile
+        _get_available_memory.return_value = 32341442560
+
+        expected_config = {
+            "bind-address": "0.0.0.0",
+            "mysqlx-bind-address": "0.0.0.0",
+            "report_host": "127.0.0.1",
+            "max_connections": "724",
+            "innodb_buffer_pool_size": "23219666944",
+            "log_error_services": "log_filter_internal;log_sink_internal",
+            "log_error": "/var/log/mysql/error.log",
+            "general_log": "ON",
+            "general_log_file": "/var/log/mysql/general.log",
+            "slow_query_log_file": "/var/log/mysql/slowquery.log",
+            "innodb_buffer_pool_chunk_size": "2902458368",
+        }
+
+        _, rendered_config = self.mysql.render_mysqld_configuration(profile="production")
+        self.assertEqual(rendered_config, expected_config)
+
+        # < 2GB of memory, production profile
+        memory_limit = 2147483600
+
+        expected_config["innodb_buffer_pool_size"] = "536870912"
+        del expected_config["innodb_buffer_pool_chunk_size"]
+        expected_config["performance-schema-instrument"] = "'memory/%=OFF'"
+        expected_config["max_connections"] = "127"
+
+        _, rendered_config = self.mysql.render_mysqld_configuration(
+            profile="production", memory_limit=memory_limit
+        )
+        self.assertEqual(rendered_config, expected_config)
+
+        # testing profile
+        expected_config["innodb_buffer_pool_size"] = "20971520"
+        expected_config["innodb_buffer_pool_chunk_size"] = "1048576"
+        expected_config["loose-group_replication_message_cache_size"] = "134217728"
+        expected_config["max_connections"] = "100"
+
+        _, rendered_config = self.mysql.render_mysqld_configuration(profile="testing")
+        self.assertEqual(rendered_config, expected_config)
+
+        # 10GB, max connections set by value
+        memory_limit = 10106700800
+        # max_connections set
+        _, rendered_config = self.mysql.render_mysqld_configuration(
+            profile="production", experimental_max_connections=500, memory_limit=memory_limit
+        )
+
+        self.assertEqual(rendered_config["max_connections"], "500")
+
+        # max_connections set,constrained by memory, but enforced
+        _, rendered_config = self.mysql.render_mysqld_configuration(
+            profile="production", experimental_max_connections=800, memory_limit=memory_limit
+        )
+
+        self.assertEqual(rendered_config["max_connections"], "800")
+
     @patch("charms.mysql.v0.mysql.MySQLBase._run_mysqlsh_script")
     def test_create_replica_cluster(self, _run_mysqlsh_script):
         """Test create_replica_cluster."""