From ddc8dcb358fca43cf82168bc47cf59d68e1aa321 Mon Sep 17 00:00:00 2001 From: Vinayak Rao Date: Mon, 21 Sep 2026 14:11:12 -0700 Subject: [PATCH 1/8] feat(uptime): Add a hash-tagged sentinel key per config partition Name one sentinel key per partition and store as {uptime:configs:N}:sentinel. The hash tag makes Redis Cluster hash only the partition key's text, so the sentinel lands on the same slot, and shard, as the hash it stands for: a shard that loses its data loses its sentinel with it. It is a separate key rather than a field in the config hash because the checker decodes every field of uptime:configs:N as a config at boot and would log invalid_config_message for anything else. The checker reads its hashes by exact name, so it never sees the sentinel. The test computes slots offline with rediscluster's NodeManager and checks every partition for both an empty and a test key prefix. Refs INFRENG-621 --- fixtures/stubs-for-mypy/rediscluster/nodemanager.pyi | 5 +++++ src/sentry/uptime/config_drift.py | 5 +++++ tests/sentry/uptime/test_config_drift.py | 12 ++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 fixtures/stubs-for-mypy/rediscluster/nodemanager.pyi diff --git a/fixtures/stubs-for-mypy/rediscluster/nodemanager.pyi b/fixtures/stubs-for-mypy/rediscluster/nodemanager.pyi new file mode 100644 index 000000000000..970b0f04a8b8 --- /dev/null +++ b/fixtures/stubs-for-mypy/rediscluster/nodemanager.pyi @@ -0,0 +1,5 @@ +from typing import Any + +class NodeManager: + def __init__(self, startup_nodes: list[dict[str, Any]], **kwargs: object) -> None: ... + def keyslot(self, key: str | bytes) -> int: ... diff --git a/src/sentry/uptime/config_drift.py b/src/sentry/uptime/config_drift.py index d15faecfc0c0..a98db44d2e94 100644 --- a/src/sentry/uptime/config_drift.py +++ b/src/sentry/uptime/config_drift.py @@ -118,3 +118,8 @@ def find_orphaned_configs(store: ConfigStore, partition: int) -> DriftResult: .values_list("uptime_subscription__subscription_id", flat=True) ) return DriftResult(checked=len(stored), drifted_ids=frozenset(stored - live)) + + +def get_sentinel_key(key_prefix: str, partition: int) -> str: + # Hash-tagged so it shares a cluster slot with the partition hash it stands for. + return f"{{{get_config_key(key_prefix, partition)}}}:sentinel" diff --git a/tests/sentry/uptime/test_config_drift.py b/tests/sentry/uptime/test_config_drift.py index 144fe9146098..ca155e7ad535 100644 --- a/tests/sentry/uptime/test_config_drift.py +++ b/tests/sentry/uptime/test_config_drift.py @@ -8,11 +8,13 @@ from django.db import connections, router from django.test import override_settings from django.test.utils import CaptureQueriesContext +from rediscluster.nodemanager import NodeManager from sentry.conf.types.uptime import UptimeRegionConfig from sentry.testutils.cases import UptimeTestCase from sentry.testutils.helpers import override_options from sentry.testutils.helpers.datetime import freeze_time +from sentry.uptime.config_drift import get_sentinel_key from sentry.uptime.config_producer import ( get_config_key, get_partition_from_subscription_id, @@ -342,3 +344,13 @@ def test_tasks_write_nothing_to_postgres(self) -> None: if q["sql"].startswith(("INSERT", "UPDATE", "DELETE")) ] assert writes == [] + + +def test_sentinel_shares_slot_with_partition_hash() -> None: + # Offline slot math; the node is never contacted. + keyslot = NodeManager(startup_nodes=[{"host": "localhost", "port": 1}]).keyslot + for key_prefix in ("", "a"): + for partition in range(128): + assert keyslot(get_sentinel_key(key_prefix, partition)) == keyslot( + get_config_key(key_prefix, partition) + ) From 61adb92acc55538008e14b0763b8913bdb8d506e Mon Sep 17 00:00:00 2001 From: Vinayak Rao Date: Mon, 21 Sep 2026 15:04:44 -0700 Subject: [PATCH 2/8] feat(uptime): Check config partition sentinels every minute Add check_config_sentinels, scheduled at one minute, which pipelines EXISTS on the 128 sentinel keys of each config store and reports the number absent as the gauge uptime.config_drift.sentinel_missing by cluster. A wiped store loses every sentinel, so the gauge jumps to 128 within a minute with no Postgres read, well before the checker's next restart drops the configs from memory. It is a gauge rather than a counter because the same 128 keys are re-checked every minute; the value is a level and must not sum across runs like the sweep's checked/missing counts do. The task shares the sweep's uptime.config-drift.enabled kill switch, so with it off nothing is emitted. Each store is checked on its own: a Redis error on one store is the event being looked for, and must not stop the others from reporting that minute. Nothing writes a sentinel yet: until repair lands the gauge reads as missing everywhere, which is the ticket's intended pre-enable state. Sentinels have no TTL by design; 128 fixed keys per cluster are recorded on the ticket as accepted durable data. Refs INFRENG-621 --- src/sentry/conf/server.py | 4 ++++ src/sentry/uptime/config_drift.py | 8 +++++++ src/sentry/uptime/subscriptions/tasks.py | 29 ++++++++++++++++++++++++ tests/sentry/uptime/test_config_drift.py | 26 +++++++++++++++++++++ 4 files changed, 67 insertions(+) diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index d6ec5166b331..d32beb986e21 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -1139,6 +1139,10 @@ def SOCIAL_AUTH_DEFAULT_USERNAME() -> str: "task": "uptime:sentry.uptime.tasks.config_drift_dispatcher", "schedule": crontab("0", "*/1", "*", "*", "*"), }, + "uptime-config-sentinel-checker": { + "task": "uptime:sentry.uptime.tasks.check_config_sentinels", + "schedule": crontab("*/1", "*", "*", "*", "*"), + }, "poll_tempest": { "task": "tempest:sentry.tempest.tasks.poll_tempest", "schedule": crontab("*/1", "*", "*", "*", "*"), diff --git a/src/sentry/uptime/config_drift.py b/src/sentry/uptime/config_drift.py index a98db44d2e94..ab665c5a8c15 100644 --- a/src/sentry/uptime/config_drift.py +++ b/src/sentry/uptime/config_drift.py @@ -123,3 +123,11 @@ def find_orphaned_configs(store: ConfigStore, partition: int) -> DriftResult: def get_sentinel_key(key_prefix: str, partition: int) -> str: # Hash-tagged so it shares a cluster slot with the partition hash it stands for. return f"{{{get_config_key(key_prefix, partition)}}}:sentinel" + + +def find_missing_sentinels(store: ConfigStore) -> list[int]: + cluster = redis.redis_clusters.get_binary(store.cluster) + pipe = cluster.pipeline() + for partition in range(settings.UPTIME_CONFIG_PARTITIONS): + pipe.exists(get_sentinel_key(store.key_prefix, partition)) + return [partition for partition, present in enumerate(pipe.execute()) if not present] diff --git a/src/sentry/uptime/subscriptions/tasks.py b/src/sentry/uptime/subscriptions/tasks.py index d3a5e95a82a8..886ff0de11ff 100644 --- a/src/sentry/uptime/subscriptions/tasks.py +++ b/src/sentry/uptime/subscriptions/tasks.py @@ -19,6 +19,7 @@ SWEEP_RUN_INTERVAL, ConfigStore, find_missing_configs, + find_missing_sentinels, find_orphaned_configs, get_config_stores, sweep_slice, @@ -371,6 +372,34 @@ def check_orphaned_configs(cluster: str, key_prefix: str, partition: int, **kwar ) +@instrumented_task( + name="sentry.uptime.tasks.check_config_sentinels", + namespace=uptime_tasks, + processing_deadline_duration=60, +) +def check_config_sentinels(**kwargs): + """ + Checks each config store's partition sentinels every minute; a missing sentinel means the + store lost data. + """ + if not options.get("uptime.config-drift.enabled"): + return + + for store in get_config_stores(): + # The store that failed is likely the one being lost; keep checking the others. + try: + missing = find_missing_sentinels(store) + except Exception: + logger.exception("uptime.config_drift.sentinel_check_failed") + continue + metrics.gauge( + "uptime.config_drift.sentinel_missing", + len(missing), + tags={"cluster": store.cluster}, + sample_rate=1.0, + ) + + def repair_missing_configs( store: ConfigStore, subscription_ids: Collection[str], *, limit: int = CONFIG_REPAIR_MAX_TASKS ) -> int: diff --git a/tests/sentry/uptime/test_config_drift.py b/tests/sentry/uptime/test_config_drift.py index ca155e7ad535..4ce7550b6743 100644 --- a/tests/sentry/uptime/test_config_drift.py +++ b/tests/sentry/uptime/test_config_drift.py @@ -23,6 +23,7 @@ from sentry.uptime.models import UptimeSubscription, UptimeSubscriptionRegion from sentry.uptime.subscriptions import tasks from sentry.uptime.subscriptions.tasks import ( + check_config_sentinels, check_missing_configs, check_orphaned_configs, config_drift_dispatcher, @@ -354,3 +355,28 @@ def test_sentinel_shares_slot_with_partition_hash() -> None: assert keyslot(get_sentinel_key(key_prefix, partition)) == keyslot( get_config_key(key_prefix, partition) ) + + +@override_settings(UPTIME_REGIONS=REGIONS) +class CheckConfigSentinelsTest(ConfigPusherTestMixin): + def setUp(self) -> None: + super().setUp() + self.enterContext(override_options({"uptime.config-drift.enabled": True})) + + def test_option_off_emits_metric_only(self) -> None: + cluster = redis.redis_clusters.get_binary("default") + keys_before = set(cluster.keys()) + + with mock.patch.object(tasks, "metrics") as metrics: + check_config_sentinels() + + # One gauge per store, both on the test cluster. + all_missing = mock.call( + "uptime.config_drift.sentinel_missing", + 128, + tags={"cluster": "default"}, + sample_rate=1.0, + ) + assert metrics.gauge.mock_calls == [all_missing, all_missing] + assert metrics.incr.mock_calls == [] + assert set(cluster.keys()) == keys_before From 31cc265d113e485647bcc5271c7a8a6f92c11d0c Mon Sep 17 00:00:00 2001 From: Vinayak Rao Date: Mon, 21 Sep 2026 15:05:23 -0700 Subject: [PATCH 3/8] feat(uptime): Repair a config store whose sentinel went missing When a sentinel is absent and uptime.config-drift.repair is on, check_config_sentinels logs the missing partitions and queues repair_config_store for that store only. The task compares the whole store at once: HKEYS on each of the 128 partition hashes against one replica read of the store's ACTIVE rows, the orphan direction's read in reverse. It hands the missing ids to repair_missing_configs under its per-run cap and writes the absent sentinels back once a run has handed off everything it found, so a wipe of N configs keeps the sentinel absent for ceil(N/cap) runs. Looping the sweep's per-prefix find_missing_configs here would cost one HEXISTS per config and 256 queries per run; on a 100k-config store that measured 100,513 Redis commands and 891 ms against 131 commands, one query and 90 ms for the same result. The per-prefix shape exists to touch 1/256 of the table per task, which is the opposite of what this task needs. The ticket asked for the write-back only when the comparison reports zero missing. That has no bound: one ACTIVE row the update task can never publish would keep the comparison non-empty forever, so every minute would re-run the store scan and the gauge could no longer tell that row from a wipe. Gating on the cap leaves such a row to the hourly sweep, which already reports it. The comparison runs in its own task so the minute check stays a pure detector and one slow store cannot delay the others. expires=60 drops a request that was not picked up before the next tick re-queues it, and that re-queue is also the retry, so the task declares none. The warning is logged only when repair is on: before the first repair pass every sentinel is absent, so the partition list would say nothing. Refs INFRENG-621 --- src/sentry/uptime/config_drift.py | 30 ++++++++++ src/sentry/uptime/subscriptions/tasks.py | 36 ++++++++++- tests/sentry/uptime/test_config_drift.py | 76 +++++++++++++++++++++++- 3 files changed, 140 insertions(+), 2 deletions(-) diff --git a/src/sentry/uptime/config_drift.py b/src/sentry/uptime/config_drift.py index ab665c5a8c15..a9e4918bf761 100644 --- a/src/sentry/uptime/config_drift.py +++ b/src/sentry/uptime/config_drift.py @@ -120,6 +120,28 @@ def find_orphaned_configs(store: ConfigStore, partition: int) -> DriftResult: return DriftResult(checked=len(stored), drifted_ids=frozenset(stored - live)) +def find_missing_configs_for_store(store: ConfigStore) -> DriftResult: + """ + Whole-store form of find_missing_configs: reads every partition's field names in one + pipeline instead of probing one id at a time, since here every row is checked anyway. + """ + # Postgres first: a row ACTIVE now had its config written before now, so a config + # published between the two reads can't show up as missing. + live = { + subscription_id + for subscription_id in _active_region_rows() + .filter(region_slug__in=store.region_slugs) + .values_list("uptime_subscription__subscription_id", flat=True) + if subscription_id is not None + } + cluster = redis.redis_clusters.get_binary(store.cluster) + pipe = cluster.pipeline() + for partition in range(settings.UPTIME_CONFIG_PARTITIONS): + pipe.hkeys(get_config_key(store.key_prefix, partition)) + stored = {field.decode() for fields in pipe.execute() for field in fields} + return DriftResult(checked=len(live), drifted_ids=frozenset(live - stored)) + + def get_sentinel_key(key_prefix: str, partition: int) -> str: # Hash-tagged so it shares a cluster slot with the partition hash it stands for. return f"{{{get_config_key(key_prefix, partition)}}}:sentinel" @@ -131,3 +153,11 @@ def find_missing_sentinels(store: ConfigStore) -> list[int]: for partition in range(settings.UPTIME_CONFIG_PARTITIONS): pipe.exists(get_sentinel_key(store.key_prefix, partition)) return [partition for partition, present in enumerate(pipe.execute()) if not present] + + +def write_sentinels(store: ConfigStore) -> None: + cluster = redis.redis_clusters.get_binary(store.cluster) + pipe = cluster.pipeline() + for partition in range(settings.UPTIME_CONFIG_PARTITIONS): + pipe.set(get_sentinel_key(store.key_prefix, partition), b"1") + pipe.execute() diff --git a/src/sentry/uptime/subscriptions/tasks.py b/src/sentry/uptime/subscriptions/tasks.py index 886ff0de11ff..b53f422183fe 100644 --- a/src/sentry/uptime/subscriptions/tasks.py +++ b/src/sentry/uptime/subscriptions/tasks.py @@ -19,10 +19,12 @@ SWEEP_RUN_INTERVAL, ConfigStore, find_missing_configs, + find_missing_configs_for_store, find_missing_sentinels, find_orphaned_configs, get_config_stores, sweep_slice, + write_sentinels, ) from sentry.uptime.config_producer import produce_config, produce_config_removal from sentry.uptime.models import ( @@ -380,7 +382,7 @@ def check_orphaned_configs(cluster: str, key_prefix: str, partition: int, **kwar def check_config_sentinels(**kwargs): """ Checks each config store's partition sentinels every minute; a missing sentinel means the - store lost data. + store lost data, so its whole-store comparison is queued when repair is on. """ if not options.get("uptime.config-drift.enabled"): return @@ -398,6 +400,38 @@ def check_config_sentinels(**kwargs): tags={"cluster": store.cluster}, sample_rate=1.0, ) + if missing and options.get("uptime.config-drift.repair"): + logger.warning( + "uptime.config_drift.sentinel_missing", + extra={"cluster": store.cluster, "count": len(missing), "partitions": missing}, + ) + repair_config_store.delay(cluster=store.cluster, key_prefix=store.key_prefix) + + +@instrumented_task( + name="sentry.uptime.tasks.repair_config_store", + namespace=uptime_tasks, + processing_deadline_duration=60, + expires=60, +) +def repair_config_store(cluster: str, key_prefix: str, **kwargs): + """ + Whole-store comparison after a sentinel went missing, then repair. + """ + store = _find_store(cluster, key_prefix) + if store is None: + return + + result = find_missing_configs_for_store(store) + missing = len(result.drifted_ids) + logger.info( + "uptime.config_drift.store_repair", + extra={"cluster": store.cluster, "checked": result.checked, "missing": missing}, + ) + if missing: + repair_missing_configs(store, result.drifted_ids, limit=CONFIG_REPAIR_MAX_TASKS) + if missing <= CONFIG_REPAIR_MAX_TASKS: + write_sentinels(store) def repair_missing_configs( diff --git a/tests/sentry/uptime/test_config_drift.py b/tests/sentry/uptime/test_config_drift.py index 4ce7550b6743..f434daee5d0a 100644 --- a/tests/sentry/uptime/test_config_drift.py +++ b/tests/sentry/uptime/test_config_drift.py @@ -27,6 +27,7 @@ check_missing_configs, check_orphaned_configs, config_drift_dispatcher, + repair_config_store, update_remote_uptime_subscription, uptime_subscription_to_check_config, ) @@ -367,7 +368,10 @@ def test_option_off_emits_metric_only(self) -> None: cluster = redis.redis_clusters.get_binary("default") keys_before = set(cluster.keys()) - with mock.patch.object(tasks, "metrics") as metrics: + with ( + mock.patch.object(tasks, "metrics") as metrics, + mock.patch.object(repair_config_store, "delay") as delay, + ): check_config_sentinels() # One gauge per store, both on the test cluster. @@ -379,4 +383,74 @@ def test_option_off_emits_metric_only(self) -> None: ) assert metrics.gauge.mock_calls == [all_missing, all_missing] assert metrics.incr.mock_calls == [] + assert not delay.called assert set(cluster.keys()) == keys_before + + @override_options({"uptime.config-drift.repair": True}) + def test_sentinel_write_touches_no_config_hash(self) -> None: + cluster = redis.redis_clusters.get_binary("default") + + with self.tasks(): + check_config_sentinels() + + for key_prefix in "ab": + for partition in range(128): + assert cluster.type(get_sentinel_key(key_prefix, partition)) == b"string" + assert not cluster.exists(get_config_key(key_prefix, partition)) + assert not cluster.exists(f"{key_prefix}uptime:updates:{partition}") + + def _seed_lost_on_b(self, count: int = 1) -> tuple[list[UptimeSubscription], str]: + """ + Writes every sentinel, then creates ``count`` subscriptions never published to store B + and drops one of their partitions' sentinel there. Returns them and the lost sentinel key. + """ + with self.tasks(): + check_config_sentinels() + subscriptions = [] + for _ in range(count): + subscription = self.create_uptime_subscription( + subscription_id=_subscription_id(), region_slugs=["a1", "b1"] + ) + _publish(subscription, ["a1"]) + subscriptions.append(subscription) + assert subscription.subscription_id is not None + partition = get_partition_from_subscription_id(UUID(subscription.subscription_id)) + sentinel = get_sentinel_key("b", partition) + redis.redis_clusters.get_binary("default").delete(sentinel) + return subscriptions, sentinel + + @override_options({"uptime.config-drift.repair": True}) + def test_missing_sentinel_repairs_only_that_store(self) -> None: + [subscription], _ = self._seed_lost_on_b() + + with mock.patch.object(repair_config_store, "delay") as delay: + check_config_sentinels() + delay.assert_called_once_with(cluster="default", key_prefix="b") + + with self.tasks(): + repair_config_store(cluster="default", key_prefix="b") + + self.assert_redis_config( + "b1", subscription, "upsert", UptimeSubscriptionRegion.RegionMode.ACTIVE + ) + + @override_options({"uptime.config-drift.repair": True}) + @mock.patch.object(tasks, "CONFIG_REPAIR_MAX_TASKS", 1) + def test_sentinel_written_only_once_missing_fits_the_cap(self) -> None: + lost, sentinel = self._seed_lost_on_b(count=2) + cluster = redis.redis_clusters.get_binary("default") + + # Two missing against a cap of one: not everything was handed off, so no sentinel. + with mock.patch.object(update_remote_uptime_subscription, "delay") as delay: + repair_config_store(cluster="default", key_prefix="b") + + assert delay.call_count == 1 + assert not cluster.exists(sentinel) + + # One missing fits the cap: the sentinel returns while that one is still in flight. + _publish(lost[0], ["b1"]) + with mock.patch.object(update_remote_uptime_subscription, "delay") as delay: + repair_config_store(cluster="default", key_prefix="b") + + delay.assert_called_once_with(uptime_subscription_id=lost[1].id, region_slugs=["b1"]) + assert cluster.exists(sentinel) From 28b204f7054ef317524c4eeb599f96619410015b Mon Sep 17 00:00:00 2001 From: Vinayak Rao Date: Tue, 22 Sep 2026 11:10:02 -0700 Subject: [PATCH 4/8] feat(uptime): Add a kill switch for sentinel-triggered store repair Add uptime.config-drift.sentinel-repair-disabled, default True, and queue repair_config_store only while it is False. The default is inverted on purpose: once repair is rolled out the option stays False as the runbook lever for alerts on the sentinel_missing gauge, and never becomes a permanently-True option that cannot be removed. The switch replaces uptime.config-drift.repair on the sentinel path only; the hourly sweep still repairs under that option. Detection is untouched and still gated by uptime.config-drift.enabled, so the gauge keeps reporting a store that lost data while repair is off. The partition warning stays with the queue call, since with repair off every sentinel stays absent and the list would always be all 128. It is checked where the task is queued, not inside repair_config_store. A flip reaches workers more slowly than a queued task is picked up, and expires=60 already bounds the backlog to one task per store, so a second check would almost never fire. Refs INFRENG-621 --- src/sentry/options/defaults.py | 9 +++++++++ src/sentry/uptime/subscriptions/tasks.py | 4 ++-- tests/sentry/uptime/test_config_drift.py | 8 ++++---- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py index 3b45356c1b3d..dea043efe401 100644 --- a/src/sentry/options/defaults.py +++ b/src/sentry/options/defaults.py @@ -3725,6 +3725,15 @@ flags=FLAG_AUTOMATOR_MODIFIABLE, ) +# Kill switch for sentinel-triggered whole-store repair. Defaults to True (repair off) so the +# option is never left permanently True with no way to remove it. +register( + "uptime.config-drift.sentinel-repair-disabled", + type=Bool, + default=True, + flags=FLAG_AUTOMATOR_MODIFIABLE, +) + # Controls whether uptime monitoring automatically detects hostnames from error events. register( "uptime.automatic-hostname-detection", diff --git a/src/sentry/uptime/subscriptions/tasks.py b/src/sentry/uptime/subscriptions/tasks.py index b53f422183fe..dcf4b22135b6 100644 --- a/src/sentry/uptime/subscriptions/tasks.py +++ b/src/sentry/uptime/subscriptions/tasks.py @@ -382,7 +382,7 @@ def check_orphaned_configs(cluster: str, key_prefix: str, partition: int, **kwar def check_config_sentinels(**kwargs): """ Checks each config store's partition sentinels every minute; a missing sentinel means the - store lost data, so its whole-store comparison is queued when repair is on. + store lost data, so its whole-store comparison is queued unless sentinel repair is disabled. """ if not options.get("uptime.config-drift.enabled"): return @@ -400,7 +400,7 @@ def check_config_sentinels(**kwargs): tags={"cluster": store.cluster}, sample_rate=1.0, ) - if missing and options.get("uptime.config-drift.repair"): + if missing and not options.get("uptime.config-drift.sentinel-repair-disabled"): logger.warning( "uptime.config_drift.sentinel_missing", extra={"cluster": store.cluster, "count": len(missing), "partitions": missing}, diff --git a/tests/sentry/uptime/test_config_drift.py b/tests/sentry/uptime/test_config_drift.py index f434daee5d0a..aea537374136 100644 --- a/tests/sentry/uptime/test_config_drift.py +++ b/tests/sentry/uptime/test_config_drift.py @@ -364,7 +364,7 @@ def setUp(self) -> None: super().setUp() self.enterContext(override_options({"uptime.config-drift.enabled": True})) - def test_option_off_emits_metric_only(self) -> None: + def test_sentinel_repair_disabled_emits_metric_only(self) -> None: cluster = redis.redis_clusters.get_binary("default") keys_before = set(cluster.keys()) @@ -386,7 +386,7 @@ def test_option_off_emits_metric_only(self) -> None: assert not delay.called assert set(cluster.keys()) == keys_before - @override_options({"uptime.config-drift.repair": True}) + @override_options({"uptime.config-drift.sentinel-repair-disabled": False}) def test_sentinel_write_touches_no_config_hash(self) -> None: cluster = redis.redis_clusters.get_binary("default") @@ -419,7 +419,7 @@ def _seed_lost_on_b(self, count: int = 1) -> tuple[list[UptimeSubscription], str redis.redis_clusters.get_binary("default").delete(sentinel) return subscriptions, sentinel - @override_options({"uptime.config-drift.repair": True}) + @override_options({"uptime.config-drift.sentinel-repair-disabled": False}) def test_missing_sentinel_repairs_only_that_store(self) -> None: [subscription], _ = self._seed_lost_on_b() @@ -434,7 +434,7 @@ def test_missing_sentinel_repairs_only_that_store(self) -> None: "b1", subscription, "upsert", UptimeSubscriptionRegion.RegionMode.ACTIVE ) - @override_options({"uptime.config-drift.repair": True}) + @override_options({"uptime.config-drift.sentinel-repair-disabled": False}) @mock.patch.object(tasks, "CONFIG_REPAIR_MAX_TASKS", 1) def test_sentinel_written_only_once_missing_fits_the_cap(self) -> None: lost, sentinel = self._seed_lost_on_b(count=2) From 0cbd24db6cdb45228205de1ebed37f765bb4e3d6 Mon Sep 17 00:00:00 2001 From: Vinayak Rao Date: Tue, 22 Sep 2026 11:19:23 -0700 Subject: [PATCH 5/8] ref(uptime): Use a decoded Redis client for config drift reads Every function in config_drift.py now takes its client from redis_clusters.get() rather than get_binary(). The config hash fields are subscription ids in hex, so the decoding client returns the same values the code was decoding by hand, and the sentinels no longer need a bytes literal. config_producer stays on the binary client because it writes msgpack values. This also covers find_missing_configs and find_orphaned_configs from the earlier sweep work, so the file uses one client throughout. Both clients are built from the same cluster config and differ only in decode_responses. find_missing_configs_for_store hoists its queryset out of the set comprehension. The queryset is lazy and still evaluates before the Redis pipeline, so Postgres is read first. Refs INFRENG-621 --- src/sentry/uptime/config_drift.py | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/sentry/uptime/config_drift.py b/src/sentry/uptime/config_drift.py index a9e4918bf761..aa7fb8cbbf7b 100644 --- a/src/sentry/uptime/config_drift.py +++ b/src/sentry/uptime/config_drift.py @@ -78,7 +78,7 @@ def _live_region_rows() -> QuerySet[UptimeSubscriptionRegion]: def find_missing_configs(store: ConfigStore, subscription_id_prefix: str) -> DriftResult: - cluster = redis.redis_clusters.get_binary(store.cluster) + cluster = redis.redis_clusters.get(store.cluster) pipe = cluster.pipeline() subscription_ids: list[str] = [] # One row per (subscription, region); slugs sharing a store must be checked once. @@ -103,10 +103,8 @@ def find_missing_configs(store: ConfigStore, subscription_id_prefix: str) -> Dri def find_orphaned_configs(store: ConfigStore, partition: int) -> DriftResult: - cluster = redis.redis_clusters.get_binary(store.cluster) - stored = { - field.decode() for field in cluster.hkeys(get_config_key(store.key_prefix, partition)) - } + cluster = redis.redis_clusters.get(store.cluster) + stored = set(cluster.hkeys(get_config_key(store.key_prefix, partition))) live: set[str | None] = set() for chunk in batched(stored, IN_CHUNK_SIZE): live.update( @@ -127,18 +125,17 @@ def find_missing_configs_for_store(store: ConfigStore) -> DriftResult: """ # Postgres first: a row ACTIVE now had its config written before now, so a config # published between the two reads can't show up as missing. - live = { - subscription_id - for subscription_id in _active_region_rows() + rows = ( + _active_region_rows() .filter(region_slug__in=store.region_slugs) .values_list("uptime_subscription__subscription_id", flat=True) - if subscription_id is not None - } - cluster = redis.redis_clusters.get_binary(store.cluster) + ) + live = {subscription_id for subscription_id in rows if subscription_id is not None} + cluster = redis.redis_clusters.get(store.cluster) pipe = cluster.pipeline() for partition in range(settings.UPTIME_CONFIG_PARTITIONS): pipe.hkeys(get_config_key(store.key_prefix, partition)) - stored = {field.decode() for fields in pipe.execute() for field in fields} + stored = {field for fields in pipe.execute() for field in fields} return DriftResult(checked=len(live), drifted_ids=frozenset(live - stored)) @@ -148,7 +145,7 @@ def get_sentinel_key(key_prefix: str, partition: int) -> str: def find_missing_sentinels(store: ConfigStore) -> list[int]: - cluster = redis.redis_clusters.get_binary(store.cluster) + cluster = redis.redis_clusters.get(store.cluster) pipe = cluster.pipeline() for partition in range(settings.UPTIME_CONFIG_PARTITIONS): pipe.exists(get_sentinel_key(store.key_prefix, partition)) @@ -156,8 +153,8 @@ def find_missing_sentinels(store: ConfigStore) -> list[int]: def write_sentinels(store: ConfigStore) -> None: - cluster = redis.redis_clusters.get_binary(store.cluster) + cluster = redis.redis_clusters.get(store.cluster) pipe = cluster.pipeline() for partition in range(settings.UPTIME_CONFIG_PARTITIONS): - pipe.set(get_sentinel_key(store.key_prefix, partition), b"1") + pipe.set(get_sentinel_key(store.key_prefix, partition), "1") pipe.execute() From 5be0a6f3f6ff963b5913ad2f08128af4432ea4db Mon Sep 17 00:00:00 2001 From: Vinayak Rao Date: Tue, 22 Sep 2026 11:36:25 -0700 Subject: [PATCH 6/8] ref(uptime): Clarify the sentinel key's hash tag comment The old comment said the sentinel shares a cluster slot with the partition hash. Here the partition is only the index N in uptime:configs:N, which Sentry computes from the subscription id, but the word reads as a Kafka partition. Name the mechanism instead: the braces are a Redis hash tag on the config key's name, so the sentinel lands in that key's slot and is lost along with it. Refs INFRENG-621 --- src/sentry/uptime/config_drift.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/uptime/config_drift.py b/src/sentry/uptime/config_drift.py index aa7fb8cbbf7b..13de674e036a 100644 --- a/src/sentry/uptime/config_drift.py +++ b/src/sentry/uptime/config_drift.py @@ -140,7 +140,7 @@ def find_missing_configs_for_store(store: ConfigStore) -> DriftResult: def get_sentinel_key(key_prefix: str, partition: int) -> str: - # Hash-tagged so it shares a cluster slot with the partition hash it stands for. + # Redis hash tag: lands in the config key's slot, so a node that loses it loses this too. return f"{{{get_config_key(key_prefix, partition)}}}:sentinel" From ad396bfc436884762f90a5d560016c0c7f0096cc Mon Sep 17 00:00:00 2001 From: Vinayak Rao Date: Tue, 22 Sep 2026 14:23:10 -0700 Subject: [PATCH 7/8] fix(uptime): Queue each missing config once per store repair pass repair_config_store wrote the sentinels back only once the missing count fit under the per-run cap. More than 1000 configs that can never publish kept it above the cap forever, so the repair re-fired every minute. Batches were also always the lowest 1000 ids, so those configs blocked every fixable id after them, and ids still in flight were queued again on the next run. A per-store cursor now walks the missing ids in order: each run queues the next 1000 after it and moves it on, and once 1000 or fewer remain past it the pass ends, the cursor is cleared and the sentinels are written. Every id is queued once per pass and every pass ends; anything that still fails is left to the drift sweep. The batch is queued before the cursor moves, so a failed cursor write costs duplicates rather than skipped ids. The cursor's 1h TTL is refreshed every run, since the missing sentinels keep calling the repair; if it lapses the pass restarts, and ids already repaired are no longer missing. Writing the sentinels once each partition has data again (HLEN) would end a real wipe after one or two batches, since each batch spreads across every partition, and would never end on a store with empty partitions. Queueing the whole wipe with countdown runs into taskbroker's 1h delay cap and its delayed-task backpressure. A loss behind the cursor during a pass is left to the sweep. Refs INFRENG-621 --- src/sentry/uptime/config_drift.py | 6 +++++ src/sentry/uptime/subscriptions/tasks.py | 31 ++++++++++++++++++------ tests/sentry/uptime/test_config_drift.py | 22 +++++++++-------- 3 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/sentry/uptime/config_drift.py b/src/sentry/uptime/config_drift.py index 13de674e036a..64e46d2a5ac6 100644 --- a/src/sentry/uptime/config_drift.py +++ b/src/sentry/uptime/config_drift.py @@ -20,6 +20,8 @@ SUBSCRIPTION_ID_PREFIX_BUCKETS = 256 # Bounds the IN list when a partition holds many configs. IN_CHUNK_SIZE = 1000 +# Refreshed by every run of a pass; if it lapses, the pass restarts from the first missing id. +REPAIR_CURSOR_TTL = 3600 @dataclass(frozen=True) @@ -144,6 +146,10 @@ def get_sentinel_key(key_prefix: str, partition: int) -> str: return f"{{{get_config_key(key_prefix, partition)}}}:sentinel" +def get_repair_cursor_key(key_prefix: str) -> str: + return f"{key_prefix}uptime:configs:repair-cursor" + + def find_missing_sentinels(store: ConfigStore) -> list[int]: cluster = redis.redis_clusters.get(store.cluster) pipe = cluster.pipeline() diff --git a/src/sentry/uptime/subscriptions/tasks.py b/src/sentry/uptime/subscriptions/tasks.py index dcf4b22135b6..0d87cdb81697 100644 --- a/src/sentry/uptime/subscriptions/tasks.py +++ b/src/sentry/uptime/subscriptions/tasks.py @@ -15,6 +15,7 @@ from sentry.tasks.base import instrumented_task from sentry.taskworker.namespaces import uptime_tasks from sentry.uptime.config_drift import ( + REPAIR_CURSOR_TTL, SUBSCRIPTION_ID_PREFIX_BUCKETS, SWEEP_RUN_INTERVAL, ConfigStore, @@ -23,6 +24,7 @@ find_missing_sentinels, find_orphaned_configs, get_config_stores, + get_repair_cursor_key, sweep_slice, write_sentinels, ) @@ -33,7 +35,7 @@ UptimeSubscriptionRegion, ) from sentry.uptime.types import CheckConfig -from sentry.utils import metrics +from sentry.utils import metrics, redis from sentry.utils.audit import create_system_audit_entry from sentry.utils.query import RangeQuerySetWrapper @@ -416,22 +418,37 @@ def check_config_sentinels(**kwargs): ) def repair_config_store(cluster: str, key_prefix: str, **kwargs): """ - Whole-store comparison after a sentinel went missing, then repair. + One step of a pass over the store's missing configs after a sentinel went missing. """ store = _find_store(cluster, key_prefix) if store is None: return + client = redis.redis_clusters.get(store.cluster) + cursor_key = get_repair_cursor_key(store.key_prefix) + cursor = client.get(cursor_key) or "" result = find_missing_configs_for_store(store) - missing = len(result.drifted_ids) + # Each missing id is queued once per pass, so ids that can't be published neither hold + # the sentinels back nor crowd out the ids after them. + pending = sorted(i for i in result.drifted_ids if i > cursor) + batch = pending[:CONFIG_REPAIR_MAX_TASKS] logger.info( "uptime.config_drift.store_repair", - extra={"cluster": store.cluster, "checked": result.checked, "missing": missing}, + extra={ + "cluster": store.cluster, + "checked": result.checked, + "missing": len(result.drifted_ids), + "pending": len(pending), + }, ) - if missing: - repair_missing_configs(store, result.drifted_ids, limit=CONFIG_REPAIR_MAX_TASKS) - if missing <= CONFIG_REPAIR_MAX_TASKS: + if batch: + repair_missing_configs(store, batch, limit=CONFIG_REPAIR_MAX_TASKS) + if len(pending) <= CONFIG_REPAIR_MAX_TASKS: + # Cleared first, so a failed sentinel write restarts the pass rather than skipping ids. + client.delete(cursor_key) write_sentinels(store) + else: + client.set(cursor_key, batch[-1], ex=REPAIR_CURSOR_TTL) def repair_missing_configs( diff --git a/tests/sentry/uptime/test_config_drift.py b/tests/sentry/uptime/test_config_drift.py index aea537374136..adee8ecaf27b 100644 --- a/tests/sentry/uptime/test_config_drift.py +++ b/tests/sentry/uptime/test_config_drift.py @@ -436,21 +436,23 @@ def test_missing_sentinel_repairs_only_that_store(self) -> None: @override_options({"uptime.config-drift.sentinel-repair-disabled": False}) @mock.patch.object(tasks, "CONFIG_REPAIR_MAX_TASKS", 1) - def test_sentinel_written_only_once_missing_fits_the_cap(self) -> None: + def test_unpublishable_configs_end_the_pass(self) -> None: lost, sentinel = self._seed_lost_on_b(count=2) + first, second = sorted(lost, key=lambda subscription: subscription.subscription_id or "") cluster = redis.redis_clusters.get_binary("default") - # Two missing against a cap of one: not everything was handed off, so no sentinel. + # The republishes never land, so both stay missing on every run. with mock.patch.object(update_remote_uptime_subscription, "delay") as delay: repair_config_store(cluster="default", key_prefix="b") - - assert delay.call_count == 1 - assert not cluster.exists(sentinel) - - # One missing fits the cap: the sentinel returns while that one is still in flight. - _publish(lost[0], ["b1"]) - with mock.patch.object(update_remote_uptime_subscription, "delay") as delay: + assert not cluster.exists(sentinel) repair_config_store(cluster="default", key_prefix="b") - delay.assert_called_once_with(uptime_subscription_id=lost[1].id, region_slugs=["b1"]) + assert delay.call_args_list == [ + mock.call(uptime_subscription_id=first.id, region_slugs=["b1"]), + mock.call(uptime_subscription_id=second.id, region_slugs=["b1"]), + ] assert cluster.exists(sentinel) + + with mock.patch.object(repair_config_store, "delay") as repair: + check_config_sentinels() + assert not repair.called From 6f0ca83504e19b511f706d6bb26b4497cc7ac79a Mon Sep 17 00:00:00 2001 From: Vinayak Rao Date: Tue, 22 Sep 2026 15:36:51 -0700 Subject: [PATCH 8/8] fix(uptime): Clear a leftover repair cursor once the sentinels are back A repair cursor should only exist while a pass is running, and a pass keeps at least one sentinel missing until its last run. Two things can still leave one behind with every sentinel present: a repair run that overlaps the pass's last run and writes its cursor after that run cleared it and wrote the sentinels, or a worker still on the old code ending the pass without touching the cursor. A leftover cursor lived for its 1h TTL, and a loss in that hour resumed from it: every missing id sorting before it was skipped, the pass then wrote the sentinels back, and those configs were left to the sweep. check_config_sentinels now deletes a store's cursor whenever all of its sentinels are present, since no pass can be running then; a leftover cursor lasts about a minute. The delete runs after the gauge in its own try, because the cursor key can sit on a different shard and a failed delete must not cost that store's gauge. A second loss during a pass is still left to the sweep. Refs INFRENG-621 --- src/sentry/uptime/config_drift.py | 4 ++++ src/sentry/uptime/subscriptions/tasks.py | 8 ++++++++ tests/sentry/uptime/test_config_drift.py | 18 +++++++++++++++++- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/sentry/uptime/config_drift.py b/src/sentry/uptime/config_drift.py index 64e46d2a5ac6..24e72862f817 100644 --- a/src/sentry/uptime/config_drift.py +++ b/src/sentry/uptime/config_drift.py @@ -150,6 +150,10 @@ def get_repair_cursor_key(key_prefix: str) -> str: return f"{key_prefix}uptime:configs:repair-cursor" +def clear_repair_cursor(store: ConfigStore) -> None: + redis.redis_clusters.get(store.cluster).delete(get_repair_cursor_key(store.key_prefix)) + + def find_missing_sentinels(store: ConfigStore) -> list[int]: cluster = redis.redis_clusters.get(store.cluster) pipe = cluster.pipeline() diff --git a/src/sentry/uptime/subscriptions/tasks.py b/src/sentry/uptime/subscriptions/tasks.py index 0d87cdb81697..e6c40d8ac9cc 100644 --- a/src/sentry/uptime/subscriptions/tasks.py +++ b/src/sentry/uptime/subscriptions/tasks.py @@ -19,6 +19,7 @@ SUBSCRIPTION_ID_PREFIX_BUCKETS, SWEEP_RUN_INTERVAL, ConfigStore, + clear_repair_cursor, find_missing_configs, find_missing_configs_for_store, find_missing_sentinels, @@ -402,6 +403,13 @@ def check_config_sentinels(**kwargs): tags={"cluster": store.cluster}, sample_rate=1.0, ) + if not missing: + # No pass runs while every sentinel is present, so a cursor here was left by an + # overlapping run or an old worker and must not carry over into the next loss. + try: + clear_repair_cursor(store) + except Exception: + logger.exception("uptime.config_drift.repair_cursor_clear_failed") if missing and not options.get("uptime.config-drift.sentinel-repair-disabled"): logger.warning( "uptime.config_drift.sentinel_missing", diff --git a/tests/sentry/uptime/test_config_drift.py b/tests/sentry/uptime/test_config_drift.py index adee8ecaf27b..3557fcadec85 100644 --- a/tests/sentry/uptime/test_config_drift.py +++ b/tests/sentry/uptime/test_config_drift.py @@ -14,7 +14,7 @@ from sentry.testutils.cases import UptimeTestCase from sentry.testutils.helpers import override_options from sentry.testutils.helpers.datetime import freeze_time -from sentry.uptime.config_drift import get_sentinel_key +from sentry.uptime.config_drift import get_repair_cursor_key, get_sentinel_key from sentry.uptime.config_producer import ( get_config_key, get_partition_from_subscription_id, @@ -456,3 +456,19 @@ def test_unpublishable_configs_end_the_pass(self) -> None: with mock.patch.object(repair_config_store, "delay") as repair: check_config_sentinels() assert not repair.called + + @override_options({"uptime.config-drift.sentinel-repair-disabled": False}) + def test_leftover_cursor_does_not_skip_the_next_loss(self) -> None: + with self.tasks(): + check_config_sentinels() + # Left by an overlapping run after the sentinels came back, past every id. + redis.redis_clusters.get("default").set(get_repair_cursor_key("b"), "f" * 32, ex=60) + + # Its first check sees every sentinel present, which is where the cursor is cleared. + [subscription], _ = self._seed_lost_on_b() + with self.tasks(): + check_config_sentinels() + + self.assert_redis_config( + "b1", subscription, "upsert", UptimeSubscriptionRegion.RegionMode.ACTIVE + )