From 6bd6ca07d78fc2dd1841c927f6d6f28d74be6605 Mon Sep 17 00:00:00 2001 From: Sina P <55766091+sinapah@users.noreply.github.com> Date: Mon, 14 Sep 2026 08:53:07 -0400 Subject: [PATCH] fix: sort grpc addresses to avoid unnecessary worker restarts (#85) * fix: sort grpc addresses * fix: comments --- coordinator/charmcraft.yaml | 11 ++++++++ coordinator/src/charm.py | 5 ++++ coordinator/src/mimir_config.py | 5 ++++ coordinator/tests/unit/test_charm.py | 30 +++++++++++++++++++++ coordinator/tests/unit/test_mimir_config.py | 20 ++++++++++++++ 5 files changed, 71 insertions(+) diff --git a/coordinator/charmcraft.yaml b/coordinator/charmcraft.yaml index b414ba6..d6d16fc 100755 --- a/coordinator/charmcraft.yaml +++ b/coordinator/charmcraft.yaml @@ -231,4 +231,15 @@ config: When this value is set to an invalid value (e.g. "1week"), the charm will be blocked along with a relevant message. To avoid data loss, the retention period will be set to the default of 0 in the worker config, effectively disabling data deletion. type: string + out_of_order_time_window: + default: "0s" + description: | + The time window within which out-of-order samples are accepted for ingestion. + Supported units: s, m, h (Prometheus duration format; "m" is minutes). + Set to "0s" to disable (default). + NOTE: This is an experimental feature in Mimir 2.17+. + See https://grafana.com/docs/mimir/latest/configure/configure-out-of-order-samples-ingestion/#configure-out-of-order-samples-ingestion + CLI flag: -ingester.out-of-order-time-window + Note: enabling out-of-order ingestion may impact CPU usage. + type: string diff --git a/coordinator/src/charm.py b/coordinator/src/charm.py index aea1cc5..03cb070 100755 --- a/coordinator/src/charm.py +++ b/coordinator/src/charm.py @@ -84,6 +84,7 @@ def __init__(self, *args: Any): ) self.alertmanager = AlertmanagerConsumer(charm=self, relation_name="alertmanager") self.retention_period = str(self.config['metrics_retention_period']) + self.out_of_order_time_window = str(self.config["out_of_order_time_window"]) self.coordinator = Coordinator( charm=self, roles_config=MIMIR_ROLES_CONFIG, @@ -118,6 +119,7 @@ def __init__(self, *args: Any): max_global_exemplars_per_user=int(self.config["max_global_exemplars_per_user"]), metrics_retention_period=self.retention_period if is_valid_timespec(self.retention_period) else None, ingestion_rate=max(0, int(self.config["ingestion_rate"])), + out_of_order_time_window=self.out_of_order_time_window if is_valid_timespec(self.out_of_order_time_window) else None, ).config, worker_ports=lambda _: tuple({8080, 9095}), resources_requests=self.get_resource_requests, @@ -470,6 +472,9 @@ def _on_collect_unit_status(self, event: ops.CollectStatusEvent): if not is_valid_timespec(self.retention_period): logger.info(f"Suspending data deletion due to invalid option set in config: {self.retention_period}. To resume data deletion, please reset value to a valid option.") event.add_status(BlockedStatus(f"Invalid config option (see debug-log): retention_period={self.retention_period}")) + if not is_valid_timespec(self.out_of_order_time_window): + logger.info(f"Suspending out-of-order ingestion due to invalid option set in config: {self.out_of_order_time_window}. To resume out-of-order ingestion, please reset value to a valid option.") + event.add_status(BlockedStatus(f"Invalid config option (see debug-log): out_of_order_time_window={self.out_of_order_time_window}")) if self._has_alert_rule_errors(): event.add_status(BlockedStatus("Invalid alert rules. See debug-log")) diff --git a/coordinator/src/mimir_config.py b/coordinator/src/mimir_config.py index fd0c345..f70f3de 100755 --- a/coordinator/src/mimir_config.py +++ b/coordinator/src/mimir_config.py @@ -114,6 +114,7 @@ def __init__( recovery_data_dir: Path = Path("/recovery-data"), metrics_retention_period: Optional[str] = None, ingestion_rate: Optional[int] = None, + out_of_order_time_window: Optional[str] = None, ): self._alertmanager_urls = alertmanager_urls self._root_data_dir = root_data_dir @@ -122,6 +123,7 @@ def __init__( self._topology = topology self._metrics_retention_period: str = metrics_retention_period or "0" self._ingestion_rate = ingestion_rate + self._out_of_order_time_window = out_of_order_time_window def config(self, coordinator: Coordinator) -> str: """Generate shared config file for mimir. @@ -399,5 +401,8 @@ def _build_limits_config(self) -> Dict[str, Any]: # This is for consistency. limits_config["compactor_blocks_retention_period"] = 0 if self._metrics_retention_period == "0" else self._metrics_retention_period + if self._out_of_order_time_window is not None: + limits_config["out_of_order_time_window"] = self._out_of_order_time_window + return limits_config diff --git a/coordinator/tests/unit/test_charm.py b/coordinator/tests/unit/test_charm.py index 0c12c1b..7a1ccf0 100644 --- a/coordinator/tests/unit/test_charm.py +++ b/coordinator/tests/unit/test_charm.py @@ -103,6 +103,36 @@ def test_config_retention_period(context, s3, all_worker, nginx_container, nginx assert isinstance(state_out.unit_status, expected_status) +@pytest.mark.parametrize( + "set_config, expected_status", + [ + ("5m", ActiveStatus), + ("1h", ActiveStatus), + ("30s", ActiveStatus), + ("0s", ActiveStatus), + ("5min", BlockedStatus), + ("banana", BlockedStatus), + ] +) +def test_config_out_of_order_time_window(context, s3, all_worker, nginx_container, nginx_prometheus_exporter_container, set_config, expected_status): + """Ensure the out_of_order_time_window config is validated and set correctly.""" + config = {"out_of_order_time_window": set_config} + + state_in = State( + relations=[ + s3, + all_worker, + ], + containers=[nginx_container, nginx_prometheus_exporter_container], + leader=True, + config=config + ) + + with context(context.on.relation_joined(all_worker), state_in) as mgr: + state_out = mgr.run() + assert isinstance(state_out.unit_status, expected_status) + + def test_alerts_hash_not_written_on_mimirtool_failure( context, s3, diff --git a/coordinator/tests/unit/test_mimir_config.py b/coordinator/tests/unit/test_mimir_config.py index 8b97987..19e09ad 100644 --- a/coordinator/tests/unit/test_mimir_config.py +++ b/coordinator/tests/unit/test_mimir_config.py @@ -242,6 +242,26 @@ def test_build_ingester_config(mimir_config, coordinator, addresses_by_role, rep assert ingester_config == expected_config +@pytest.mark.parametrize( + "time_window, expected_key_present, expected_value", + [ + (None, False, None), + ("0s", True, "0s"), + ("0m", True, "0m"), + ("10m", True, "10m"), + ("1h", True, "1h"), + ("30m", True, "30m"), + ], +) +def test_out_of_order_time_window(topology, time_window, expected_key_present, expected_value): + cfg = MimirConfig(topology=topology, out_of_order_time_window=time_window) + limits_config = cfg._build_limits_config() + if expected_key_present: + assert limits_config["out_of_order_time_window"] == expected_value + else: + assert "out_of_order_time_window" not in limits_config + + def test_build_ruler_config(mimir_config): ruler_config = mimir_config._build_ruler_config() expected_config = {