-
Notifications
You must be signed in to change notification settings - Fork 269
feat(budget): park lost settlements durably and fold them at pre-check #162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 18 commits
2934804
228396e
033c637
d36b960
f530d9b
a3a34ce
994ac27
f6059fb
9f9d06d
c4ac608
b3f6ad2
afa9e41
93bb71f
eac6f2a
56606e1
9b82eb2
d26d14f
f0eb0ce
8fa4850
12cc7fc
99b4326
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| """Idempotent startup schema migrations for columns added after the first release. | ||
|
|
||
| `Base.metadata.create_all` creates new tables but never alters existing ones, so a | ||
| deployment that already ran a release (a SQLite named volume, a fly.io/Postgres | ||
| volume) keeps an `api_keys` table without the `spent_microcents` column. After an | ||
| upgrade the ORM would then `SELECT` every mapped column and hit "no such column" | ||
| on every authenticated request — a 503 for the whole API. | ||
|
|
||
| `ensure_budget_columns` runs at boot, after `create_all`, on every start: it | ||
| inspects the live schema, only acts where the change is missing, tolerates | ||
| another process racing it to the same change, and re-attempts a repair that an | ||
| earlier boot applied only halfway. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from collections.abc import Callable | ||
| from typing import Any | ||
|
|
||
| from sqlalchemy import BigInteger, inspect, text | ||
| from sqlalchemy.exc import DBAPIError | ||
|
|
||
| from packages.db.models.budget_park import BudgetPark | ||
|
|
||
|
|
||
| def _already_applied(err: DBAPIError) -> bool: | ||
| """Whether a DDL failure means someone else applied the change first.""" | ||
| msg = str(err).lower() | ||
| if "already exists" in msg or "duplicate column" in msg: | ||
| return True | ||
| # Two concurrent CREATE TABLE are serialised by the catalog rather than by | ||
| # the wording of a complaint, so the loser gets a unique violation on | ||
| # pg_class/pg_type (`*_relname_nsp_index`, `*_typname_nsp_index`) instead of | ||
| # "already exists". Same meaning: the object is there now. | ||
| return "duplicate key value" in msg and "nsp_index" in msg | ||
|
|
||
|
|
||
| async def _apply_ddl(conn, statement: str | Callable[..., Any]) -> None: | ||
| """Run one startup DDL statement, tolerating a boot that raced us to it. | ||
|
|
||
| Every worker runs this in its lifespan, so the first boot after an upgrade | ||
| has several processes inspecting a schema none of them has altered yet. Each | ||
| then issues the same statement and all but one fail — "column ... already | ||
| exists" on Postgres, "duplicate column name" on SQLite — which is success | ||
| from here, not a reason to keep the worker from booting. The failure is | ||
| caught inside a SAVEPOINT because on Postgres an error would otherwise abort | ||
| the whole transaction and take the rest of the startup with it. | ||
|
|
||
| `statement` is raw SQL, or a callable handed to `run_sync` for DDL that only | ||
| the dialect's own generator can emit (a `Table.create`). | ||
| """ | ||
| try: | ||
| async with conn.begin_nested(): | ||
| if callable(statement): | ||
| await conn.run_sync(statement) | ||
| else: | ||
| await conn.execute(text(statement)) | ||
| except DBAPIError as err: | ||
| if not _already_applied(err): | ||
| raise | ||
|
|
||
|
|
||
| async def ensure_budget_columns(engine) -> None: | ||
| """Make `api_keys.spent_microcents` correct, whatever schema it started from. | ||
|
|
||
| Adds the column when an upgraded volume lacks it, and re-seeds lifetime spend | ||
| from historical request logs on every boot so the `ALTER` is never mistaken | ||
| for proof that the seed ran. Also widens `budget_limit_cents` to BIGINT on | ||
| Postgres (the column is scaled into microcents for every comparison against | ||
| spend, and an int4 ceiling is about 214,748 dollars of lifetime budget), | ||
| creates the `ix_requests_log_api_key_spend` index that create_all only builds | ||
| on fresh databases, and creates `budget_parks` for deployments that predate | ||
| the durable-recovery release — a lost settlement needs somewhere every | ||
| worker, and every reboot, can see it. Each step costs nothing on a database | ||
| that needs none of it; there the seed is a single indexed UPDATE that matches | ||
| no row. | ||
| """ | ||
| async with engine.begin() as conn: | ||
| tables = set( | ||
| await conn.run_sync(lambda sync: inspect(sync).get_table_names()) | ||
| ) | ||
| cols = { | ||
| c["name"]: c["type"] | ||
| for c in await conn.run_sync(lambda sync: inspect(sync).get_columns("api_keys")) | ||
| } | ||
| is_postgres = engine.dialect.name == "postgresql" | ||
|
|
||
| if BudgetPark.__tablename__ not in tables: | ||
| # `create_all` covers fresh databases; this covers upgrades whose | ||
| # schema predates the table. `checkfirst` re-reads the catalog, and | ||
| # that read cannot see another boot's uncommitted CREATE — so two | ||
| # workers both get here and one loses anyway. It goes through | ||
| # `_apply_ddl` for the same reason the ALTER does: losing that race | ||
| # has to count as having won, and the savepoint is what keeps the | ||
| # error from aborting the transaction the rest of startup runs in. | ||
| await _apply_ddl( | ||
| conn, | ||
| lambda sync: BudgetPark.__table__.create(sync, checkfirst=True), | ||
| ) | ||
|
|
||
| # The model declares ix_requests_log_api_key_spend (api_key_id, | ||
| # is_deleted); create_all only builds it on fresh databases, so an | ||
| # upgraded deployment would drift. Built before the seed below, which is | ||
| # a correlated aggregate over requests_log and otherwise full-scans the | ||
| # one table that grows without bound here — once per key, inside the | ||
| # transaction that already holds the api_keys lock. is_deleted has | ||
| # existed since the first release (SoftDeleteMixin), so the index is | ||
| # always creatable. | ||
| idx = { | ||
| i["name"] | ||
| for i in await conn.run_sync( | ||
| lambda sync: inspect(sync).get_indexes("requests_log") | ||
| ) | ||
| } | ||
| if "ix_requests_log_api_key_spend" not in idx: | ||
| await _apply_ddl( | ||
| conn, | ||
| "CREATE INDEX IF NOT EXISTS ix_requests_log_api_key_spend " | ||
| "ON requests_log (api_key_id, is_deleted)", | ||
| ) | ||
|
|
||
| if "spent_microcents" not in cols: | ||
| await _apply_ddl( | ||
| conn, | ||
| "ALTER TABLE api_keys ADD COLUMN spent_microcents BIGINT " | ||
| "NOT NULL DEFAULT 0", | ||
| ) | ||
|
|
||
| # Seed lifetime spend from historical request logs so an existing key's | ||
| # cap is not silently reset to zero (which would re-grant a leaked key | ||
| # a full new budget). Deliberately unfiltered by is_deleted, unlike the | ||
| # analytics reads over the same table: this restores an accrued total, | ||
| # so counting a row a retention job has hidden can only ever make the | ||
| # cap tighter, while honouring the filter would hand a capped key | ||
| # back the spend it was capped for. | ||
| # | ||
| # Every boot, not only beside the ALTER, because the ALTER is no record | ||
| # of the seed: on SQLite that DDL is durable the instant it executes | ||
| # while this is DML in the transaction a kill — or the `database is | ||
| # locked` this very aggregate provokes on an upgrade that overlaps the | ||
| # old machine's writes — rolls back, and a gate keyed on the column | ||
| # would then never retry. It is idempotent because a log row and its | ||
| # charge are one commit, so this SUM *is* the lifetime counter and a key | ||
| # already holding spend has nothing to restore. Capped keys only: an | ||
| # uncapped key is never charged, so its counter stays at zero by design | ||
| # and nothing reads it. | ||
| await conn.execute( | ||
| text( | ||
| "UPDATE api_keys SET spent_microcents = (" | ||
| " SELECT CAST(COALESCE(SUM(cost_microcents), 0) AS BIGINT) FROM requests_log " | ||
| " WHERE requests_log.api_key_id = api_keys.id" | ||
| ") WHERE spent_microcents = 0 AND budget_limit_cents IS NOT NULL" | ||
| ) | ||
| ) | ||
|
|
||
| limit_type = cols.get("budget_limit_cents") | ||
| if is_postgres and limit_type is not None and not isinstance(limit_type, BigInteger): | ||
| await _apply_ddl( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 P1 Seed UPDATE that races a live budget charge permanently drops the key's historical spend The boot seed
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 99b4326 — the seed is now monotonic instead of gated on The gate is now Why this closes the race: a live charge and its The monotonic direction also means the seed can only ever tighten a cap, never loosen one, so it cannot hand a capped key back spend it was capped for. Covered by |
||
| conn, "ALTER TABLE api_keys ALTER COLUMN budget_limit_cents TYPE BIGINT" | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| """Unsettled budget obligations — delivered spend the ledger never recorded. | ||
|
|
||
| A settlement that gives up after every retry leaves a delivered cost with no | ||
| record anywhere: the log row and the charge are one transaction, so both roll | ||
| back and `spent_microcents` never moves. The obligation is parked here, one row | ||
| per settlement, so it outlives the process that lost it and is visible to every | ||
| worker behind the same database. | ||
|
|
||
| Rows are keyed by the settlement's `trace_id`, which makes every park write | ||
| idempotent: a commit that applied but whose ack was lost retries into the same | ||
| primary key instead of recording the obligation twice, and a write that fails | ||
| outright is checked for having landed anyway before the process falls back to | ||
| holding it in memory. A fold that bills a row either deletes it or shrinks it to | ||
| what the cap could not absorb, in the same transaction that moves | ||
| `spent_microcents`, and it drops the writer's memory hold for any row it fully | ||
| billed. | ||
|
|
||
| What that leaves is a double charge needing three faults at once — the ack lost, | ||
| the durability probe failing alongside it, and another worker folding the row | ||
| before this one retries — at which point the extra charge lands on a key that had | ||
| already breached its cap. Closing that last window means a fold leaving a tombstone | ||
| behind instead of deleting, so a re-file always collides with something; that is a | ||
| second state the queue has to drain, and it is not worth its weight here. | ||
| """ | ||
|
|
||
| from datetime import datetime, timezone | ||
|
|
||
| from sqlalchemy import BigInteger, DateTime, String | ||
| from sqlalchemy.orm import Mapped, mapped_column | ||
|
|
||
| from packages.db.models.base import Base, TimestampMixin, UUIDMixin | ||
|
|
||
|
|
||
| class BudgetPark(Base, UUIDMixin, TimestampMixin): | ||
| __tablename__ = "budget_parks" | ||
|
|
||
| trace_id: Mapped[str] = mapped_column(String(36), nullable=False, unique=True) | ||
| api_key_id: Mapped[str] = mapped_column(String(36), nullable=False, index=True) | ||
| microcents: Mapped[int] = mapped_column(BigInteger, nullable=False) | ||
| # Overrides TimestampMixin's column, whose `server_default=func.now()` is | ||
| # CURRENT_TIMESTAMP: one second wide on SQLite, where a recovered outage | ||
| # re-files a whole batch of obligations in a single pre-check and every row | ||
| # in it ties. The fold bills oldest debt first, and two workers have to | ||
| # agree on which row the partial one is, so the stamp needs enough | ||
| # resolution to settle that on its own instead of falling through to the | ||
| # `trace_id` tiebreak — which is a uuid4, and so picks at random. | ||
| created_at: Mapped[datetime] = mapped_column( | ||
| DateTime(timezone=True), | ||
| default=lambda: datetime.now(timezone.utc), | ||
| nullable=False, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 P1 Make the lifetime-spend seed monotonic so legacy rows committed during the rollout overlap are not lost forever
The seed
UPDATE api_keys SET spent_microcents = (SELECT SUM(cost_microcents) FROM requests_log ...) WHERE spent_microcents = 0 AND budget_limit_cents IS NOT NULLruns at every boot, but thespent_microcents = 0gate makes it one-shot per key: the first boot after upgrade snapshots the SUM at that instant. The comment above the statement itself anticipates "an upgrade that overlaps the old machine's writes" — the previous release keeps serving against the same database and keeps committing requests_log rows with real cost_microcents. Any such row that commits after the boot's seed snapshot is absent from the SUM, and the moment the new release charges even one request for that capped key (charge_budget), spent_microcents leaves 0, so every later boot's seed skips the key and the late legacy row is never counted. The key's lifetime spend is then permanently understated by the amount the old release recorded during the overlap, and the hard cap (which is only enforced against this counter) lets the key serve past its true lifetime spend. The re-seed-every-boot mechanism only repairs a seed that rolled back, not rows that arrive after a successful seed. Fix: make the seed monotonic instead of gated on exactly zero — e.g.SET spent_microcents = max(spent_microcents, (SELECT CAST(COALESCE(SUM(cost_microcents),0) AS BIGINT) FROM requests_log WHERE requests_log.api_key_id = api_keys.id)) WHERE budget_limit_cents IS NOT NULL— since every new charge also writes a log row, the SUM is always >= the counter, so the max tops the counter up with late legacy rows without double-counting and never moves it down.