Deprecate 'guest_log_long_query_time'

Slow query time should be configurable on per-instance basis
via the existing configuration group mechanism (see the bug).

We need to deprecate the existing global conf property first.
The property has been deprecated on all datastores.

Validation rules have been updated on those that
support log retrieval as well:

  - MySQL: long_query_time
  - Percona: long_query_time
  - Percona XtraDB Cluster: long_query_time
  - PostgreSQL: log_min_duration_statement
  - MariaDB: long_query_time

The logging settings will be saved with lower 'priority' than
any user settings. This will ensure the user settings
(if present) always override the global value.
Once the deprecated options go away we can again apply
the logging settings at the system level.

Change-Id: I9c28ceb933b6819b8e96556584b2e26cab2155fb
DocImpact: Document deprecated properties
Partial-Bug: 1542485

releasenotes/notes/deprecate-long-query-time-b85af24772e2e7cb.yaml

@@ -0,0 +1,12 @@
+---
+deprecations:
+  - Make 'long query time' manageable via
+    configuration groups (see bug 1542485).
+    Deprecate the global 'guest_log_long_query_time'
+    option in preference of datastore-specific
+    configurations.
+    MySQL long_query_time
+    Percona long_query_time
+    Percona XtraDB Cluster long_query_time
+    MariaDB long_query_time
+    PostgreSQL log_min_duration_statement

trove/common/cfg.py

@@ -548,7 +548,10 @@ mysql_opts = [
                help='List of Guest Logs to expose for publishing.'),
     cfg.IntOpt('guest_log_long_query_time', default=1000,
                help='The time in milliseconds that a statement must take in '
-                    'in order to be logged in the slow_query log.'),
+                    'in order to be logged in the slow_query log.',
+               deprecated_for_removal=True,
+               deprecated_reason='Will be replaced by a configuration group '
+               'option: long_query_time'),
     cfg.IntOpt('default_password_length', default=36,
                help='Character length of generated passwords.',
                deprecated_name='default_password_length',
@@ -632,7 +635,10 @@ percona_opts = [
                help='List of Guest Logs to expose for publishing.'),
     cfg.IntOpt('guest_log_long_query_time', default=1000,
                help='The time in milliseconds that a statement must take in '
-                    'in order to be logged in the slow_query log.'),
+                    'in order to be logged in the slow_query log.',
+               deprecated_for_removal=True,
+               deprecated_reason='Will be replaced by a configuration group '
+               'option: long_query_time'),
     cfg.IntOpt('default_password_length',
                default='${mysql.default_password_length}',
                help='Character length of generated passwords.',
@@ -721,7 +727,10 @@ pxc_opts = [
                help='List of Guest Logs to expose for publishing.'),
     cfg.IntOpt('guest_log_long_query_time', default=1000,
                help='The time in milliseconds that a statement must take in '
-                    'in order to be logged in the slow_query log.'),
+                    'in order to be logged in the slow_query log.',
+               deprecated_for_removal=True,
+               deprecated_reason='Will be replaced by a configuration group '
+               'option: long_query_time'),
     cfg.IntOpt('default_password_length',
                default='${mysql.default_password_length}',
                help='Character length of generated passwords.',
@@ -1102,7 +1111,10 @@ postgresql_opts = [
                help="The time in milliseconds that a statement must take in "
                     "in order to be logged in the 'general' log.  A value of "
                     "'0' logs all statements, while '-1' turns off "
-                    "statement logging."),
+                    "statement logging.",
+               deprecated_for_removal=True,
+               deprecated_reason='Will be replaced by configuration group '
+               'option: log_min_duration_statement'),
     cfg.IntOpt('default_password_length', default=36,
                help='Character length of generated passwords.',
                deprecated_name='default_password_length',
@@ -1378,7 +1390,10 @@ mariadb_opts = [
                help='List of Guest Logs to expose for publishing.'),
     cfg.IntOpt('guest_log_long_query_time', default=1000,
                help='The time in milliseconds that a statement must take in '
-                    'in order to be logged in the slow_query log.'),
+                    'in order to be logged in the slow_query log.',
+               deprecated_for_removal=True,
+               deprecated_reason='Will be replaced by a configuration group '
+               'option: long_query_time'),
     cfg.BoolOpt('cluster_support', default=True,
                 help='Enable clusters to be created and managed.'),
     cfg.IntOpt('min_cluster_member_count', default=3,

trove/configuration/service.py

@@ -311,6 +311,8 @@ class ConfigurationsController(wsgi.Controller):
             return six.string_types
         elif value_type == "integer":
             return six.integer_types
+        elif value_type == "float":
+            return float
         else:
             raise exception.TroveError(_(
                 "Invalid or unsupported type defined in the "

trove/guestagent/datastore/manager.py

@@ -453,8 +453,8 @@ class Manager(periodic_task.PeriodicTasks):
         self.guest_log_context = context
         gl_cache = self.guest_log_cache
         result = filter(None, [gl_cache[log_name].show()
-                        if gl_cache[log_name].exposed else None
-                        for log_name in gl_cache.keys()])
+                               if gl_cache[log_name].exposed else None
+                               for log_name in gl_cache.keys()])
         LOG.info(_("Returning list of logs: %s") % result)
         return result
 
@@ -467,7 +467,7 @@ class Manager(periodic_task.PeriodicTasks):
         if publish and not disable:
             enable = True
         LOG.info(_("Processing guest log '%(log)s' "
-                 "(enable=%(en)s, disable=%(dis)s, "
+                   "(enable=%(en)s, disable=%(dis)s, "
                    "publish=%(pub)s, discard=%(disc)s).") %
                  {'log': log_name, 'en': enable, 'dis': disable,
                   'pub': publish, 'disc': discard})
@@ -554,8 +554,12 @@ class Manager(periodic_task.PeriodicTasks):
             config_man_values = cfg_values
             if section_label:
                 config_man_values = {section_label: cfg_values}
-            self.configuration_manager.apply_system_override(
-                config_man_values, change_id=apply_label)
+            # Applying the changes with a group id lower than the one used
+            # by user overrides. Any user defined value will override these
+            # settings (irrespective of order in which they are applied).
+            # See Bug 1542485
+            self.configuration_manager._apply_override(
+                '10-system-low-priority', apply_label, config_man_values)
         if restart_required:
             self.status.set_status(instance.ServiceStatuses.RESTART_REQUIRED)
         else:

trove/templates/mariadb/validation-rules.json

@@ -231,6 +231,12 @@
             "name": "performance_schema",
             "restart_required": true,
             "type": "boolean"
+        },
+        {
+            "name": "long_query_time",
+            "restart_required": false,
+            "min": 0,
+            "type": "float"
         }
     ]
 }

