From 665bdaefc13f3e91a3db557d615b4b96eed3b47c Mon Sep 17 00:00:00 2001 From: Petr Malik Date: Fri, 29 Apr 2016 15:34:51 -0400 Subject: [PATCH] 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 --- ...cate-long-query-time-b85af24772e2e7cb.yaml | 12 +++++++++ trove/common/cfg.py | 25 +++++++++++++++---- trove/configuration/service.py | 2 ++ trove/guestagent/datastore/manager.py | 14 +++++++---- trove/templates/mariadb/validation-rules.json | 6 +++++ trove/templates/mysql/validation-rules.json | 6 +++++ trove/templates/percona/validation-rules.json | 6 +++++ .../postgresql/validation-rules.json | 6 +++++ trove/templates/pxc/validation-rules.json | 6 +++++ trove/tests/scenario/helpers/mysql_helper.py | 3 ++- .../scenario/helpers/postgresql_helper.py | 3 ++- .../scenario/runners/configuration_runners.py | 11 +++++++- 12 files changed, 87 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/deprecate-long-query-time-b85af24772e2e7cb.yaml diff --git a/releasenotes/notes/deprecate-long-query-time-b85af24772e2e7cb.yaml b/releasenotes/notes/deprecate-long-query-time-b85af24772e2e7cb.yaml new file mode 100644 index 0000000000..2efdb77d6f --- /dev/null +++ b/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 diff --git a/trove/common/cfg.py b/trove/common/cfg.py index 9b8876c5a3..d404a37e30 100644 --- a/trove/common/cfg.py +++ b/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, diff --git a/trove/configuration/service.py b/trove/configuration/service.py index fc68146d81..19dd51c3b6 100644 --- a/trove/configuration/service.py +++ b/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 " diff --git a/trove/guestagent/datastore/manager.py b/trove/guestagent/datastore/manager.py index 36273212d0..0fd010f0d6 100644 --- a/trove/guestagent/datastore/manager.py +++ b/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: diff --git a/trove/templates/mariadb/validation-rules.json b/trove/templates/mariadb/validation-rules.json index c5c4111c5a..abebfe1894 100644 --- a/trove/templates/mariadb/validation-rules.json +++ b/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" } ] } diff --git a/trove/templates/mysql/validation-rules.json b/trove/templates/mysql/validation-rules.json index c5c4111c5a..abebfe1894 100644 --- a/trove/templates/mysql/validation-rules.json +++ b/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" } ] } diff --git a/trove/templates/percona/validation-rules.json b/trove/templates/percona/validation-rules.json index acf0019990..f152558872 100644 --- a/trove/templates/percona/validation-rules.json +++ b/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" } ] } \ No newline at end of file diff --git a/trove/templates/postgresql/validation-rules.json b/trove/templates/postgresql/validation-rules.json index 702158eac2..9a285eb7d3 100644 --- a/trove/templates/postgresql/validation-rules.json +++ b/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" } ] } diff --git a/trove/templates/pxc/validation-rules.json b/trove/templates/pxc/validation-rules.json index 8b7153fcea..c1d8a97012 100644 --- a/trove/templates/pxc/validation-rules.json +++ b/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" } ] } diff --git a/trove/tests/scenario/helpers/mysql_helper.py b/trove/tests/scenario/helpers/mysql_helper.py index fcd0ba96b3..7fada6817b 100644 --- a/trove/tests/scenario/helpers/mysql_helper.py +++ b/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'}] diff --git a/trove/tests/scenario/helpers/postgresql_helper.py b/trove/tests/scenario/helpers/postgresql_helper.py index ffa9b8a5c3..5b0c37b27c 100644 --- a/trove/tests/scenario/helpers/postgresql_helper.py +++ b/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} diff --git a/trove/tests/scenario/runners/configuration_runners.py b/trove/tests/scenario/runners/configuration_runners.py index 6d0a81019a..916c6813fa 100644 --- a/trove/tests/scenario/runners/configuration_runners.py +++ b/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):