Browse Source

fix(config): add some validation for sliceable-invariant slicing config (#44336)

Verify:
- each logical partition has one, and only one slice ID assignment
- for the consumer being started with `SlicingRouter`, each logical
partition should be mapped to a slice ID that has a defined Kafka
cluster
Oliver Newland 2 years ago
parent
commit
8b60a5c264

+ 32 - 3
src/sentry/sentry_metrics/consumers/indexer/slicing_router.py

@@ -31,6 +31,32 @@ class MissingOrgInRoutingHeader(Exception):
     """
 
 
+def _validate_slicing_config() -> None:
+    """
+    Validates the generalized slicing config (not focusing on an individual
+    sliceable)
+    """
+    for (sliceable, assignments) in settings.SENTRY_SLICING_CONFIG.items():
+        acc = {}
+        for ((assign_lo, assign_hi), _slice_id) in assignments.items():
+            for logical_part in range(assign_lo, assign_hi):
+                if logical_part in acc:
+                    raise SlicingConfigurationException(
+                        f"'{sliceable}' has two assignments to logical partition {logical_part}"
+                    )
+                else:
+                    acc[logical_part] = _slice_id
+
+        missing_logical_parts = set(
+            range(0, settings.SENTRY_SLICING_LOGICAL_PARTITION_COUNT)
+        ) - set(acc.keys())
+
+        if not len(missing_logical_parts) == 0:
+            raise SlicingConfigurationException(
+                f"'{sliceable}' is missing logical partition assignments: {missing_logical_parts}"
+            )
+
+
 def _validate_slicing_consumer_config(sliceable: Sliceable) -> None:
     """
     Validate all the required settings needed for a slicing router.
@@ -70,6 +96,7 @@ class SlicingRouter(MessageRouter):
     ) -> None:
         self.__sliceable = sliceable
         self.__slice_to_producer: MutableMapping[int, MessageRoute] = {}
+        _validate_slicing_config()
         _validate_slicing_consumer_config(self.__sliceable)
 
         for (
@@ -82,9 +109,11 @@ class SlicingRouter(MessageRouter):
                 ),
                 topic=Topic(configuration["topic"]),
             )
-        assert len(self.__slice_to_producer) == len(
-            settings.SENTRY_SLICING_CONFIG[sliceable].keys()
-        )
+        # All logical partitions should be routed to a slice ID that's present in the slice
+        # ID to producer message route mapping
+        assert set(settings.SENTRY_SLICING_CONFIG[sliceable].values()).issubset(
+            self.__slice_to_producer.keys()
+        ), f"Unknown slice ID in SENTRY_SLICING_CONFIG for {sliceable}"
 
     def get_all_producers(self) -> Sequence[Producer]:
         return [route.producer for route in self.__slice_to_producer.values()]

+ 34 - 0
tests/sentry/sentry_metrics/consumers/test_slicing_router.py

@@ -16,6 +16,7 @@ from sentry.sentry_metrics.consumers.indexer.slicing_router import (
     MissingOrgInRoutingHeader,
     SlicingConfigurationException,
     SlicingRouter,
+    _validate_slicing_config,
     _validate_slicing_consumer_config,
 )
 
@@ -175,3 +176,36 @@ def test_validate_slicing_consumer_config(monkeypatch) -> None:
         _validate_slicing_consumer_config("sliceable")
     except SlicingConfigurationException as e:
         assert False, f"Should not raise exception: {e}"
+
+
+def test_validate_slicing_config(monkeypatch) -> None:
+    # Valid setup(s)
+    monkeypatch.setitem(
+        SENTRY_SLICING_CONFIG, "sliceable", {(0, 128): 0, (128, 256): 1}  # type: ignore
+    )
+    _validate_slicing_config()
+
+    monkeypatch.setitem(
+        SENTRY_SLICING_CONFIG, "sliceable", {(0, 64): 0, (64, 66): 1, (66, 100): 0, (100, 256): 1}  # type: ignore
+    )
+    _validate_slicing_config()
+
+    # Assign a given logical partition to two slices
+    monkeypatch.setitem(
+        SENTRY_SLICING_CONFIG, "sliceable", {(0, 129): 0, (128, 256): 1}  # type: ignore
+    )
+    with pytest.raises(
+        SlicingConfigurationException,
+        match=r"'sliceable' has two assignments to logical partition 128",
+    ):
+        _validate_slicing_config()
+
+    # Fail to assign a logical partition to a slice
+    monkeypatch.setitem(
+        SENTRY_SLICING_CONFIG, "sliceable", {(0, 127): 0, (128, 256): 1}  # type: ignore
+    )
+    with pytest.raises(
+        SlicingConfigurationException,
+        match=r"'sliceable' is missing logical partition assignments: \{127\}",
+    ):
+        _validate_slicing_config()