From 815842c4c6f5ade8807eb879b6f0e31f62026522 Mon Sep 17 00:00:00 2001 From: 72004 Date: Sat, 17 Oct 2026 22:56:14 +0500 Subject: [PATCH 1/2] Read collection-stats-sample-interval for SolrCollectionStats The parameter was documented but never passed to the device, which always polled every 30 s. Values <= 0 are rejected like shard-stats-sample-interval. Fixes #136 --- solrorbit/telemetry.py | 5 +++++ .../worker_coordinator/worker_coordinator.py | 5 ++++- tests/unit/solr/test_telemetry.py | 6 ++++++ .../worker_coordinator/worker_coordinator_test.py | 15 +++++++++++++++ 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/solrorbit/telemetry.py b/solrorbit/telemetry.py index bb0c90e8..7ad5fcf6 100644 --- a/solrorbit/telemetry.py +++ b/solrorbit/telemetry.py @@ -992,6 +992,11 @@ class SolrCollectionStats(SolrTelemetryDevice): def __init__(self, admin_client, metrics_store, collections: list = None, sample_interval_s: float = 30.0): + if sample_interval_s <= 0: + raise exceptions.SystemSetupError( + "The telemetry parameter 'collection-stats-sample-interval' must be greater than zero " + f"but was {sample_interval_s}." + ) super().__init__(admin_client, metrics_store, sample_interval_s) self._collections = collections diff --git a/solrorbit/worker_coordinator/worker_coordinator.py b/solrorbit/worker_coordinator/worker_coordinator.py index 8db170fe..af82b762 100644 --- a/solrorbit/worker_coordinator/worker_coordinator.py +++ b/solrorbit/worker_coordinator/worker_coordinator.py @@ -932,7 +932,10 @@ def _create_solr_telemetry_devices(self, clients): return [ telemetry.SolrJvmStats(sc, self.metrics_store), telemetry.SolrNodeStats(sc, self.metrics_store), - telemetry.SolrCollectionStats(sc, self.metrics_store), + telemetry.SolrCollectionStats( + sc, self.metrics_store, + sample_interval_s=telemetry_params.get("collection-stats-sample-interval", 30), + ), telemetry.SolrQueryStats(sc, self.metrics_store), telemetry.SolrIndexingStats(sc, self.metrics_store), telemetry.SolrCacheStats(sc, self.metrics_store), diff --git a/tests/unit/solr/test_telemetry.py b/tests/unit/solr/test_telemetry.py index 4f0df25a..a753cc1c 100644 --- a/tests/unit/solr/test_telemetry.py +++ b/tests/unit/solr/test_telemetry.py @@ -21,6 +21,7 @@ import unittest from unittest.mock import MagicMock +from solrorbit import exceptions from solrorbit.telemetry import ( SolrCacheStats, SolrCollectionStats, @@ -394,6 +395,11 @@ def test_luke_stats_on_error_no_crash(self): device = SolrCollectionStats(client, MagicMock(), collections=["coll"]) device._collect() # must not raise + def test_rejects_non_positive_sample_interval(self): + for interval in (0, -5): + with self.assertRaisesRegex(exceptions.SystemSetupError, "collection-stats-sample-interval"): + SolrCollectionStats(MagicMock(), MagicMock(), sample_interval_s=interval) + # --------------------------------------------------------------------------- # SolrQueryStats diff --git a/tests/worker_coordinator/worker_coordinator_test.py b/tests/worker_coordinator/worker_coordinator_test.py index 2ee7a4b8..fe482dd8 100644 --- a/tests/worker_coordinator/worker_coordinator_test.py +++ b/tests/worker_coordinator/worker_coordinator_test.py @@ -189,6 +189,21 @@ def test_assign_worker_coordinators_round_robin(self): # Did we start all load generators? There is no specific mock assert for this... self.assertEqual(4, target.start_worker.call_count) + def test_collection_stats_uses_sample_interval_param(self): + # pylint: disable=import-outside-toplevel + from solrorbit import telemetry + from solrorbit.client import SolrClient + + self.cfg.add(config.Scope.applicationOverride, "telemetry", "params", {"collection-stats-sample-interval": 7}) + target = self.create_test_worker_coordinator_target() + d = worker_coordinator.WorkerCoordinator(target, self.cfg, client_factory_class=WorkerCoordinatorTests.StaticClientFactory) + + devices = d._create_solr_telemetry_devices({"default": mock.MagicMock(spec=SolrClient)}) + + collection_stats = [device for device in devices if isinstance(device, telemetry.SolrCollectionStats)] + self.assertEqual(1, len(collection_stats)) + self.assertEqual(7, collection_stats[0]._sample_interval) + def test_client_reaches_join_point_others_still_executing(self): target = self.create_test_worker_coordinator_target() d = worker_coordinator.WorkerCoordinator(target, self.cfg, client_factory_class=WorkerCoordinatorTests.StaticClientFactory) From 1015b71ff3fb7eed4e103537c58201d687b49425 Mon Sep 17 00:00:00 2001 From: 72004 Date: Thu, 22 Oct 2026 01:14:54 +0500 Subject: [PATCH 2/2] Pass telemetry_params to SolrCollectionStats and read the sample interval there --- solrorbit/telemetry.py | 8 ++++++-- solrorbit/worker_coordinator/worker_coordinator.py | 5 +---- tests/unit/solr/test_telemetry.py | 7 ++++++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/solrorbit/telemetry.py b/solrorbit/telemetry.py index 7ad5fcf6..e7e5d566 100644 --- a/solrorbit/telemetry.py +++ b/solrorbit/telemetry.py @@ -990,8 +990,12 @@ class SolrCollectionStats(SolrTelemetryDevice): human_name = "Solr Collection Stats" help = "Per-collection: doc count, deleted docs, index size, and segment count (30 s interval)" - def __init__(self, admin_client, metrics_store, - collections: list = None, sample_interval_s: float = 30.0): + def __init__(self, admin_client, metrics_store, telemetry_params=None, collections: list = None): + """ + :param telemetry_params: May optionally specify + ``collection-stats-sample-interval``: positive number, seconds between polls. Default: 30. + """ + sample_interval_s = (telemetry_params or {}).get("collection-stats-sample-interval", 30) if sample_interval_s <= 0: raise exceptions.SystemSetupError( "The telemetry parameter 'collection-stats-sample-interval' must be greater than zero " diff --git a/solrorbit/worker_coordinator/worker_coordinator.py b/solrorbit/worker_coordinator/worker_coordinator.py index 25cea506..c176a691 100644 --- a/solrorbit/worker_coordinator/worker_coordinator.py +++ b/solrorbit/worker_coordinator/worker_coordinator.py @@ -932,10 +932,7 @@ def _create_solr_telemetry_devices(self, clients): return [ telemetry.SolrJvmStats(sc, self.metrics_store), telemetry.SolrNodeStats(sc, self.metrics_store), - telemetry.SolrCollectionStats( - sc, self.metrics_store, - sample_interval_s=telemetry_params.get("collection-stats-sample-interval", 30), - ), + telemetry.SolrCollectionStats(sc, self.metrics_store, telemetry_params), telemetry.SolrQueryStats(sc, self.metrics_store), telemetry.SolrIndexingStats(sc, self.metrics_store), telemetry.SolrCacheStats(sc, self.metrics_store), diff --git a/tests/unit/solr/test_telemetry.py b/tests/unit/solr/test_telemetry.py index a753cc1c..05bb40f2 100644 --- a/tests/unit/solr/test_telemetry.py +++ b/tests/unit/solr/test_telemetry.py @@ -398,7 +398,12 @@ def test_luke_stats_on_error_no_crash(self): def test_rejects_non_positive_sample_interval(self): for interval in (0, -5): with self.assertRaisesRegex(exceptions.SystemSetupError, "collection-stats-sample-interval"): - SolrCollectionStats(MagicMock(), MagicMock(), sample_interval_s=interval) + SolrCollectionStats(MagicMock(), MagicMock(), {"collection-stats-sample-interval": interval}) + + def test_sample_interval_from_telemetry_params(self): + self.assertEqual(30, SolrCollectionStats(MagicMock(), MagicMock())._sample_interval) + device = SolrCollectionStats(MagicMock(), MagicMock(), {"collection-stats-sample-interval": 7}) + self.assertEqual(7, device._sample_interval) # ---------------------------------------------------------------------------