DPE-3547 bump floor value for max_connections (#398)

* couple optimizations (mainly for k8s)

* fix floor value

* fix typo

Author: paulomach

lib/charms/mysql/v0/mysql.py

@@ -111,7 +111,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 = 54
+LIBPATCH = 55
 
 UNIT_TEARDOWN_LOCKNAME = "unit-teardown"
 UNIT_ADD_LOCKNAME = "unit-add"
@@ -122,6 +122,7 @@ 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
 
 SECRET_INTERNAL_LABEL = "secret-id"
 SECRET_DELETED_LABEL = "None"
@@ -710,7 +711,7 @@ def render_mysqld_configuration(
             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 = 20
+            max_connections = MIN_MAX_CONNECTIONS
             performance_schema_instrument = "'memory/%=OFF'"
         else:
             available_memory = self.get_available_memory()
@@ -723,7 +724,7 @@ def render_mysqld_configuration(
                 innodb_buffer_pool_chunk_size,
                 group_replication_message_cache_size,
             ) = self.get_innodb_buffer_pool_parameters(available_memory)
-            max_connections = self.get_max_connections(available_memory)
+            max_connections = max(self.get_max_connections(available_memory), MIN_MAX_CONNECTIONS)
             if available_memory < 2 * BYTES_1GiB:
                 # disable memory instruments if we have less than 2GiB of RAM
                 performance_schema_instrument = "'memory/%=OFF'"
@@ -1209,7 +1210,11 @@ def initialize_juju_units_operations_table(self) -> None:
             raise MySQLInitializeJujuOperationsTableError(e.message)
 
     def add_instance_to_cluster(
-        self, instance_address: str, instance_unit_label: str, from_instance: Optional[str] = None
+        self,
+        instance_address: str,
+        instance_unit_label: str,
+        from_instance: Optional[str] = None,
+        method: str = "auto",
     ) -> None:
         """Add an instance to the InnoDB cluster.
 
@@ -1223,6 +1228,7 @@ def add_instance_to_cluster(
             instance_address: address of the instance to add to the cluster
             instance_unit_label: the label/name of the unit
             from_instance: address of the adding instance, e.g. primary
+            method: recovery method to use, either "auto" or "clone"
         """
         options = {
             "password": self.cluster_admin_password,
@@ -1243,39 +1249,37 @@ def add_instance_to_cluster(
             "shell.options['dba.restartWaitTimeout'] = 3600",
         )
 
-        for recovery_method in ["auto", "clone"]:
-            # Prefer "auto" recovery method, but if it fails, try "clone"
-            try:
-                options["recoveryMethod"] = recovery_method
-                add_instance_command = (
-                    f"cluster.add_instance('{self.cluster_admin_user}@{instance_address}', {json.dumps(options)})",
-                )
+        # Prefer "auto" recovery method, but if it fails, try "clone"
+        try:
+            options["recoveryMethod"] = method
+            add_instance_command = (
+                f"cluster.add_instance('{self.cluster_admin_user}@{instance_address}', {options})",
+            )
 
-                logger.debug(
-                    f"Adding instance {instance_address}/{instance_unit_label} to cluster {self.cluster_name} with recovery method {recovery_method}"
-                )
-                self._run_mysqlsh_script("\n".join(connect_commands + add_instance_command))
+            logger.info(
+                f"Adding instance {instance_address}/{instance_unit_label} to {self.cluster_name=}"
+                f"with recovery {method=}"
+            )
+            self._run_mysqlsh_script("\n".join(connect_commands + add_instance_command))
 
-                break
-            except MySQLClientError as e:
-                if recovery_method == "clone":
-                    logger.exception(
-                        f"Failed to add instance {instance_address} to cluster {self.cluster_name} on {self.instance_address}",
-                        exc_info=e,
-                    )
-                    self._release_lock(
-                        from_instance or self.instance_address,
-                        instance_unit_label,
-                        UNIT_ADD_LOCKNAME,
-                    )
-                    raise MySQLAddInstanceToClusterError(e.message)
-
-                logger.debug(
-                    f"Failed to add instance {instance_address} to cluster {self.cluster_name} with recovery method 'auto'. Trying method 'clone'"
+        except MySQLClientError:
+            if method == "clone":
+                logger.exception(
+                    f"Failed to add {instance_address=} to {self.cluster_name=} on {self.instance_address=}",
                 )
-        self._release_lock(
-            from_instance or self.instance_address, instance_unit_label, UNIT_ADD_LOCKNAME
-        )
+                raise MySQLAddInstanceToClusterError
+
+            logger.debug(
+                f"Cannot add {instance_address=} to {self.cluster_name=} with recovery {method=}. Trying method 'clone'"
+            )
+            self.add_instance_to_cluster(
+                instance_address, instance_unit_label, from_instance, method="clone"
+            )
+        finally:
+            # always release the lock
+            self._release_lock(
+                from_instance or self.instance_address, instance_unit_label, UNIT_ADD_LOCKNAME
+            )
 
     def is_instance_configured_for_innodb(
         self, instance_address: str, instance_unit_label: str
@@ -1398,6 +1402,11 @@ def is_instance_in_cluster(self, unit_label: str) -> bool:
             )
             return False
 
+    @retry(
+        wait=wait_fixed(2),
+        stop=stop_after_attempt(3),
+        retry=retry_if_exception_type(TimeoutError),
+    )
     def get_cluster_status(self, extended: Optional[bool] = False) -> Optional[dict]:
         """Get the cluster status.
 
@@ -2505,13 +2514,19 @@ def get_pid_of_port_3306(self) -> Optional[str]:
         except MySQLExecError:
             return None
 
-    def flush_mysql_logs(self, logs_type: MySQLTextLogs) -> None:
+    def flush_mysql_logs(self, logs_type: Union[MySQLTextLogs, list[MySQLTextLogs]]) -> None:
         """Flushes the specified logs_type logs."""
-        flush_logs_commands = (
+        flush_logs_commands = [
             f"shell.connect('{self.server_config_user}:{self.server_config_password}@{self.instance_address}')",
             'session.run_sql("SET sql_log_bin = 0")',
-            f'session.run_sql("FLUSH {logs_type.value}")',
-        )
+        ]
+
+        if type(logs_type) is list:
+            flush_logs_commands.extend(
+                [f"session.run_sql('FLUSH {log.value}')" for log in logs_type]
+            )
+        else:
+            flush_logs_commands.append(f'session.run_sql("FLUSH {logs_type.value}")')  # type: ignore
 
         try:
             self._run_mysqlsh_script("\n".join(flush_logs_commands))

src/flush_mysql_logs.py

@@ -42,7 +42,7 @@ def __init__(self, charm: "MySQLOperatorCharm"):
 
     def _flush_mysql_logs(self, _) -> None:
         """Flush the specified (via LOGS_TYPE env var) mysql logs."""
-        logs_type = os.environ.get("LOGS_TYPE")
+        logs_type = os.environ.get("LOGS_TYPE", "")
 
         try:
             text_logs = MySQLTextLogs[logs_type]

tests/integration/high_availability/test_replication.py

@@ -123,7 +123,7 @@ async def test_custom_variables(ops_test: OpsTest, mysql_charm_series) -> None:
 
     for unit in application.units:
         custom_vars = {}
-        custom_vars["max_connections"] = 20
+        custom_vars["max_connections"] = 100
         for k, v in custom_vars.items():
             logger.info(f"Checking that {k} is set to {v} on {unit.name}")
             value = await retrieve_database_variable_value(ops_test, unit, k)

tests/unit/test_mysql.py

@@ -370,16 +370,18 @@ def test_create_cluster_set_exceptions(self, _run_mysqlsh_script):
     def test_add_instance_to_cluster(self, _run_mysqlsh_script, _acquire_lock, _release_lock):
         """Test a successful execution of create_cluster."""
         add_instance_to_cluster_commands = (
-            "shell.connect('clusteradmin:clusteradminpassword@127.0.0.1')",
-            "cluster = dba.get_cluster('test_cluster')",
-            "shell.options['dba.restartWaitTimeout'] = 3600",
-            "cluster.add_instance('clusteradmin@127.0.0.2', {\"password\": "
-            '"clusteradminpassword", "label": "mysql-1", "recoveryMethod": "auto"})',
+            "shell.connect('clusteradmin:clusteradminpassword@127.0.0.1')\n"
+            "cluster = dba.get_cluster('test_cluster')\n"
+            "shell.options['dba.restartWaitTimeout'] = 3600\n"
+            "cluster.add_instance('clusteradmin@127.0.0.2', {'password': 'clusteradminpassword',"
+            " 'label': 'mysql-1', 'recoveryMethod': 'auto'})"
         )
 
         self.mysql.add_instance_to_cluster("127.0.0.2", "mysql-1")
 
-        _run_mysqlsh_script.assert_called_once_with("\n".join(add_instance_to_cluster_commands))
+        _run_mysqlsh_script.assert_called_once_with(add_instance_to_cluster_commands)
+        _acquire_lock.assert_called_once()
+        _release_lock.assert_called_once()
 
     @patch("charms.mysql.v0.mysql.MySQLBase._release_lock")
     @patch("charms.mysql.v0.mysql.MySQLBase._acquire_lock", return_value=True)
@@ -392,6 +394,9 @@ def test_add_instance_to_cluster_exception(
 
         with self.assertRaises(MySQLAddInstanceToClusterError):
             self.mysql.add_instance_to_cluster("127.0.0.2", "mysql-1")
+            _acquire_lock.assert_called_once()
+            _release_lock.assert_called_once()
+            _run_mysqlsh_script.assert_called()
 
     @patch(
         "charms.mysql.v0.mysql.MySQLBase._run_mysqlsh_script", return_value="INSTANCE_CONFIGURED"

tests/unit/test_mysqlsh_helpers.py

@@ -305,7 +305,7 @@ def test_write_mysqld_config(
                 "bind-address = 0.0.0.0",
                 "mysqlx-bind-address = 0.0.0.0",
                 "report_host = 127.0.0.1",
-                "max_connections = 20",
+                "max_connections = 100",
                 "innodb_buffer_pool_size = 20971520",
                 "log_error_services = log_filter_internal;log_sink_internal",
                 "log_error = /var/snap/charmed-mysql/common/var/log/mysql/error.log",