mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-04 00:08:55 +00:00
Loakesh Indiran
f16e8e53ff
Synthetics performance and correctness work. Four areas: caching the
config reads on the job path, a false-alarm bug in validation, four HA
correctness bugs found by auditing what happens with more than one alert
manager, and the Steps tab moving to a step-grain stream.
Pairs with o2-enterprise#2325 — **merge this first**, that one calls
functions introduced here.
---
## Caching
- [x] Probe-token auth cache — 10s TTL, negative results cached so a bad
token can't force a query per request
- [x] Locations whole-table cache — 30s, serves `get` / `find_by_pool` /
`list_visible` from one load
- [x] Check definition cache via `get_cached()` — was read twice per job
(resolve + ack)
- [x] Agent capabilities cache via `get_cached()` — was read on every
lease poll
- [x] Org ingest-token cache — `find_default_enabled()` replaces three
hand-rolled `list_by_org().find(enabled)` copies
- [x] `list_by_locations()` — one batched agents query, replacing one
per location
## Cross-node invalidation
- [x] `coordinator::synthetics` — event invalidation following the
alerts/pipelines pattern
- [x] Write paths call `invalidate_and_publish()`, the watcher calls
plain `invalidate_cache()` so events don't echo
- [x] Deletes emit a delete event, so coordinator keys don't accumulate
one per check ever created
- [x] Ingest-token invalidation reuses the existing
`/org_ingestion_tokens/` watch — no second event stream
- [x] SQLite write mutex released before emitting — `get_lock()` and the
coordinator's `put()` take the same lock, so emitting while holding it
deadlocked the process
## HA — correctness with more than one alert manager
Confirmed with @subhradeep that a region can run multiple alert
managers. The synthetics scheduler, dispatcher, reaper and staleness
watcher all run on **every** one of them.
- [x] **Duplicate runs** — `claim_due()` claims due checks with `SELECT
… FOR UPDATE SKIP LOCKED`, advancing the schedule inside the same
transaction
- [x] **Duplicate "location down" pages** — the one-shot flag moved from
an in-process `HashSet` to `synthetics_locations.down_notified_at`,
claimed with a CAS
- [x] **Duplicate run completion** — `increment_jobs_done` claims
completion via `completed_at IS NULL`; two acks of the same run could
both be told they completed it
- [x] **Inert dedup index** — `scheduled_ts` is now the schedule slot,
not each node's tick clock, so the existing `synthetics_jobs_dedup_uq` +
`ON CONFLICT DO NOTHING` actually fires
- [x] Migration `m20260803_000001` adds `down_notified_at`, defaulted so
no backfill is needed
- [x] `DB_SCHEMA_VERSION` 62 → 63 — without it the migration never runs
on an already-upgraded deployment
## The false-alarm fix
- [x] Validate against the run budget (840s), not the job lease (900s)
- [x] The three bounds are deployment-configurable:
`O2_SYNTHETICS_MAX_CHECK_BUDGET_SECS` / `_JOB_LEASE_SECS` /
`_MAX_NET_TIMEOUT_MS`
- [x] Rejected limits are logged, not fatal — nothing installed means
validation keeps the conservative defaults
- [x] Browser retries capped at 2; protocol types stay at 3; default
stays 0 for every type
## Steps tab — the step-grain stream (B10)
- [x] Three `GROUP BY`s over the step stream replace downloading 5000
execution rows and tallying their JSON in the browser
- [x] Falls back to the old tally when the stream is empty, so the tab
does not blank for checks whose agents have not been upgraded
- [x] Gated on the `step_id` column rather than stream existence — a
partially-created stream would otherwise cause four failed queries
## Overview KPIs
- [x] Count tiles summed from the cached histogram; `buildKpiSql`
deleted
## Rename
- [x] `synthetics_monitors` → `synthetics_checks`, `monitor_type` →
`check_type`
- [x] Three UI call sites that read the old field names, including a
delete guard that silently disabled itself
---
**Why validation moved off the lease.** Three bounds stack:
```
check worst case <= max_check_budget_secs < job_lease_secs
840s 840s 900s
```
Validation used the lease, so the server accepted checks the execution
environment cannot finish. On the managed path the probe is a Lambda,
and a function that hits its timeout is killed mid-run — lease headroom
is irrelevant once the process is gone. The net probe's function timeout
had drifted to **60s** while validation accepted up to 900s, so a plain
`timeout_ms=30s, retries=2` (worst case 100s) was accepted and then
killed, reporting a failure the target never had.
**Why the limits are ENT-declared but OSS-consumed.** Synthetics is
enterprise-only, so the values live in `SyntheticsConfig`. The
validation they bound lives in `config`, which cannot depend on
`o2_enterprise`. `SyntheticsLimits` + `init_limits` is the seam, falling
back to built-in defaults in OSS builds and tests. A bad limit is logged
rather than fatal: refusing to start ingest, search and dashboards
because a probe ceiling is misconfigured would be the worse outage.
**The duplicate-run bug.** `fetch_due` was a plain `SELECT` and the
advance was an unconditional `UPDATE … WHERE id = ?`, so nothing stopped
two alert managers firing the same check. Each inserted its own
`synthetics_runs` row under a fresh KSUID, which made the resulting job
rows genuinely distinct — `lease_batch`'s `status = 0` guard cannot
collapse them, because they are not contending for one row. N alert
managers meant every check ran N times.
`SKIP LOCKED` rather than a compare-and-swap because it *distributes*: a
second scheduler skips the rows the first holds and picks up different
due checks, so N replicas split the backlog. A CAS gives the same
exactly-once guarantee but every node reads the same candidates, one
wins them all, and the losers each issue a doomed `UPDATE`. The lock and
the advance share a transaction because the locks only live until
`COMMIT`; the fan-out stays outside it so config rows are not locked
across N job inserts.
**Duplicate run completion needed no second alert manager.** The
increment was atomic, but the completion decision re-read the row in a
second statement and returned "complete" to whichever caller saw
`jobs_done >= job_count` — which both acks of a 2-job run do. Acks are
served on ingesters, of which every environment runs two, and the probes
of one run finish independently. Effect: `consecutive_failures` counted
one run twice on every check, plus a genuinely duplicated notification
on checks left at the default `cooldown_mins = 0`.
**The Steps tab was truncating silently.** It downloaded up to 5000
execution rows and tallied `last_attempt_steps` in the browser. On the
busiest check measured — 18,079 executions over 7 days — that cap
covered 4 of the 7 days and reported a **56.3% fail rate against a true
33.7%**. The win is correctness; the ~18MB payload reduction is
secondary.
## Verification
`cargo check --workspace`, clippy, fmt clean. Config 98, infra 33
synthetics tests pass. Frontend: **839 synthetics tests pass**. Three
frontend spec files fail to import on `useLocalTimezone` — verified
identical on clean `main`, unrelated to this branch.