From 7b79fdcdbe2d80b8d54612649541d9190d8a212f Mon Sep 17 00:00:00 2001 From: Vamsi Karthik R Date: Wed, 19 Aug 2026 00:58:33 +0000 Subject: [PATCH] ci(playwright): stop SLO-Measurement backfill from serializing 21 daily chunks (#13870) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What this fixes The `SLO-Measurement` shard (`slo-timeslice.spec.js` + `slo-alerts.spec.js`) has been taking **~26 minutes** ## Root cause Both specs seed **8 days of history** and create SLOs with a **7-day rolling window**, then block in `beforeAll` until the SLO is actually measured (`waitForSloMeasured`). The server backfills history in `ZO_SLO_BACKFILL_CHUNK_SECS` chunks — default **86400s (1 day)** — and only **one chunk at a time** (`ZO_SCHEDULER_SLO_BACKFILL_CONCURRENCY=1`, by design, so bulk backfill never starves real-time alerts). The shard backfills **3 SLOs** across the two specs, each needing 7 daily chunks to cover its 7-day window: - `slo-timeslice.spec.js` `beforeAll`: 2 SLOs (`sloLt`, `sloGt`) × 7 chunks = 14 - `slo-alerts.spec.js` `beforeAll`: 1 SLO × 7 chunks = 7 **21 chunks total, strictly serial**, at roughly 40s/chunk ≈ **~14 minutes of pure backfill wait**, before any UI assertion runs. This is the server behaving correctly and exactly as designed for production — the test is just paying for it the slow way. ## Fix Neither spec asserts on per-chunk timing or partial coverage, only on the final measured state, so raising the chunk size to cover the whole window in one pass is free wall-clock, not a behavior change. - Threaded `slo_backfill_chunk_secs` through `build-ci-matrix.js`'s existing per-shard passthrough, same pattern already used for `ingest_allowed_upto` / `quick_mode_enabled`. - Set it to `604800` (7 days) for the `SLO-Measurement` shard only — the server-wide default (`86400`) is untouched everywhere else. - Documented the new knob in `tests/ui-testing/ci-matrix/README.md`. ## Validation **Synthetic (API-only), same seed shape as the real specs** (8 days, 1-min interval, deterministic 3h fault window), against a local build: | | Default (86400s chunk) | Fixed (604800s chunk) | |---|---|---| | Time to measured | 246.9s (~4.1 min) for **one** SLO, stepping 1/7 → 2/7 → ... every ~40s | **3.0s** | | Final SLI | 98.21428571428571% | 98.21428571428571% (**identical**) | Same math, same result — only the number of scheduler round-trips changes. **Real shard, end-to-end**, run exactly as CI invokes it (`npx playwright test slo-timeslice.spec.js slo-alerts.spec.js --workers=1`) against a server mirroring this shard's full env including the fix: - **25/25 tests passed**, 0 regressions - Total wall-clock (server start → global teardown): **4m Avg** Local timing won't map 1:1 onto the GitHub Actions runner (different disk/scheduler characteristics — even the "before" baseline was faster locally than the ~26min/~5min-per-spec seen in CI), so the authoritative number is whatever this PR's own CI run reports. But the structural win — 1 chunk instead of 7 per SLO — is verified correct and regression-free independent of machine. ## Companion PR The enterprise repo's `playwright.yml` independently consumes per-shard matrix fields (it explicitly mirrors OSS's `ingest_allowed_upto` handling for the same reason), so it needs the equivalent env passthrough or it'll keep paying the old cost even after this merges. Companion PR: openobserve/o2-enterprise#2460 --- .github/scripts/build-ci-matrix.js | 11 +++++++++++ .github/workflows/playwright.yml | 10 ++++++++++ tests/ui-testing/ci-matrix/README.md | 7 +++++++ tests/ui-testing/ci-matrix/ci_matrix.json | 3 ++- 4 files changed, 30 insertions(+), 1 deletion(-) diff --git a/.github/scripts/build-ci-matrix.js b/.github/scripts/build-ci-matrix.js index f69398d76a..394c0c68a7 100644 --- a/.github/scripts/build-ci-matrix.js +++ b/.github/scripts/build-ci-matrix.js @@ -115,6 +115,13 @@ for (const s of include) { // silently DROPS older rows while still returning a success-shaped response. A // suite that seeds history — SLOs measure a rolling 7-day window — needs a wider // one, and scoping it to those shards keeps every other shard on the default. +// +// slo_backfill_chunk_secs follows the same rule for the same class of reason: +// it's how much history one SLO backfill chunk covers, read once at server +// start. The workflow default (86400 = 1 day) makes a 7-day-window test SLO +// backfill in 7 sequential chunks under ZO_SCHEDULER_SLO_BACKFILL_CONCURRENCY=1; +// SLO-Measurement widens it to the full window so its 3 backfilled SLOs don't +// pay 21 chunks serially. const matrix = include.map((s) => ({ testfolder: s.testfolder, actual_folder: s.actual_folder, @@ -126,6 +133,10 @@ const matrix = include.map((s) => ({ s.ingest_allowed_upto === undefined || s.ingest_allowed_upto === null ? "" : String(s.ingest_allowed_upto), + slo_backfill_chunk_secs: + s.slo_backfill_chunk_secs === undefined || s.slo_backfill_chunk_secs === null + ? "" + : String(s.slo_backfill_chunk_secs), // Per-shard worker count. Empty = use playwright.config.js (5 in CI). // // `fullyParallel` runs separate spec FILES concurrently, and diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index 5cd4d1ebd5..0d8441bc07 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -32,6 +32,7 @@ env: ZO_QUICK_MODE_STRATEGY: first ZO_ALLOW_USER_DEFINED_SCHEMAS: true ZO_INGEST_ALLOWED_UPTO: 5 + ZO_SLO_BACKFILL_CHUNK_SECS: 86400 # matches the server default; per-shard override below ZO_FEATURE_QUERY_EXCLUDE_ALL: false ZO_USAGE_BATCH_SIZE: 200 ZO_USAGE_PUBLISH_INTERVAL: 2 @@ -259,6 +260,15 @@ jobs: # Set via "ingest_allowed_upto" in ci-matrix/ci_matrix.json; shards that # omit it keep the workflow default. ZO_INGEST_ALLOWED_UPTO: ${{ matrix.ingest_allowed_upto || env.ZO_INGEST_ALLOWED_UPTO }} + # Same per-shard rule: how much history one SLO backfill chunk covers. + # The production default (86400 = 1 day) makes a 7-day-window test SLO + # backfill in 7 sequential chunks under ZO_SCHEDULER_SLO_BACKFILL_CONCURRENCY=1 + # (SLO-Measurement's shard note above) — no test here asserts on + # per-chunk timing or partial coverage, only on the final measured + # state, so a shard-only larger chunk is free wall-clock, not a + # behavior change. Set via "slo_backfill_chunk_secs" in + # ci-matrix/ci_matrix.json; shards that omit it keep the server default. + ZO_SLO_BACKFILL_CHUNK_SECS: ${{ matrix.slo_backfill_chunk_secs || env.ZO_SLO_BACKFILL_CHUNK_SECS }} run: chmod +x ./release-ci-binary/openobserve && ./release-ci-binary/openobserve > o2.log 2>&1 & - name: Wait for start diff --git a/tests/ui-testing/ci-matrix/README.md b/tests/ui-testing/ci-matrix/README.md index 4c24e73534..4d0a39f9c6 100644 --- a/tests/ui-testing/ci-matrix/README.md +++ b/tests/ui-testing/ci-matrix/README.md @@ -64,6 +64,13 @@ The last three exist because a shard is the smallest unit that can change them. `workers: 1`; the specs cannot express that themselves. `SLO-Measurement` does, because both of its specs wait on the SLO backfill job, which runs with `ZO_SCHEDULER_SLO_BACKFILL_CONCURRENCY=1`. +- `slo_backfill_chunk_secs` — the server default is **86400** (1 day), so a test + SLO with a 7-day window backfills in 7 sequential chunks under the concurrency-1 + rule above. `SLO-Measurement` sets `604800` so each of its 7-day-window SLOs + backfills in a single chunk instead. Safe only because no spec in that shard + asserts on per-chunk timing or partial coverage — only on the final measured + state. If a future spec in this shard needs to see backfill mid-flight, give + that spec its own shard rather than lowering this back down. ## Disabling a spec (JSON has no `//` comments) diff --git a/tests/ui-testing/ci-matrix/ci_matrix.json b/tests/ui-testing/ci-matrix/ci_matrix.json index ab9e3e2f45..ddddc8a734 100644 --- a/tests/ui-testing/ci-matrix/ci_matrix.json +++ b/tests/ui-testing/ci-matrix/ci_matrix.json @@ -436,10 +436,11 @@ }, { "testfolder": "SLO-Measurement", - "_comment": "SEPARATE SHARD ON PURPOSE. These two specs seed 8 days of history and then wait for the backfill job, which fills roughly one day-chunk per 40s with ZO_SCHEDULER_SLO_BACKFILL_CONCURRENCY=1. Any spec creating SLOs on the same instance queues behind them and starves the wait, so they must not share a runner with the SLO shard. Both specs are mode:serial and pay the backfill once in beforeAll (~5 min each).", + "_comment": "SEPARATE SHARD ON PURPOSE. These two specs seed 8 days of history and wait for the backfill job (ZO_SCHEDULER_SLO_BACKFILL_CONCURRENCY=1, so its 3 backfilled SLOs queue strictly one at a time). slo_backfill_chunk_secs is bumped from the 86400 (1-day) server default to the full 604800 (7-day) window each of those SLOs uses, so each finishes in 1 chunk instead of 7 -- no test here asserts on per-chunk timing or partial coverage, only on the final measured state. Any spec creating SLOs on the same instance queues behind them and starves the wait, so they must not share a runner with the SLO shard. Both specs are mode:serial and pay the backfill once in beforeAll.", "actual_folder": "SLO", "browser": "chrome", "ingest_allowed_upto": "240", + "slo_backfill_chunk_secs": "604800", "workers": 1, "run_files": [ "slo-timeslice.spec.js",