trove/templates/mysql/validation-rules.json

@@ -231,6 +231,12 @@
             "name": "performance_schema",
             "restart_required": true,
             "type": "boolean"
+        },
+        {
+            "name": "long_query_time",
+            "restart_required": false,
+            "min": 0,
+            "type": "float"
         }
     ]
 }

trove/templates/percona/validation-rules.json

@@ -219,6 +219,12 @@
             "name": "collation_server",
             "restart_required": false,
             "type": "string"
+        },
+        {
+            "name": "long_query_time",
+            "restart_required": false,
+            "min": 0,
+            "type": "float"
         }
     ]
 }

trove/templates/postgresql/validation-rules.json

@@ -898,6 +898,12 @@
             "name": "restart_after_crash",
             "restart_required": false,
             "type": "boolean"
+        },
+        {
+            "name": "log_min_duration_statement",
+            "restart_required": false,
+            "min": -1,
+            "type": "integer"
         }
     ]
 }

trove/templates/pxc/validation-rules.json

@@ -219,6 +219,12 @@
             "name": "collation_server",
             "restart_required": false,
             "type": "string"
+        },
+        {
+            "name": "long_query_time",
+            "restart_required": false,
+            "min": 0,
+            "type": "float"
         }
     ]
 }

trove/tests/scenario/helpers/mysql_helper.py

@@ -46,7 +46,8 @@ class MysqlHelper(SqlHelper):
                 'join_buffer_size': 10485760}
 
     def get_non_dynamic_group(self):
-        return {'innodb_buffer_pool_size': 10485760}
+        return {'innodb_buffer_pool_size': 10485760,
+                'long_query_time': 59.1}
 
     def get_invalid_groups(self):
         return [{'key_buffer_size': -1}, {"join_buffer_size": 'string_value'}]

trove/tests/scenario/helpers/postgresql_helper.py

@@ -45,7 +45,8 @@ class PostgresqlHelper(SqlHelper):
                  'databases': [{'name': 'db1'}, {'name': 'db2'}]}]
 
     def get_dynamic_group(self):
-        return {'effective_cache_size': '528MB'}
+        return {'effective_cache_size': '528MB',
+                'log_min_duration_statement': 257}
 
     def get_non_dynamic_group(self):
         return {'max_connections': 113}

trove/tests/scenario/runners/configuration_runners.py

@@ -323,7 +323,16 @@ class ConfigurationRunner(TestRunner):
         host = self.get_instance_host(instance_id)
         for name, value in expected_configs.items():
             actual = self.test_helper.get_configuration_value(name, host)
-            self.assert_equal(str(value), str(actual),
+            # Compare floating point numbers as floats to avoid rounding
+            # and precision issues.
+            try:
+                expected_value = float(value)
+                actual_value = float(actual)
+            except ValueError:
+                expected_value = str(value)
+                actual_value = str(actual)
+
+            self.assert_equal(expected_value, actual_value,
                               "Unexpected value of property '%s'" % name)
 
     def run_list_dynamic_inst_conf_groups_after(self):