From f357dba177a61b63de5c826a6692f05b4eb25838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Thu, 23 May 2024 10:42:05 -0300 Subject: [PATCH 01/14] Automatically detect deleted resources While executing some Gnocchi optimizations (https://github.com/gnocchixyz/gnocchi/pull/1307), we noticed that some deleted/removed resources do not have the "ended_at" field with a datetime. This can cause slowness with time, as more and more "zombie" resources are left there, and this has a direct impact in the MySQL queries executed with the aggregates API. This patch introduces a new parameter called `metric_inactive_after`, which defines for how long a metric can go without receiving new datapoints until we consider it as inactive. Then, when all metrics of a resource are in inactive state, we can mark/consider the resource as removed. --- gnocchi/chef.py | 75 +++++++++++++++++++ gnocchi/cli/metricd.py | 11 +++ gnocchi/indexer/__init__.py | 6 +- ...n_for_truncate_inactive_metrics_process.py | 4 +- ...ec_create_last_measure_timestamp_column.py | 39 ++++++++++ gnocchi/indexer/sqlalchemy.py | 7 ++ gnocchi/indexer/sqlalchemy_base.py | 15 +++- gnocchi/opts.py | 14 +++- gnocchi/storage/__init__.py | 20 +++++ run-upgrade-tests.sh | 2 +- 10 files changed, 183 insertions(+), 10 deletions(-) create mode 100644 gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py diff --git a/gnocchi/chef.py b/gnocchi/chef.py index 9c71bb1fd..36ed9bf85 100644 --- a/gnocchi/chef.py +++ b/gnocchi/chef.py @@ -18,9 +18,11 @@ import daiquiri import random +import datetime from gnocchi import carbonara from gnocchi import indexer +from gnocchi import utils LOG = daiquiri.getLogger(__name__) @@ -51,6 +53,79 @@ def __init__(self, coord, incoming, index, storage): self.index = index self.storage = storage + def resource_ended_at_normalization(self, metric_inactive_after): + """Marks resources as ended at if needed. + + This method will check all metrics that have not received new + datapoints after a given period. The period is defined by + 'metric_inactive_after' parameter. If all metrics of resource are in + inactive state, we mark the ended_at field with a timestmap. Therefore, + we consider that the resource has ceased existing. + + In this process we will handle only metrics that are considered as + inactive, according to `metric_inactive_after` parameter. Therefore, + we do not need to lock these metrics while processing, as they are + inactive, and chances are that they will not receive measures anymore. + """ + + momment_now = utils.utcnow() + momment = momment_now - datetime.timedelta( + seconds=metric_inactive_after) + + inactive_metrics = self.index.list_metrics( + attribute_filter={"<": { + "last_measure_timestamp": momment}}, + resource_policy_filter={"==": {"ended_at": None}} + ) + + LOG.debug("Inactive metrics found for processing: [%s].", + inactive_metrics) + + metrics_by_resource_id = {} + for metric in inactive_metrics: + resource_id = metric.resource_id + if metrics_by_resource_id.get(resource_id) is None: + metrics_by_resource_id[resource_id] = [] + + metrics_by_resource_id[resource_id].append(metric) + + for resource_id in metrics_by_resource_id.keys(): + if resource_id is None: + LOG.debug("We do not need to process inactive metrics that do " + "not have resource. Therefore, these metrics [%s] " + "will be considered inactive, but there is nothing " + "else we can do in this process.", + metrics_by_resource_id[resource_id]) + continue + resource = self.index.get_resource( + "generic", resource_id, with_metrics=True) + resource_metrics = resource.metrics + resource_inactive_metrics = metrics_by_resource_id.get(resource_id) + + all_metrics_are_inactive = True + for m in resource_metrics: + if m not in resource_inactive_metrics: + all_metrics_are_inactive = False + LOG.debug("Not all metrics of resource [%s] are inactive. " + "Metric [%s] is not inactive. The inactive " + "metrics are [%s].", + resource, m, resource_inactive_metrics) + break + + if all_metrics_are_inactive: + LOG.info("All metrics [%s] of resource [%s] are inactive." + "Therefore, we will mark it as finished with an" + "ended at timestmap.", resource_metrics, resource) + if resource.ended_at is not None: + LOG.debug( + "Resource [%s] already has an ended at value.", resource) + else: + LOG.info("Marking ended at timestamp for resource " + "[%s] because all of its metrics are inactive.", + resource) + self.index.update_resource( + "generic", resource_id, ended_at=momment_now) + def clean_raw_data_inactive_metrics(self): """Cleans metrics raw data if they are inactive. diff --git a/gnocchi/cli/metricd.py b/gnocchi/cli/metricd.py index a22e1646b..f3c488a1f 100644 --- a/gnocchi/cli/metricd.py +++ b/gnocchi/cli/metricd.py @@ -278,6 +278,17 @@ def _run_job(self): LOG.debug("Finished the cleaning of raw data points for metrics that " "are no longer receiving measures.") + if (self.conf.metricd.metric_inactive_after and + self.conf.metricd.metric_inactive_after > 0): + LOG.debug("Starting resource ended at field normalization.") + self.chef.resource_ended_at_normalization( + self.conf.metricd.metric_inactive_after) + LOG.debug("Finished resource ended at field normalization.") + else: + LOG.debug("Resource ended at field normalization is not " + "activated. See 'metric_inactive_after' parameter if " + "you wish to activate it.") + class MetricdServiceManager(cotyledon.ServiceManager): def __init__(self, conf): diff --git a/gnocchi/indexer/__init__.py b/gnocchi/indexer/__init__.py index 1949cd63a..7416948e8 100644 --- a/gnocchi/indexer/__init__.py +++ b/gnocchi/indexer/__init__.py @@ -446,7 +446,11 @@ def update_backwindow_changed_for_metrics_archive_policy( raise exceptions.NotImplementedError @staticmethod - def update_needs_raw_data_truncation(metric_id): + def update_needs_raw_data_truncation(metric_id, value): + raise exceptions.NotImplementedError + + @staticmethod + def update_last_measure_timestmap(metric_id): raise exceptions.NotImplementedError @staticmethod diff --git a/gnocchi/indexer/alembic/versions/18fff4509e3e_create_column_for_truncate_inactive_metrics_process.py b/gnocchi/indexer/alembic/versions/18fff4509e3e_create_column_for_truncate_inactive_metrics_process.py index d67bb6064..18a2f1910 100644 --- a/gnocchi/indexer/alembic/versions/18fff4509e3e_create_column_for_truncate_inactive_metrics_process.py +++ b/gnocchi/indexer/alembic/versions/18fff4509e3e_create_column_for_truncate_inactive_metrics_process.py @@ -13,17 +13,15 @@ # under the License. # -"""create metric truncation status column +"""Create metric truncation status column Revision ID: 18fff4509e3e Revises: 04eba72e4f90 Create Date: 2024-04-24 09:16:00 """ -import datetime from alembic import op -from sqlalchemy.sql import func import sqlalchemy diff --git a/gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py b/gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py new file mode 100644 index 000000000..4f503e779 --- /dev/null +++ b/gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py @@ -0,0 +1,39 @@ +# Copyright 2015 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""Create last measure push timestamp column +Revision ID: f89ed2e3c2ec +Revises: 18fff4509e3e +Create Date: 2024-04-24 09:16:00 +""" + +from alembic import op + +import sqlalchemy + +from sqlalchemy.sql import func + +# revision identifiers, used by Alembic. +revision = 'f89ed2e3c2ec' +down_revision = '18fff4509e3e' +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + "metric", sqlalchemy.Column( + "last_measure_timestamp", sqlalchemy.DateTime, + nullable=False, server_default=func.current_timestamp())) diff --git a/gnocchi/indexer/sqlalchemy.py b/gnocchi/indexer/sqlalchemy.py index 4028bca90..57b811ffb 100644 --- a/gnocchi/indexer/sqlalchemy.py +++ b/gnocchi/indexer/sqlalchemy.py @@ -1403,6 +1403,13 @@ def update_needs_raw_data_truncation(self, metrid_id, value=False): if session.execute(stmt).rowcount == 0: raise indexer.NoSuchMetric(metrid_id) + def update_last_measure_timestmap(self, metrid_id): + with self.facade.writer() as session: + stmt = update(Metric).filter(Metric.id == metrid_id).values( + last_measure_timestamp=datetime.datetime.utcnow()) + if session.execute(stmt).rowcount == 0: + raise indexer.NoSuchMetric(metrid_id) + def update_backwindow_changed_for_metrics_archive_policy( self, archive_policy_name): with self.facade.writer() as session: diff --git a/gnocchi/indexer/sqlalchemy_base.py b/gnocchi/indexer/sqlalchemy_base.py index 0d704f35c..470850da2 100644 --- a/gnocchi/indexer/sqlalchemy_base.py +++ b/gnocchi/indexer/sqlalchemy_base.py @@ -19,6 +19,7 @@ import sqlalchemy from sqlalchemy.ext import declarative from sqlalchemy.orm import declarative_base +from sqlalchemy.sql import func import sqlalchemy_utils @@ -114,6 +115,14 @@ class Metric(Base, GnocchiBase, indexer.Metric): nullable=False, default=True, server_default=sqlalchemy.sql.true()) + # Timestamp that represents when the last measure push was received for the + # given metric. This allows us to identify when a metric ceased receiving + # measurements; thus, if all metric for a resource are in this situation, + # chances are that the resource ceased existing in the backend. + last_measure_timestamp = sqlalchemy.Column( + "last_measure_timestamp", sqlalchemy.DateTime, + nullable=False, server_default=func.current_timestamp()) + def jsonify(self): d = { "id": self.id, @@ -257,7 +266,8 @@ def type(cls): creator = sqlalchemy.Column(sqlalchemy.String(255)) started_at = sqlalchemy.Column(types.TimestampUTC, nullable=False, default=lambda: utils.utcnow()) - revision_start = sqlalchemy.Column(types.TimestampUTC, nullable=False, + revision_start = sqlalchemy.Column(types.TimestampUTC, + nullable=False, default=lambda: utils.utcnow()) ended_at = sqlalchemy.Column(types.TimestampUTC) user_id = sqlalchemy.Column(sqlalchemy.String(255)) @@ -299,7 +309,8 @@ class ResourceHistory(ResourceMixin, Base, GnocchiBase): ondelete="CASCADE", name="fk_rh_id_resource_id"), nullable=False) - revision_end = sqlalchemy.Column(types.TimestampUTC, nullable=False, + revision_end = sqlalchemy.Column(types.TimestampUTC, + nullable=False, default=lambda: utils.utcnow()) metrics = sqlalchemy.orm.relationship( Metric, primaryjoin="Metric.resource_id == ResourceHistory.id", diff --git a/gnocchi/opts.py b/gnocchi/opts.py index d51da43b0..4bb5d6505 100644 --- a/gnocchi/opts.py +++ b/gnocchi/opts.py @@ -62,7 +62,6 @@ def __getitem__(self, key): for opt in _INCOMING_OPTS: opt.default = '${storage.%s}' % opt.name - API_OPTS = ( cfg.HostAddressOpt('host', default="0.0.0.0", @@ -78,7 +77,7 @@ def __getitem__(self, key): but not chunked encoding (InfluxDB) * http-socket/socket: support chunked encoding, but require a upstream HTTP Server for HTTP/1.1, keepalive and HTTP protocol correctness. -""") +"""), ) @@ -177,7 +176,16 @@ def list_opts(): default=10000, min=1, help="Number of metrics that should be deleted " - "simultaneously by one janitor.") + "simultaneously by one janitor."), + cfg.IntOpt('metric_inactive_after', + default=0, + help="Number of seconds to wait before we consider a " + "metric inactive. An inactive metric is a metric " + "that has not received new measurements for a " + "given period. If all metrics of a resource are " + "inactive, we mark the resource with the " + "'ended_at' timestamp. The default is 0 (zero), " + "which means that we never execute process.") )), ("api", ( cfg.StrOpt('paste_config', diff --git a/gnocchi/storage/__init__.py b/gnocchi/storage/__init__.py index e41d150a4..04c218a28 100644 --- a/gnocchi/storage/__init__.py +++ b/gnocchi/storage/__init__.py @@ -688,6 +688,26 @@ def _map_compute_splits_operations(bound_timeserie): if metric.needs_raw_data_truncation: indexer_driver.update_needs_raw_data_truncation(metric.id) + # Mark when the metric receives its latest measures + indexer_driver.update_last_measure_timestmap(metric.id) + + resource_id = metric.resource_id + if resource_id: + resource = indexer_driver.get_resource('generic', resource_id) + LOG.debug("Checking if resource [%s] of metric [%s] with " + "resource ID [%s] needs to be 'undeleted.'", + resource, metric.id, resource_id) + if resource.ended_at is not None: + LOG.info("Resource [%s] was marked with a timestamp for the " + "'ended_at' field. However, it received a " + "measurement for metric [%s]. Therefore, we undelete " + "it.", resource, metric) + indexer_driver.update_resource( + "generic", resource_id, ended_at=None) + else: + LOG.debug("Metric [%s] does not have a resource " + "assigned to it.", metric) + with self.statistics.time("splits delete"): self._delete_metric_splits(splits_to_delete) self.statistics["splits delete"] += len(splits_to_delete) diff --git a/run-upgrade-tests.sh b/run-upgrade-tests.sh index c9b7456a8..40835e742 100755 --- a/run-upgrade-tests.sh +++ b/run-upgrade-tests.sh @@ -107,7 +107,7 @@ export GNOCCHI_USER=$GNOCCHI_USER_ID # needs to be released. Otherwise, the logs stop to be writen, and the # execution of the code is "frozen", due to the lack of buffer in the # process output. To work around that, we can read the buffer, and dump it -# into a lof file. Then, we can cat the log file content at the end of the +# into a log file. Then, we can cat the log file content at the end of the # process. UWSGI_LOG_FILE=/tmp/uwsgi-new-version.log METRICD_LOG_FILE=/tmp/gnocchi-metricd-new-version.log From 1bffac0bdbc2af0e62d962945837db97392c2973 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Wed, 3 Jul 2024 12:19:45 -0300 Subject: [PATCH 02/14] Fix pep8 --- gnocchi/chef.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnocchi/chef.py b/gnocchi/chef.py index 36ed9bf85..f3208cbdc 100644 --- a/gnocchi/chef.py +++ b/gnocchi/chef.py @@ -17,8 +17,8 @@ import hashlib import daiquiri -import random import datetime +import random from gnocchi import carbonara from gnocchi import indexer From 0df5c2c3808fb1a68478e96e4cac8a47527da7c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Wed, 3 Jul 2024 12:32:38 -0300 Subject: [PATCH 03/14] add some comment --- gnocchi/chef.py | 1 + 1 file changed, 1 insertion(+) diff --git a/gnocchi/chef.py b/gnocchi/chef.py index f3208cbdc..8195360d4 100644 --- a/gnocchi/chef.py +++ b/gnocchi/chef.py @@ -66,6 +66,7 @@ def resource_ended_at_normalization(self, metric_inactive_after): inactive, according to `metric_inactive_after` parameter. Therefore, we do not need to lock these metrics while processing, as they are inactive, and chances are that they will not receive measures anymore. + Moreover, we are only touching metadata, and not the actual data. """ momment_now = utils.utcnow() From 65a087fee70b2511bce6f42ab5ded6e9633f343f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Wed, 3 Jul 2024 15:39:11 -0300 Subject: [PATCH 04/14] address Pedro's review --- gnocchi/chef.py | 14 +++++++------- gnocchi/opts.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/gnocchi/chef.py b/gnocchi/chef.py index 8195360d4..ede2053fb 100644 --- a/gnocchi/chef.py +++ b/gnocchi/chef.py @@ -82,26 +82,26 @@ def resource_ended_at_normalization(self, metric_inactive_after): LOG.debug("Inactive metrics found for processing: [%s].", inactive_metrics) - metrics_by_resource_id = {} + inactive_metrics_by_resource_id = {} for metric in inactive_metrics: resource_id = metric.resource_id - if metrics_by_resource_id.get(resource_id) is None: - metrics_by_resource_id[resource_id] = [] + if inactive_metrics_by_resource_id.get(resource_id) is None: + inactive_metrics_by_resource_id[resource_id] = [] - metrics_by_resource_id[resource_id].append(metric) + inactive_metrics_by_resource_id[resource_id].append(metric) - for resource_id in metrics_by_resource_id.keys(): + for resource_id in inactive_metrics_by_resource_id.keys(): if resource_id is None: LOG.debug("We do not need to process inactive metrics that do " "not have resource. Therefore, these metrics [%s] " "will be considered inactive, but there is nothing " "else we can do in this process.", - metrics_by_resource_id[resource_id]) + inactive_metrics_by_resource_id[resource_id]) continue resource = self.index.get_resource( "generic", resource_id, with_metrics=True) resource_metrics = resource.metrics - resource_inactive_metrics = metrics_by_resource_id.get(resource_id) + resource_inactive_metrics = inactive_metrics_by_resource_id.get(resource_id) all_metrics_are_inactive = True for m in resource_metrics: diff --git a/gnocchi/opts.py b/gnocchi/opts.py index 4bb5d6505..1179ffbe1 100644 --- a/gnocchi/opts.py +++ b/gnocchi/opts.py @@ -77,7 +77,7 @@ def __getitem__(self, key): but not chunked encoding (InfluxDB) * http-socket/socket: support chunked encoding, but require a upstream HTTP Server for HTTP/1.1, keepalive and HTTP protocol correctness. -"""), +""") ) From 953515ce9d2505ff14e2869dc478493dbc84fd5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Tue, 9 Jul 2024 08:35:37 -0300 Subject: [PATCH 05/14] Update gnocchi/indexer/sqlalchemy.py with @chungg review Co-authored-by: gord chung <5091603+chungg@users.noreply.github.com> --- gnocchi/indexer/sqlalchemy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnocchi/indexer/sqlalchemy.py b/gnocchi/indexer/sqlalchemy.py index 57b811ffb..66c38f89c 100644 --- a/gnocchi/indexer/sqlalchemy.py +++ b/gnocchi/indexer/sqlalchemy.py @@ -1403,7 +1403,7 @@ def update_needs_raw_data_truncation(self, metrid_id, value=False): if session.execute(stmt).rowcount == 0: raise indexer.NoSuchMetric(metrid_id) - def update_last_measure_timestmap(self, metrid_id): + def update_last_measure_timestamp(self, metrid_id): with self.facade.writer() as session: stmt = update(Metric).filter(Metric.id == metrid_id).values( last_measure_timestamp=datetime.datetime.utcnow()) From f39c6dd325d7a236c12a7458fcae679949982ed2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Tue, 9 Jul 2024 08:35:58 -0300 Subject: [PATCH 06/14] Update gnocchi/storage/__init__.py with @chungg review Co-authored-by: gord chung <5091603+chungg@users.noreply.github.com> --- gnocchi/storage/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnocchi/storage/__init__.py b/gnocchi/storage/__init__.py index 04c218a28..d03ee4a2d 100644 --- a/gnocchi/storage/__init__.py +++ b/gnocchi/storage/__init__.py @@ -695,7 +695,7 @@ def _map_compute_splits_operations(bound_timeserie): if resource_id: resource = indexer_driver.get_resource('generic', resource_id) LOG.debug("Checking if resource [%s] of metric [%s] with " - "resource ID [%s] needs to be 'undeleted.'", + "resource ID [%s] needs to be restored.", resource, metric.id, resource_id) if resource.ended_at is not None: LOG.info("Resource [%s] was marked with a timestamp for the " From 11e39fa13af1dfe6da8ec9ae85894fa94dda454c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Tue, 9 Jul 2024 08:36:46 -0300 Subject: [PATCH 07/14] Update gnocchi/chef.py with @chungg review Co-authored-by: gord chung <5091603+chungg@users.noreply.github.com> --- gnocchi/chef.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/gnocchi/chef.py b/gnocchi/chef.py index ede2053fb..5110817c4 100644 --- a/gnocchi/chef.py +++ b/gnocchi/chef.py @@ -82,12 +82,9 @@ def resource_ended_at_normalization(self, metric_inactive_after): LOG.debug("Inactive metrics found for processing: [%s].", inactive_metrics) - inactive_metrics_by_resource_id = {} + inactive_metrics_by_resource_id = collections.defaultdict(list) for metric in inactive_metrics: resource_id = metric.resource_id - if inactive_metrics_by_resource_id.get(resource_id) is None: - inactive_metrics_by_resource_id[resource_id] = [] - inactive_metrics_by_resource_id[resource_id].append(metric) for resource_id in inactive_metrics_by_resource_id.keys(): From d9f1252a1ac40c839e89968ef73e6f9022a0ec63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Tue, 9 Jul 2024 08:51:45 -0300 Subject: [PATCH 08/14] Address some other reviews from @chungg --- gnocchi/chef.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/gnocchi/chef.py b/gnocchi/chef.py index 5110817c4..c7f06b177 100644 --- a/gnocchi/chef.py +++ b/gnocchi/chef.py @@ -15,11 +15,12 @@ # See the License for the specific language governing permissions and # limitations under the License. import hashlib +import datetime import daiquiri -import datetime import random +from collections import defaultdict from gnocchi import carbonara from gnocchi import indexer from gnocchi import utils @@ -69,20 +70,20 @@ def resource_ended_at_normalization(self, metric_inactive_after): Moreover, we are only touching metadata, and not the actual data. """ - momment_now = utils.utcnow() - momment = momment_now - datetime.timedelta( + moment_now = utils.utcnow() + moment = moment_now - datetime.timedelta( seconds=metric_inactive_after) inactive_metrics = self.index.list_metrics( attribute_filter={"<": { - "last_measure_timestamp": momment}}, + "last_measure_timestamp": moment}}, resource_policy_filter={"==": {"ended_at": None}} ) LOG.debug("Inactive metrics found for processing: [%s].", inactive_metrics) - inactive_metrics_by_resource_id = collections.defaultdict(list) + inactive_metrics_by_resource_id = defaultdict(list) for metric in inactive_metrics: resource_id = metric.resource_id inactive_metrics_by_resource_id[resource_id].append(metric) @@ -122,7 +123,7 @@ def resource_ended_at_normalization(self, metric_inactive_after): "[%s] because all of its metrics are inactive.", resource) self.index.update_resource( - "generic", resource_id, ended_at=momment_now) + "generic", resource_id, ended_at=moment_now) def clean_raw_data_inactive_metrics(self): """Cleans metrics raw data if they are inactive. From f8701fc12c04c218a64fae1690c0a77828597b19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Tue, 9 Jul 2024 08:54:18 -0300 Subject: [PATCH 09/14] fix import --- gnocchi/chef.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnocchi/chef.py b/gnocchi/chef.py index c7f06b177..e0248e287 100644 --- a/gnocchi/chef.py +++ b/gnocchi/chef.py @@ -14,8 +14,8 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. -import hashlib import datetime +import hashlib import daiquiri import random From f3bca5e8aa67fe5f2ce14029dd23d50c64b02c0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Tue, 9 Jul 2024 09:02:53 -0300 Subject: [PATCH 10/14] other fixes --- gnocchi/indexer/__init__.py | 2 +- gnocchi/storage/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gnocchi/indexer/__init__.py b/gnocchi/indexer/__init__.py index 7416948e8..c9dba919d 100644 --- a/gnocchi/indexer/__init__.py +++ b/gnocchi/indexer/__init__.py @@ -450,7 +450,7 @@ def update_needs_raw_data_truncation(metric_id, value): raise exceptions.NotImplementedError @staticmethod - def update_last_measure_timestmap(metric_id): + def update_last_measure_timestamp(metric_id): raise exceptions.NotImplementedError @staticmethod diff --git a/gnocchi/storage/__init__.py b/gnocchi/storage/__init__.py index d03ee4a2d..461eccaa7 100644 --- a/gnocchi/storage/__init__.py +++ b/gnocchi/storage/__init__.py @@ -689,7 +689,7 @@ def _map_compute_splits_operations(bound_timeserie): indexer_driver.update_needs_raw_data_truncation(metric.id) # Mark when the metric receives its latest measures - indexer_driver.update_last_measure_timestmap(metric.id) + indexer_driver.update_last_measure_timestamp(metric.id) resource_id = metric.resource_id if resource_id: From f7c5e4ddc6774916d9f1204bb5aa314d29cad6a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Tue, 11 Feb 2025 13:55:18 -0300 Subject: [PATCH 11/14] Update gnocchi/storage/__init__.py Co-authored-by: gord chung <5091603+chungg@users.noreply.github.com> --- gnocchi/storage/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnocchi/storage/__init__.py b/gnocchi/storage/__init__.py index 461eccaa7..c54b96a6e 100644 --- a/gnocchi/storage/__init__.py +++ b/gnocchi/storage/__init__.py @@ -700,7 +700,7 @@ def _map_compute_splits_operations(bound_timeserie): if resource.ended_at is not None: LOG.info("Resource [%s] was marked with a timestamp for the " "'ended_at' field. However, it received a " - "measurement for metric [%s]. Therefore, we undelete " + "measurement for metric [%s]. Therefore, restoring " "it.", resource, metric) indexer_driver.update_resource( "generic", resource_id, ended_at=None) From 39dc1a3b7aabb7b702dead49e0053c655d81c339 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Thu, 13 Feb 2025 10:45:58 -0300 Subject: [PATCH 12/14] Fix broken tests with timestamp --- ...ec_create_last_measure_timestamp_column.py | 11 ++-- gnocchi/indexer/sqlalchemy_base.py | 6 +-- .../indexer/sqlalchemy/test_migrations.py | 53 ++++++++++++++++--- 3 files changed, 55 insertions(+), 15 deletions(-) diff --git a/gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py b/gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py index 4f503e779..a1837c826 100644 --- a/gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py +++ b/gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py @@ -19,12 +19,11 @@ Create Date: 2024-04-24 09:16:00 """ +import datetime from alembic import op import sqlalchemy -from sqlalchemy.sql import func - # revision identifiers, used by Alembic. revision = 'f89ed2e3c2ec' down_revision = '18fff4509e3e' @@ -33,7 +32,9 @@ def upgrade(): - op.add_column( - "metric", sqlalchemy.Column( + sql_column = sqlalchemy.Column( "last_measure_timestamp", sqlalchemy.DateTime, - nullable=False, server_default=func.current_timestamp())) + nullable=False, default=datetime.datetime.now(datetime.UTC), + server_default=sqlalchemy.sql.func.current_timestamp()) + op.add_column( + "metric", sql_column) diff --git a/gnocchi/indexer/sqlalchemy_base.py b/gnocchi/indexer/sqlalchemy_base.py index 470850da2..5a0425e0a 100644 --- a/gnocchi/indexer/sqlalchemy_base.py +++ b/gnocchi/indexer/sqlalchemy_base.py @@ -15,11 +15,11 @@ # License for the specific language governing permissions and limitations # under the License. +import datetime from oslo_db.sqlalchemy import models import sqlalchemy from sqlalchemy.ext import declarative from sqlalchemy.orm import declarative_base -from sqlalchemy.sql import func import sqlalchemy_utils @@ -120,8 +120,8 @@ class Metric(Base, GnocchiBase, indexer.Metric): # measurements; thus, if all metric for a resource are in this situation, # chances are that the resource ceased existing in the backend. last_measure_timestamp = sqlalchemy.Column( - "last_measure_timestamp", sqlalchemy.DateTime, - nullable=False, server_default=func.current_timestamp()) + "last_measure_timestamp", sqlalchemy.DateTime, default=datetime.datetime.now(datetime.UTC), nullable=False, + server_default=sqlalchemy.sql.func.current_timestamp()) def jsonify(self): d = { diff --git a/gnocchi/tests/indexer/sqlalchemy/test_migrations.py b/gnocchi/tests/indexer/sqlalchemy/test_migrations.py index b61d815ce..975a6b950 100644 --- a/gnocchi/tests/indexer/sqlalchemy/test_migrations.py +++ b/gnocchi/tests/indexer/sqlalchemy/test_migrations.py @@ -17,13 +17,15 @@ import fixtures import oslo_db.exception from oslo_db.sqlalchemy import test_migrations -import sqlalchemy.schema + +import sqlalchemy as sa import sqlalchemy_utils + from unittest import mock from gnocchi import indexer -from gnocchi.indexer import sqlalchemy -from gnocchi.indexer import sqlalchemy_base +from gnocchi.indexer import sqlalchemy as gnocchi_sqlalchemy +from gnocchi.indexer import sqlalchemy_base as gnocchi_sqlalchemy_base from gnocchi.tests import base @@ -41,7 +43,7 @@ def setUp(self): self.db = mock.Mock() self.conf.set_override( 'url', - sqlalchemy.SQLAlchemyIndexer._create_new_database( + gnocchi_sqlalchemy.SQLAlchemyIndexer._create_new_database( self.conf.indexer.url), 'indexer') self.index = indexer.get_driver(self.conf) @@ -56,10 +58,10 @@ def setUp(self): # NOTE(sileht): load it in sqlalchemy metadata self.index._RESOURCE_TYPE_MANAGER.get_classes(rt) - for table in sqlalchemy_base.Base.metadata.sorted_tables: + for table in gnocchi_sqlalchemy_base.Base.metadata.sorted_tables: if (table.name.startswith("rt_") and table.name not in valid_resource_type_tables): - sqlalchemy_base.Base.metadata.remove(table) + gnocchi_sqlalchemy_base.Base.metadata.remove(table) self.index._RESOURCE_TYPE_MANAGER._cache.pop( table.name.replace('_history', ''), None) @@ -72,7 +74,44 @@ def _drop_database(self): @staticmethod def get_metadata(): - return sqlalchemy_base.Base.metadata + return gnocchi_sqlalchemy_base.Base.metadata def get_engine(self): return self.index.get_engine() + + def compare_server_default(self, ctxt, ins_col, meta_col, insp_def, meta_def, rendered_meta_def): + """Compare default values between model and db table. + + Return True if the defaults are different, False if not, or None to + allow the default implementation to compare these defaults. + + :param ctxt: alembic MigrationContext instance + :param ins_col: reflected column + :param insp_def: reflected column default value + :param meta_col: column from model + :param meta_def: column default value from model + :param rendered_meta_def: rendered column default value (from model) + + """ + + # When the column has server_default=sqlalchemy.sql.func.now(), the diff includes the followings diff + # [ [ ( 'modify_default', + # None, + # 'metric', + # 'last_measure_timestamp', + # { 'existing_comment': None, + # 'existing_nullable': False, + # 'existing_type': DATETIME()}, + # DefaultClause(, for_update=False), + # DefaultClause(, for_update=False))]] + method_return = super(ModelsMigrationsSync, self).compare_server_default(ctxt, ins_col, meta_col, insp_def, + meta_def, rendered_meta_def) + + is_server_default_current_timestamp = isinstance(meta_def.arg, sa.sql.functions.current_timestamp) and\ + isinstance(ins_col.server_default.arg, sa.sql.elements.TextClause) + + if not is_server_default_current_timestamp: + return method_return + + # If it is different from "CURRENT_TIMESTAMP", then we must throw an error. + return rendered_meta_def != "CURRENT_TIMESTAMP" From cd1d6492bc089b6d5916b3d4a48fcb01a63ad2c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Thu, 13 Feb 2025 10:57:35 -0300 Subject: [PATCH 13/14] Adjust DB schema revision path --- ...ec_create_last_measure_timestamp_column.py | 6 ++-- gnocchi/indexer/sqlalchemy_base.py | 2 +- .../indexer/sqlalchemy/test_migrations.py | 31 +++++++++++-------- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py b/gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py index a1837c826..0ee9abfbb 100644 --- a/gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py +++ b/gnocchi/indexer/alembic/versions/f89ed2e3c2ec_create_last_measure_timestamp_column.py @@ -15,7 +15,7 @@ """Create last measure push timestamp column Revision ID: f89ed2e3c2ec -Revises: 18fff4509e3e +Revises: 94544ca86c7e Create Date: 2024-04-24 09:16:00 """ @@ -26,7 +26,7 @@ # revision identifiers, used by Alembic. revision = 'f89ed2e3c2ec' -down_revision = '18fff4509e3e' +down_revision = '94544ca86c7e' branch_labels = None depends_on = None @@ -34,7 +34,7 @@ def upgrade(): sql_column = sqlalchemy.Column( "last_measure_timestamp", sqlalchemy.DateTime, - nullable=False, default=datetime.datetime.now(datetime.UTC), + nullable=False, default=datetime.datetime.utcnow(), server_default=sqlalchemy.sql.func.current_timestamp()) op.add_column( "metric", sql_column) diff --git a/gnocchi/indexer/sqlalchemy_base.py b/gnocchi/indexer/sqlalchemy_base.py index 5a0425e0a..71cc6917e 100644 --- a/gnocchi/indexer/sqlalchemy_base.py +++ b/gnocchi/indexer/sqlalchemy_base.py @@ -120,7 +120,7 @@ class Metric(Base, GnocchiBase, indexer.Metric): # measurements; thus, if all metric for a resource are in this situation, # chances are that the resource ceased existing in the backend. last_measure_timestamp = sqlalchemy.Column( - "last_measure_timestamp", sqlalchemy.DateTime, default=datetime.datetime.now(datetime.UTC), nullable=False, + "last_measure_timestamp", sqlalchemy.DateTime, default=datetime.datetime.utcnow(), nullable=False, server_default=sqlalchemy.sql.func.current_timestamp()) def jsonify(self): diff --git a/gnocchi/tests/indexer/sqlalchemy/test_migrations.py b/gnocchi/tests/indexer/sqlalchemy/test_migrations.py index 975a6b950..f54144797 100644 --- a/gnocchi/tests/indexer/sqlalchemy/test_migrations.py +++ b/gnocchi/tests/indexer/sqlalchemy/test_migrations.py @@ -92,26 +92,31 @@ def compare_server_default(self, ctxt, ins_col, meta_col, insp_def, meta_def, re :param meta_def: column default value from model :param rendered_meta_def: rendered column default value (from model) + When the column has server_default=sqlalchemy.sql.func.now(), the diff includes the followings diff + [ [ ( 'modify_default', + None, + 'metric', + 'last_measure_timestamp', + { 'existing_comment': None, + 'existing_nullable': False, + 'existing_type': DATETIME()}, + DefaultClause(, for_update=False), + DefaultClause(, for_update=False))]] + """ - # When the column has server_default=sqlalchemy.sql.func.now(), the diff includes the followings diff - # [ [ ( 'modify_default', - # None, - # 'metric', - # 'last_measure_timestamp', - # { 'existing_comment': None, - # 'existing_nullable': False, - # 'existing_type': DATETIME()}, - # DefaultClause(, for_update=False), - # DefaultClause(, for_update=False))]] method_return = super(ModelsMigrationsSync, self).compare_server_default(ctxt, ins_col, meta_col, insp_def, meta_def, rendered_meta_def) - is_server_default_current_timestamp = isinstance(meta_def.arg, sa.sql.functions.current_timestamp) and\ - isinstance(ins_col.server_default.arg, sa.sql.elements.TextClause) + is_meta_column_default_timestamp = meta_def is not None and isinstance( + meta_def.arg, sa.sql.functions.current_timestamp) + is_reflected_column_default_text_type = ins_col is not None and ins_col.server_default is not None and \ + isinstance(ins_col.server_default.arg, sa.sql.elements.TextClause) + + is_server_default_current_timestamp = is_meta_column_default_timestamp and is_reflected_column_default_text_type if not is_server_default_current_timestamp: return method_return - # If it is different from "CURRENT_TIMESTAMP", then we must throw an error. + # If it is different from "CURRENT_TIMESTAMP", then we must return True, so the test flow continues. return rendered_meta_def != "CURRENT_TIMESTAMP" From 8fc168f03d1418b670748c771de6029c37f61e2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Weing=C3=A4rtner?= Date: Mon, 17 Feb 2025 09:35:16 -0300 Subject: [PATCH 14/14] test --- .../tests/functional/gabbits/aggregates-with-resources.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gnocchi/tests/functional/gabbits/aggregates-with-resources.yaml b/gnocchi/tests/functional/gabbits/aggregates-with-resources.yaml index d65f25d6f..aa025463f 100644 --- a/gnocchi/tests/functional/gabbits/aggregates-with-resources.yaml +++ b/gnocchi/tests/functional/gabbits/aggregates-with-resources.yaml @@ -6,6 +6,9 @@ defaults: # User foobar authorization: "basic Zm9vYmFyOg==" content-type: application/json + poll: + count: 3 + delay: 1 tests: - name: create archive policy