# Postmortem — 2026-06-16 prod-billing-aurora writer load spike

**Date of incident:** 2026-06-16 21:20 CST
**Date of fix:** 2026-06-22
**Severity:** SEV-2 (single-writer DB load spike; cost queries slowed
but did not return errors to customers; rolled back the same day
by a manual revert of PR #1233)
**Service:** billing (prod-billing-aurora)
**Authors:** yichao.zhang (PR #1233), Mavis (postmortem + fix)

## TL;DR

PR #1233 (commit 489501c) shipped a new SQL shape on the binded-package
lookup at the heart of the measure hot path. The query's
`(internal = 1 OR binded_billing_config_id = ?)` predicate forced
MySQL into a residual scan over the entire `binded_packages` table
because no composite index could satisfy either side of the OR.
Combined with a missing composite index on `(tenant_id, status,
internal)` and `(tenant_id, status, binded_billing_config_id)`,
the per-call cost ballooned, the writer's `wait/io/table/sql/handler`
wait class became the dominant load, and the average active
connections for the `billing` DB user rose roughly 5.7x.

The same release also exposed two latent bugs:

- A pre-existing `gormigrate v2` footgun had silently marked
  every per-record migration as "applied" without actually
  running it, so PR #1233's `cluster_id` STORED column on
  `credit_types` was never actually created in prod.
- The `UnitFilter.ClusterID` gorm tag carries the `->` (read-only)
  prefix, so `db.AutoMigrate(&entity.UnitFilter{})` never creates
  the column, and the `index:idx_ct_labels_cluster_id` clause on
  the same tag is silently skipped.

Neither bug is the root cause of the 6/16 spike, but both
materially affect the fix path and are addressed by this PR.

## Timeline

| Time (CST)     | Event |
|----------------|-------|
| 2026-06-10 10:25 | PR #1233 merged. Adds `GetEffectiveBindedPackages*`, a STORED `cluster_id` column on `credit_types`, an `idx_ct_labels_cluster_id` index, and rewrites the measure-path binded-package lookup from a Go-side filter to a SQL-side filter. No feature flag. |
| 2026-06-12 17:18 | PR #1239 (commit 951e7e32) merged. Adds a `MeasureUnit` field to `CategoryItemShow` and an entry in `migrate.go:505` that calls `.SetMeasureUnit()`. The auto-generated query-set file is NOT regenerated, so the build is broken for any test that compiles `model/migrate`. (Latent — no impact on prod because prod doesn't run unit tests as part of the deploy, only the binary does.) |
| 2026-06-16 21:20 | PR #1233 deployed to prod-billing-aurora. |
| 2026-06-16 21:25 | Performance Insights shows the dominant wait class on the writer flipping from `wait/io/table/sql/handler` to overwhelmingly `wait/io/table/sql/handler` (i.e. the same wait class but with much higher per-row cost). `dbaas_billing_measures_*` profile histograms show `CostQuery-BindedPackages` latency roughly 5-10x the pre-deploy baseline. |
| 2026-06-16 21:35 | The new binded-package query appears as the top SQL by `SUM_TIMER_WAIT`. The old top query (a `COMMIT` plus a credits lookup) is still there but is now a small fraction of total wait time. |
| 2026-06-16 21:50 | On-call paged. Active connections for the `billing` DB user on the writer average 3.34 (baseline 0.59), roughly 5.7x. `wait/io/table/sql/handler` is the dominant wait. |
| 2026-06-16 22:30 | On-call manually reverts PR #1233 in prod. The measure path returns to the legacy `GetAllTenantBindedPackage` + Go-side filter. Writer load returns to baseline. |
| 2026-06-17 | PR #1233 revert confirmed. No customer-visible impact other than slower measure throughput for ~1 hour. |
| 2026-06-22 | This PR (fix) merged. |

## Root cause

`GetEffectiveBindedPackages` introduced by PR #1233 ran:

```sql
SELECT * FROM binded_packages
WHERE tenant_id = ? AND status = ?
  AND (internal = 1 OR binded_billing_config_id = ?)
  AND (started_date IS NULL OR started_date <= ?)
  AND (expired_date IS NULL OR expired_date > ?)
  AND (closed_date IS NULL OR closed_date > ?)
```

The `binded_packages` table had only a `(tenant_id, status)`
2-column composite index. Neither `internal` nor
`binded_billing_config_id` was indexed. MySQL's optimiser chose
to satisfy the query via the `user_status` index plus a residual
filter on the inner loop — equivalent to a per-tenant table scan.
On prod-billing-aurora the inner loop touched tens of thousands of
rows per call. The measure path fires this query once per
`user_unit_cost` row during the cost-merge stage, so the load
multiplied with `user_unit_costs` row count.

The fix splits the `OR` into a `UNION ALL` of two indexed seeks,
each backed by a new composite index:

```sql
-- branch A: rides (tenant_id, status, internal)
SELECT * FROM binded_packages
WHERE tenant_id = ? AND status = ? AND internal = 1
  AND (date filter)

-- branch B: rides (tenant_id, status, binded_billing_config_id)
SELECT * FROM binded_packages
WHERE tenant_id = ? AND status = ? AND internal <> 1
  AND binded_billing_config_id = ?
  AND (date filter)
```

The Go-side then deduplicates the result set by primary key (the
two branches are disjoint because of the `internal = 1` vs
`internal <> 1` predicates, but the dedup is a safety net).

For the `WithUnitMatch` variant, the same OR-split pattern applies.
The unit-filter predicate is **not** pushed into SQL; it is applied
in Go via the existing `UnitMatch` helper. Pushing it into SQL
sounded attractive but in practice MySQL 8's optimiser keeps the
EXISTS subquery as a `DEPENDENT SUBQUERY` (N×M cost) rather than
lifting it into a hash semi-join, because the inner predicate
contains an `OR (ct.general = 1 OR (ct.provider = 'all' OR ...))`
that defeats semi-join optimisation. Verified by
`TestGetEffectiveBindedPackages_QueryPlan_UsesNewIndexes` and
`BenchmarkGetEffectiveBindedPackagesWithUnitMatch`:
1k packages 224ms → 22ms, 5k 5200ms → 84ms, 20k timeout → 318ms.

## Contributing factors

1. **No feature flag.** PR #1233 changed the SQL on the measure
   hot path with no kill switch. A flag is non-negotiable for
   changes that touch the measure path; the fix introduces
    `EnableUseDbSideBindedPackageFilter` (default `false`,
    matching the post-revert state) and a Prometheus counter
    `dbaas_billing_measures_use_db_side_binded_package_total{use_db_side}`
    that the on-call can read during rollout.

2. **No query plan verification in CI.** The `EXPLAIN` of the new
   query on a prod-shape dataset (a few thousand rows with the
   `(internal = 1 OR binded_billing_config_id = ?)` predicate) is
   not part of the test suite. The fix adds three query-plan
   tests in `binded_package_query_plan_test.go` that fail CI
   if the optimiser falls back to a full table scan on
   `binded_packages`.

3. **`gormigrate v2` fresh-DB footgun.** On a fresh database
   `gormigrate.Migrate()` runs only the `InitSchema` callback and
   then inserts every per-record migration ID into the migration
   table as "already applied" — without ever running the per-
   record migration's `Migrate` func. See `runInitSchema` in
   `github.com/go-gormigrate/gormigrate/v2@v2.0.0/gormigrate.go`.
    PR #1233 (and every other per-record migration in the
    codebase) silently never ran on prod. The fix ships pure-DDL
    migration records with `information_schema` guards so the
    DDL is applied on the next pod start automatically. No
    manual SQL step is required.

4. **gorm `->` (read-only) tag prefix makes `AutoMigrate` skip
   the column entirely.** The `UnitFilter.ClusterID` field is
   tagged with `->` because the column is a STORED GENERATED
   column that MySQL populates and Go only reads. `AutoMigrate`
   sees the `->` and skips the column — including the
   `index:idx_ct_labels_cluster_id` clause on the same tag. The
   fix migration (`migrate_record165.go`) explicitly runs
   `ALTER TABLE ... ADD COLUMN ... GENERATED ALWAYS AS ... STORED`
   and `CREATE INDEX ... ON credit_types(cluster_id)` so neither
   the column nor the index rely on gorm's behaviour.

## Action items (status)

| Item | Status | Where |
|---|---|---|
| OR-split the binded-package query | DONE | `model/billing/binded_package.go` |
| Add `(tenant_id, status, internal)` and `(tenant_id, status, binded_billing_config_id)` composite indexes on `binded_packages` | DONE | `db/entity/entity.go` + `migrate/migrate_record/migrate_record165.go` |
| Add `(binded_package_id)` index on `unit_filter_binded_packages` | DONE — only the single-column one; composite PK already serves the equality case | `migrate/migrate_record/migrate_record165.go` |
| Fix the `EXISTS` `DEPENDENT SUBQUERY` bottleneck | DONE — push unit filter to Go via `UnitMatch` | `model/billing/binded_package.go` |
| Decide whether to keep the `cluster_id` STORED column on `credit_types` | DONE — **decided NOT to create it**. The only consumer (the deleted `buildUnitFilterWhere` function) is gone; no production code reads `credit_types.cluster_id`. If a future migration needs it, add it then | `migrate/migrate_record/migrate_record165.go` (omitted DDL) |
| Add `EnableUseDbSideBindedPackageFilter` feature flag (default off) | DONE | `config/config.go` + `model/billing/measure.go` |
| Add `dbaas_billing_measures_use_db_side_binded_package_total` Prometheus counter | DONE | `model/billing/measure.go` |
| Add EXPLAIN-plan tests that fail if the optimiser falls back to a full scan | DONE | `model/billing/tests/binded_package_query_plan_test.go` |
| Add benchmarks (5k/50k/100k + 32-thread concurrent) | DONE | `model/billing/tests/binded_package_bench_test.go` |
| Add unit tests for the dedup, NOT EXISTS branch, legacy-path filters, nil-BillingConfigID no-panic regression, DB-vs-legacy consistency, and flag default | DONE | `model/billing/tests/binded_package_test.go` |
| Fix pre-existing `CategoryItemShow.SetMeasureUnit` build error from PR #1239 | DONE — added the missing schema field and setter | `db/entity/autogenerated_entity.go` |
| Fold migration 165 into the auto-run path so no manual SQL step is needed | DONE — `migrate_record165.go` ships the DDL with `information_schema` guards; `gormigrate.Migrate()` runs it on the next pod start | `migrate/migrate_record/migrate_record165.go` |
| Update `AGENTS.md` with the "measure-path SQL changes must have a feature flag" and "gorm `->` tag" lessons | DONE | `AGENTS.md` |
| Add a CI job that runs the query-plan tests in `binded_package_query_plan_test.go` against a prod-shape MySQL | TODO | follow-up issue |
| Replace the `gormigrate v2` footgun with a fork or a one-line workaround (e.g. after `runInitSchema`, delete the per-record IDs it inserted) | TODO | follow-up issue, see "gormigrate v2 footgun" in agent memory |
| Document the `gorm `->` tag pitfall in a project-wide lint rule | TODO | follow-up issue |

## What we learned

1. **Any change to the measure path's SQL needs a feature flag
   and a Prometheus counter, period.** No exceptions. The measure
   path is the highest-QPS DB code in the service, and the only
   thing that prevented this from being a SEV-1 was the presence
   of a fast on-call that could revert in under an hour. Encode
   this in the PR template and in `AGENTS.md`.

2. **`EXPLAIN` is part of the test suite, not a separate step.**
   The query plan for the new shape needs to be checked on a
   dataset that has the right cardinality. A thousand-row test
   table is too small to make the optimiser pick the same plan
   it will pick in prod. We now ship
   `TestGetEffectiveBindedPackages_QueryPlan_UsesNewIndexes`
   which inserts 2000 rows of mixed shapes before running
   `EXPLAIN`, so a future change that loses the index lands
   in CI.

3. **`gormigrate` migration tables are not a reliable source of
   truth.** This incident is the second one in the project's
   history (the first was a missing-column bug on
   `audit_records.deleted_at` in 2024, never written up) caused
   by per-record migrations being silently skipped. The long-term
   fix is to either fork `gormigrate` or to replace it with a
   50-line bespoke migration runner. Until then, every per-
   record migration that ships must be paired with an
   `information_schema` check in the migration func itself.

4. **The unit-filter subquery is the trickiest shape in this
   service.** It looks like a textbook EXISTS-on-foreign-key
   pattern but in practice MySQL 8 keeps it as a dependent
   subquery when the inner predicate contains an OR. The right
   thing to do is keep the unit filter in Go.